Invoke a mask callable exactly once - #102
Open
cemde wants to merge 1 commit into
Open
Conversation
`_process_mask` probed a mask callable's arity by calling it with `is_pytree=True` and retrying on `TypeError`. That cannot distinguish a callable that does not accept the keyword from one that ran and raised, so a broken mask executed twice and reported the second failure chained onto the first. Inspect the signature instead. Closes liukidar#78
cemde
force-pushed
the
fix/78-process-mask-double-call
branch
from
August 9, 2026 18:23
8b67f7e to
2b7acf4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
_process_maskdiscovered whether a mask callable accepts theis_pytreehint by calling it and catching the failure:except TypeErrorcannot distinguish "this callable does not acceptis_pytree" from "this callable ran and raisedTypeErrorfor its own reasons".Impact
A genuinely broken mask executes twice. The user sees the second failure with the first chained onto it, and the traceback points at
_process_maskrather than at their own code. Any side effect in the callable happens twice.It also hid real bugs. Reviewing #68 found that a broken
_M_notwas silently degraded to the one-argument path by this fallback rather than surfacing.Fix
Ask the signature instead of inferring from a failure:
The mask is now invoked exactly once, and any error it raises propagates from the user's own frame, unchained.
inspectwas already imported in this file for_repr_function, so the diff adds no dependency.Why not a narrower
exceptTelling the arity
TypeErrorapart from a user's own would mean matching on the exception message text, which is wording-dependent across Python versions and no smaller than this.Cost
inspect.signatureruns once per callable mask leaf, not per parameter. Measured: +13 µs on_process_mask, which is 0.24% of one eager training step and inside run-to-run noise. Underpxf.jit, how every tutorial spells the step, it runs once per trace rather than per step. No caching warranted.Closes #78