Skip to content

Conversation

@kmehant
Copy link
Contributor

@kmehant kmehant commented Aug 30, 2025

What does this PR do?

Allow mixed precision to be passed as a dtype string from accelerate cli flag or fsdp_config in accelerate config file.

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,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? 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, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@S1ro1 @SunMarc

Signed-off-by: Mehant Kammakomati <[email protected]>
Signed-off-by: Mehant Kammakomati <[email protected]>
Signed-off-by: Mehant Kammakomati <[email protected]>
@kmehant kmehant changed the title feat: allow mixed precision as dtype feat: allow mixed precision policy as dtype Aug 30, 2025
@kmehant
Copy link
Contributor Author

kmehant commented Sep 1, 2025

@S1ro1 WDYT on this?

@SunMarc SunMarc requested a review from S1ro1 September 1, 2025 09:15
@S1ro1
Copy link
Contributor

S1ro1 commented Sep 1, 2025

Hey @kmehant currently OOO, will give it a look next week!

@kmehant
Copy link
Contributor Author

kmehant commented Sep 1, 2025

Sure, @S1ro1 thanks!

@muellerzr
Copy link
Contributor

IMO: this simplifies quite a few things without over-bloating params too much. I think it's an easy win, however I'll leave this to @S1ro1 and @SunMarc and go back to the leave-grave for now :)

Copy link
Contributor

@S1ro1 S1ro1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can we maybe add tests? (should be simple enough to just adapt the current ones for MP to also test this)

Copy link
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, just add a test if possible

Signed-off-by: Mehant Kammakomati <[email protected]>
@kmehant
Copy link
Contributor Author

kmehant commented Sep 8, 2025

@S1ro1 @SunMarc
Screenshot 2025-09-09 at 1 09 43 AM

Extended the test case and passes!

I have removed the flag part of the PR since, I see mixed_precision accelerate argument is already taking care of it on setting the mixed precision for FSDP1/2.

Copy link
Contributor

@S1ro1 S1ro1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@S1ro1 S1ro1 merged commit a0bc36e into huggingface:main Sep 8, 2025
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants