Skip to content
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

Fix bug in custom_vjp with optimize_remat and custom_vmap #23000

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

dfm
Copy link
Collaborator

@dfm dfm commented Aug 12, 2024

When used with a custom_vmap that introduces a new const the previous implementation of optimize_remat would error in its DCE rule because of unexpected consts when closing the fwd jaxpr. This shouldn't have ever been hit, but there was a bug in the batching rule for remat_opt_p where we weren't properly converting constvars to invars. This fixes this bug and should unbreak internal users.

@dfm dfm requested a review from froystig August 12, 2024 12:36
@dfm dfm self-assigned this Aug 12, 2024
@dfm dfm added the pull ready Ready for copybara import and testing label Aug 12, 2024
tests/api_test.py Show resolved Hide resolved
When used with a `custom_vmap` that introduces a new const the previous
implementation of `optimize_remat` would error in its DCE rule because
of unexpected consts when closing the fwd jaxpr. This shouldn't have
ever been hit, but there was a bug in the batching rule for
`remat_opt_p` where we weren't properly converting constvars to invars.
This fixes this bug and should unbreak internal users.
copybara-service bot pushed a commit that referenced this pull request Aug 13, 2024
This is needed for hermetic CUDA integration in Google ML projects since tensorRT is not distributed in the same free way as other CUDA/CUDNN distributives.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#23000 from dfm:dce-bug 850edee
PiperOrigin-RevId: 635618263
@copybara-service copybara-service bot merged commit 955699c into jax-ml:main Aug 13, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants