Add support for limit_train_batches to megatron sampler classes

[trias702]

What does this PR do ?

Adds support for limit_train_batches (as used in PTL) to work correctly. Without this PR, use of limit_train_batches less than 1.0 or num_samples will not work correctly when doing multi-epoch training in downstream libraries such as Nemo-Aligner.

Collection: NLP

Changelog

• nemo/collections/nlp/data/language_modeling/megatron/data_samplers.py: alters constructor parameters for limit_train_batches and adds additional constructor logic to limit total_samples as necessary
• nemo/collections/nlp/data/language_modeling/megatron/megatron_batch_samplers.py: alters constructor parameters for limit_train_batches and adds additional constructor logic to limit total_samples as necessary

Usage

You can now pass an additional parameter, limit_train_batches to all 4 Sampler classes

GitHub Actions CI

The Jenkins CI system has been replaced by GitHub Actions self-hosted runners.

The GitHub Actions CI will run automatically when the "Run CICD" label is added to the PR.
To re-run CI remove and add the label again.
To run CI on an untrusted fork, a NeMo user with write access must first click "Approve and run".

Before your PR is "Ready for review"

Pre checks:

• Make sure you read and followed Contributor guidelines
• Did you write any new necessary tests?
• Did you add or update any necessary documentation?
• Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
  • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

• New Feature
• Bugfix
• Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

• Related to # (issue)

[github-actions]

[🤖]: Hi @trias702 👋,

I just wanted to let you know that, you know, a CICD pipeline for this PR just finished successfully ✨

So it might be time to merge this PR or like to get some approvals 🚀

But I'm just a 🤖 so I'll leave it you what to do next.

Have a great day!

//cc @ko3n1g

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

[trias702]

jenkins

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

[trias702]

jenkins

…

On Mon, Oct 28, 2024, 19:00 github-actions[bot] ***@***.***> wrote: This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days. — Reply to this email directly, view it on GitHub <#10648 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGFLGND56EFTP65AIVAI4PTZ53T3HAVCNFSM6AAAAABO6AYA72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBTGAYDONZZHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

[trias702]

Jenkins

…

On Fri, Nov 15, 2024, 18:01 github-actions[bot] ***@***.***> wrote: This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days. — Reply to this email directly, view it on GitHub <#10648 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGFLGNBIXM2W3FQ2QGNSDCD2A2RO3AVCNFSM6AAAAABO6AYA72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBQGMYDQNZYGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[odelalleau]

At first sight it seems to me that this parameter doesn't belong to the sampler, and it should be up to the caller to provide the correct value for total_samples. Why do you think we need it in the sampler?

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

[trias702]

At first sight it seems to me that this parameter doesn't belong to the sampler, and it should be up to the caller to provide the correct value for total_samples. Why do you think we need it in the sampler?

We spoke about this before in a meeting, and I told you we can either hack it in via the algorithm class, or put it inside the Sampler, and you said inside the Sampler was the correct place for it.

[odelalleau]

At first sight it seems to me that this parameter doesn't belong to the sampler, and it should be up to the caller to provide the correct value for total_samples. Why do you think we need it in the sampler?

We spoke about this before in a meeting, and I told you we can either hack it in via the algorithm class, or put it inside the Sampler, and you said inside the Sampler was the correct place for it.

Ah, my memory is failing me, though as discussed in NVIDIA/NeMo-Aligner#321 (comment) there's a way to do it without it being a hack -- I assume we hadn't thought of this option when we spoke about it (really can't remember, sorry).

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

[github-actions]

This PR was closed because it has been inactive for 7 days since being marked as stale.