Fix varlen ref_gdr chunk offsets after prepare_chunk_offsets return change#16
Open
ReinforcedKnowledge wants to merge 1 commit into
Open
Conversation
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.
Summary
This PR updates those call sites to destructure the return value and pass only
chunk_offsets.Hi! This is a small fix for the varlen reference implementation in
tests/ref_gdr.py.prepare_chunk_offsetsnow returns(chunk_offsets, num_chunks), and the kernel callers already destructure that return value. A few reference helpers intests/ref_gdr.pywere still passing the full tuple intopack/unpack, which expect a tensor.This PR updates those call sites to destructure the return value and pass only
chunk_offsets.Reproduction
On current
main, a varlen reference forward call can fail before any kernel comparison:Minimal code to reproduce it, from
tests:Calling
tests/ref_gdr.py::chunk_gated_delta_rule_fwd(..., cu_seqlens=cu_seqlens)hits the tuple passed into pack.Verification
After this patch, the same varlen reference forward call completes and returns the expected reference tensors.
Note: I’m still getting familiar with this codebase, so I’d be very happy to adjust the fix if you prefer a different style. The change is intentionally small and follows the existing production caller pattern of destructuring
prepare_chunk_offsets. I'd also like to say that I love the work you did here! Trying to learn from it 😄