add sequential sampling
Author: jiosephlee
Pull request: yes

# What does this PR do?

Adds Sequential Sampling! All huggingface trainers default to random sampling, and there is no way to allow a sequential sampling, which is valuable when the user has a custom data curriculum, for instance.


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker @Cyrilvallez
- vision models: @yonigozlan @molbap
- audio models: @eustlb @ebezzam @vasqu
- multimodal models: @zucchini-nlp
- graph models: @clefourrier

Library:

- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- continuous batching: @remi-or @ArthurZucker @McPatate
- pipelines: @Rocketknight1
- tokenizers: @ArthurZucker and @itazap
- trainer: @SunMarc
- attention: @vasqu @ArthurZucker @CyrilVallez
- model loading (from pretrained, etc): @CyrilVallez
- distributed: @3outeille @ArthurZucker
- CIs: @ydshieh

Integrations:

- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization: @SunMarc @MekkCyber
- kernels: @MekkCyber @drbh
- peft: @BenjaminBossan @githubnemo

Devices/Backends:

- AMD ROCm: @ivarflakstad
- Intel XPU: @IlyasMoutawwakil
- Ascend NPU: @ivarflakstad 

Documentation: @stevhliu

Research projects are not maintained and should be taken as is.

 -->


Comments:
cc @sunmarc

Hey @jiosephlee, I don't mind adding this but did you manage to do an experiment where this leads to better training overall ? 

Hey @SunMarc ! Sequential sampling in and of itself wouldn't improve training, but yes, I'm doing research currently where a well-designed curriculum can lead to better performance. There are also prior works that support this e.g. https://arxiv.org/abs/2405.07490. A common way of organizing data in this field is from easy to hard: https://dl.acm.org/doi/abs/10.1145/3589335.3641257


Maybe we can first gauge the interest from the community before merging this ? TrainingArgs is already super bloated so I prefer to not add any args that have low usage. Could you create an issue so that we see if folks are interested in ? 

@SunMarc, that totally makes sense. I just created an issue, and I can re-bump this PR once it's garnered some interest. 

I know folks like  @qgallouedec have wanted this as well, if this helps clarify the need for this feature (correct me if I'm wrong, Quentin!)

Thznks!

Yes, to give more context:

Currently, if I want to sample my data in order, I can't. The PR in the current state is a solution, but in my opinion, sequential sampling should be the default.

If we agree on this, I see two options

1. Add a parameter such as `random_sampling` (`False` by default) that would enable random sampling. But that would add another argument to `TrainingArguments`.

2. (Better, in my opinion) Simply replace the random sampler with a sequential sampler, and if the user wants random sampling, they can simply shuffle their dataset beforehand, which amounts to the same thing. No new arg.

In both cases it would change the default behavior of the trainer 

@qgallouedec I will +1 on the default behavior change. The default behavior is likely unexpected for many, especially for those coming from PyTorch, which also has the shuffle off as default https://docs.pytorch.org/docs/stable/data.html#torch.utils.data.DataLoader

Concerning (2), there is an issue that would arise from forgoing RandomSampler. RandomSampler has a nice property where you can reset the shuffle at each epoch, and it appears to be used in Trainer.

https://github.com/huggingface/transformers/blob/d6a6c82680cba9c51decdacac6dd6315ea4a766a/src/transformers/trainer.py#L2465

Otherwise, it seems difficult to implement epoch-level shuffling outside of Trainer.





> Currently, if I want to sample my data in order, I can't. The PR in the current state is a solution, but in my opinion, sequential sampling should be the default.

Can you explain why ? For the training dataset, it should be better to have have random sampling in general no ? 

Maybe as a first set, we can create a new arg called `train_sampler` and we remove that the same time `group_by_length` arg. In this new arg, we will have three choices `random` (default), `group_by_length` and `sequential` wdyt ? Since v5 is not out yet, if we are quick enough, we won't have to do dep cycle regarding `group_by_length` arg. cc @jiosephlee 

Happy new year btw ;) 

@SunMarc happy new year :) 

And that's a pretty clean suggestion! I took a stab at refactoring it based on what you said. I took the liberty of changing one of the ValueErrors to a warning because the IterableDataset incompatibility seems to be an issue for any Sampler, afaik, which is why samplers are entirely skipped in line 1037. 



Yes, that sounds good to me too. With one small reservation about the name. Because this argument would not take a sampler, but rather a strategy. So maybe `sampling_strategy`?

> Can you explain why ? For the training dataset, it should be better to have random sampling in general no ?

Yes indeed. The motivation was mainly to avoid the surprise caused by the fact that the PyTorch loader is sequential by default, but shuffling by default is also a good default.

Moved this to new PR #43138