Fix get_label_mask docstrings and get_assistant_mask return type#85
Open
lonexreb wants to merge 1 commit intoNVlabs:mainfrom
Open
Fix get_label_mask docstrings and get_assistant_mask return type#85lonexreb wants to merge 1 commit intoNVlabs:mainfrom
lonexreb wants to merge 1 commit intoNVlabs:mainfrom
Conversation
Three small docstring/typing nits in src/alpamayo_r1/utils/get_label_mask.py:
1. fill_masks_between_special_tokens: the labels_mask docstring entry
was truncated mid-sentence ("...indicating the positions that should").
Complete the sentence and document that the tensor is mutated
in-place and that the matched span includes the end token (matching
the existing `# NOTE: we should include the end token in the mask`
comment in the body).
2. get_assistant_mask: the function returns either a torch.Tensor or a
list[bool] depending on the input type (line `return masks.tolist()`
when tokens is a list[int]), but the annotation declared only
torch.Tensor. Update the annotation to torch.Tensor | list[bool] and
reflect this in the docstring.
3. get_assistant_mask: drop the dangling "Reference: Adapted from:"
block that has no content. If a future reference is identified it
can be added back in a separate commit.
Pure docstring/annotation hygiene -- no behavioral change.
Signed-off-by: lonexreb <[email protected]>
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.
Problem
Three small accuracy issues in
src/alpamayo_r1/utils/get_label_mask.py:Truncated docstring in
fill_masks_between_special_tokens— the entry forlabels_maskcuts off mid-sentence:Readers can't tell what the function actually does to the mask, and the in-place mutation isn't documented.
Wrong return-type annotation on
get_assistant_mask. The function declares-> torch.Tensor, but the body returnsmasks.tolist()(alist[bool]) whentokensis alist[int]:This trips up type-checkers and is misleading to callers passing a list.
Dangling reference block in
get_assistant_mask— the docstring ends with:No URL or citation follows. Either fill it in or drop it; this PR drops it.
Fix
Pure docstring + annotation hygiene. No behavioral change.
labels_maskdescription and note in-place mutation + inclusive end-token semantics (matches the# NOTE: we should include the end token in the maskcomment that already exists in the body).get_assistant_maskannotation totorch.Tensor | list[bool]and reflect this in the docstring.Reference: Adapted from:block.Verification
No code paths changed. Mypy / pyright now agree with the actual runtime types.