Skip to content

[mutable-arrays] re-land #29353 #29421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

mattjj
Copy link
Collaborator

@mattjj mattjj commented Jun 12, 2025

Re-land #29353 after rollback

The code in #29353 had a bug: on this line we forgot to offset by len(jaxpr.consts), since the int entries of in_fwd_res: list[int | None] index into [*jaxpr.consts, *known_inputs] where known_inputs = [t.pval.get_known() for t in tracers if t.pval.is_known()].

In #29353 I made the classic hasty blunder of not writing a systematic test for the new code. This updated PR has a very thorough systematic test. I verified it catches the bug in #29353 with JAX_NUM_GENERATED_CASES=100. This new PR passes all tests with JAX_NUM_GENERATED_CASES=9999, which is the damage limit.

@mattjj mattjj self-assigned this Jun 12, 2025
@mattjj mattjj added the pull ready Ready for copybara import and testing label Jun 12, 2025
@mattjj mattjj force-pushed the mutable-array-custom-vjp-scan2-again branch 4 times, most recently from f6ea9ac to 1c06a3d Compare June 13, 2025 05:19
@mattjj mattjj requested a review from dougalm June 19, 2025 19:25
@mattjj mattjj marked this pull request as ready for review June 19, 2025 19:25
@mattjj mattjj force-pushed the mutable-array-custom-vjp-scan2-again branch 2 times, most recently from cfb382e to 30e7c63 Compare June 19, 2025 19:54
@mattjj mattjj force-pushed the mutable-array-custom-vjp-scan2-again branch from 30e7c63 to bb4fbc5 Compare June 19, 2025 20:03
@copybara-service copybara-service bot merged commit 64736c9 into jax-ml:main Jun 20, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kokoro:force-run pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants