Patch EOD out of inference results#3866
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
8ed5d71 to
ec4ba8a
Compare
|
/claude review |
megatron/core/inference/text_generation_controllers/text_generation_controller.py
Outdated
Show resolved
Hide resolved
ec4ba8a to
fb160de
Compare
santhnm2
left a comment
There was a problem hiding this comment.
LGTM but just 1 question - how does this work for stop sequences as opposed to single stop tokens? Does any additional functionality need to be added for these cases?
Beyond a doubt, additional functionality needs to be added for these cases. However, there's two reasons I'm not including that in this PR:
|
The bug in stop words processing seems concerning, @shanmugamr1992 do you have more details here? But otherwise maybe for now we can just add a TODO to this PR to handle stop words eventually. |
d5ce157 to
7ae15f7
Compare
santhnm2
left a comment
There was a problem hiding this comment.
LGTM but can we add a unit test for the stop sequence case also?
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/23262285801 |
What does this PR do ?
Contribution process
Pre-checks
Code review
Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!
All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.
Step 1: Mark PR as "Ready for Review"
.github/CODEOWNERS.Final Review might get declined if these requirements are not fulfilled.
Step 2: Final Review
For PRs that change
megatron/core, once all expert reviewers have approved, theFinal Reviewlabel is applied automatically and final reviewers are assigned.For PRs outside
megatron/core, this step is skipped.Step 3: Approved
Once all required reviewers have approved, the
Approvedlabel is applied automatically.Merge
Any member of mcore-engineers will be able to merge your PR.
For MRs into `dev` branch
The proposed review process for `dev` branch is under active discussion.MRs are mergable after one approval by either
eharper@nvidia.comorzijiey@nvidia.com.