Update modeling_mamba2.py, fix pad size
Author: klae01
Pull request: yes
Labels: name: run-slow

# What does this PR do?

The update involves a change in how `pad_size` is calculated. Originally, `pad_size` was computed as `self.chunk_size - (seq_len % self.chunk_size)`. The new formula ensures that `pad_size` will be the remainder of the subtraction `self.chunk_size - seq_len % self.chunk_size` and then a modulus with `self.chunk_size`. This change is intended to fix an issue where unnecessary padding was added when `seq_len % self.chunk_size == 0`. This behavior optimizes the padding process and reduces computational overhead.


## Before submitting
- [x] 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
- vision models: @amyeroberts
- speech models: @sanchit-gandhi
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- pipelines: @Narsil
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @muellerzr and @SunMarc

Integrations:

- deepspeed: HF Trainer/Accelerate: @muellerzr
- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization (bitsandbytes, autogpt): @SunMarc

Documentation: @stevhliu

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->


Comments:
@amyeroberts Original author who created this in mamba2 (for transformers). Yup, it's to reduce the padding size to 0 if we have `seq_len % chunk_size == 0`. The way it is now, we have an unnecessary padding of size `chunk_size` so fixing this definitely saves compute :smile:. 

It was originally meant to pad `seq_len` to a multiple of `chunk_size` (due to the algorithm working in a fixed chunk len fashion). So `pad_size` is the remaining length to get to a multiple of it.

LGTM cc @molbap for Mamba2 

@vasqu OK great, thanks for confirming and clarifying! 

@klae01 Could you: 
* Update the PR description to reflect this
* Rebase on main to include upstream changes? This should resolve the failing tests 

We should wait for rebase until #32694 is merged or something similar. There's currently a bug in cached generation with input_embeds.

Hi @vasqu, @amyeroberts,

Thank you for the feedback and guidance. I noticed that the force push I made may have been a bit premature, especially considering the advice to wait until PR #32694 (or a similar PR) is merged to avoid potential issues with cached generation.

To align with the suggestion, I’ll hold off on any further rebasing or force pushes until PR #32694 is merged. Once that’s done, I’ll rebase my branch on the updated `main` and ensure everything is in order.

Please let me know if there’s anything else I should do in the meantime. I appreciate your time and help with reviewing this PR!

Thanks!

@klae01 Thanks for bearing with me. You can only wait for now I guess but I'll ping you again when the other PR is merged and then this should be a quick rebase and go :eyes: 

@klae01 The patch has been merged :smile: thanks for waiting!

@amyeroberts I've rebased my branch now that PR #32694 is merged and force-pushed the updates. Everything should be up-to-date. Let me know if anything else is needed!

@klae01 Could you push an empty commit with the message `[run_slow] mamba2`. That should trigger the CI.

cc @amyeroberts for slow run

Ah and maybe rebase before, we removed a flag in #32686

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_32599). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

> Change looks good to me - thanks!
> 
> Just looking into how we can enable the tests cc @ydshieh

Hi, I am lacking a bit of the context. Do you still wait my inputs ..? If so could you elaborate a bit more the situation to resolve?

@ydshieh Apologies for not providing context! This was one of the related issues with tests failing because of access to a gated model. I'll trigger a re-run of the slow tests, as I think this was resolved (by you - thank you!) yesterday

OK! I also sent you a Slack DM regarding CI bot :-)

Any updates on this PR? 

gentle ping @amyeroberts 

@vasqu Apologies for the delay. The runner should now have access to run the mistral tests. I re-triggered the tests, but it seems they still failed, I'm guessing because retriggering uses the same settings i.e. tokens. @vasqu Could you push another `[run-slow]` commit to trigger these? IT should all pass and then we can merge 

No worries, I'm not the author so can't commit.

@klae01 Could you push an empty commit again, i.e. `[run-slow] mamba2`. This time it should clear the CI 🤞 

Hello, @amyeroberts 

I've added the `[run-slow] mamba2` commit as requested, and I see that all CI tests have passed. Please let me know if there's anything else I should address.

Thank you!

Hello @amyeroberts,

I've added the `[run-slow] mamba2` commit to re-trigger the tests, but it seems the CI tests are failing due to permission issues. Checking the logs, the tests are failing because they cannot access the gated repository:

```text
Cannot access gated repo for url https://huggingface.co/mistralai/Mamba-Codestral-7B-v0.1/resolve/refs%2Fpr%2F9/config.json.
Access to model mistralai/Mamba-Codestral-7B-v0.1 is restricted. You must have access to it and be authenticated to access it. Please log in.
```

Could you please advise on how to resolve this issue? Thank you!

@klae01 Could you add the `@require_read_token` decorator on the tests which are failing? This should properly propogate the required access token 

@klae01 It's a bit annoying, but anything after `[run-slow]` will be interpreted as a model name (so in this case it's looking for the models "add" and "@require_read_token"

Could you push an empty commit with `[run_slow] mamba2`? 

Hello, @amyeroberts.

I’ve just pushed a commit that rebases my branch with the latest changes from the main branch. I expect that these updates should properly resolve the issue with mamba2.

By the way, could you let me know how long the run-slow tests typically take to execute? Also, where can I find documentation on commit conventions (or guidelines) for transformers?

Hi @klae01 

We don't have a public doc for that yet. We are trying to add a comment like below

---



Before merging this pull request, slow tests CI should be triggered. To enable this:
- Add the `run-slow` label to the PR
- When your PR is ready for merge and all reviewers' comments have been addressed, push an empty commit with the command `[run-slow]` followed by a comma separated list of all the models to be tested, i.e. `[run_slow] model_to_test_1, model_to_test_2`
- A `transformers` maintainer will then approve the workflow to start the tests


Let's ignore the `multi-gpu` job (it's an infra issue)

@amyeroberts 

> It's a bit annoying, but anything after [run-slow] will be interpreted as a model name

I can change to make it less annoying, like  `[run-slow] ....  models=[model_1, model_2] ....` if that is more flexible 

> I can change to make it less annoying, like [run-slow] ....  models=[model_1, model_2] .... if that is more flexible

@ydshieh I think it's hard to find a good solution tbh -- most of the time the commit to trigger the tests are empty as they're requested at the end of PR review, so just `[run-slow] model_1, model_2, ...` etc. is easier. Happy to go for `[run-slow] ....  models=[model_1, model_2]` though if you think it's better as it's more explicit 

@klae01 On top of what @ydshieh has said above, regarding your question: `By the way, could you let me know how long the run-slow tests typically take to execute?` the answer is it depends on a few things:
* When they're actually triggered. A HF employee has to approve the workflow, so there's going to be time between your commit and when it "starts". It's done this way for security reasons
* Availability of compute - we sometimes have to wait for runners to become available 
* The tests themselves. Some models have lots of slow tests, or the model being tested is very large, both of which will increase the completion time. 

You can see in the [last single-gpu run for mamba2](https://github.com/huggingface/transformers/actions/runs/10951481120/job/30422919215?pr=32599) that running the tests themselves took about 26s, but initializing the container for testing took 4m 27s.

All passing. Thanks for your patience and the fix @klae01!