Fix for RL Test - #3148
Conversation
|
/ok to test 8f77f6c |
jon-barker
left a comment
There was a problem hiding this comment.
this is great - thanks!
|
/ok to test 27254b4 |
ko3n1g
left a comment
There was a problem hiding this comment.
One comment regarding the model config
| - "mem-allocated-bytes" | ||
| - "mem-max-allocated-bytes" | ||
|
|
||
| THROUGHPUT_TEST_PARAMS: |
There was a problem hiding this comment.
Please don’t introduce new top-level attributes. If you need to feed metadata into your job, please use the pre-existing ENV mapping. It will export all of its key value pairs to the shell session
There was a problem hiding this comment.
@jon-barker I think I got this from you I think. Do you think we need this?
There was a problem hiding this comment.
Yes, the script that checks the metrics uses these to determine which metrics to compare and which iteration steps to use
There was a problem hiding this comment.
I'm happy for them to be moved into ENV_VARS @ko3n1g but I can't do it today and someone needs to check all the tests using this get updated. I can do it when i get back from vacation on 2/9
There was a problem hiding this comment.
Ill do it in this MR and make sure the tests pass on gitlab before review
|
/ok to test 02a5bc3 |
|
/ok to test 02a5bc3 |
1 similar comment
|
/ok to test 02a5bc3 |
Co-authored-by: Dmytro Pykhtar <37850217+dimapihtar@users.noreply.github.com>
Co-authored-by: Dmytro Pykhtar <37850217+dimapihtar@users.noreply.github.com>
Co-authored-by: Dmytro Pykhtar <37850217+dimapihtar@users.noreply.github.com>
What does this PR do ?
Bringing back one of the RL tests with some changes to help make tests more deterministic.
Contribution process
flowchart LR A[Pre-checks] --> B[PR Tests] subgraph Code Review/Approval C1[Expert Review] --> C2[Final Review] end B --> C1 C2 --> D[Merge]Pre-checks
Core 0.8)Code review
The following process is enforced via the CODEOWNERS file for changes into
megatron/core. For changes outside ofmegatron/core, it is up to the PR author whether or not to tag the Final Reviewer team.For MRs into `main` branch
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!
(Step 1): Add PR label
Expert Review(Step 2): Collect the expert reviewers reviews
Expert Reviewlabel when your PR is ready for review.Final Review might get declined if these requirements are not fulfilled.
(Step 3): Final Review
Final Reviewlabel(Optional Step 4): Cherry-pick into release branch
If this PR also needs to be merged into
core_r*release branches, after this PR has been merged, selectCherry-pickto open a new PR into the release branch.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.Merging your PR
Any member of core-adlr and
core-nemowill be able to merge your PR.