Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple prepare_data_loader() from Accelerator #3047

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

siddk
Copy link
Contributor

@siddk siddk commented Aug 26, 2024

What does this PR do?

Following #3039, decouples prepare_data_loader() (and all DataLoader classes/mixins in data_loader.py) from relying on AcceleratorState (and by extension, a properly initialized Accelerator), instead relying on PartialState. Fixes and extends tests in test_data_loader.py to validate both the non-Accelerator and decoupled workflows.

The benefit of this is that now folks can use prepare_data_loader and the nice features it provides (put_on_device, split_batches) without restructuring the rest of their training code around Accelerator.

Before submitting

Who can review?

@muellerzr @BenjaminBossan @SunMarc

@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.

Copy link
Collaborator

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

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

Very nicely done. I ran the tests locally and on two GPUs we do in fact pass! Nice work :)

@muellerzr muellerzr requested a review from SunMarc August 26, 2024 14:18
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 work @siddk !

@muellerzr muellerzr merged commit 2789933 into huggingface:main Aug 26, 2024
25 checks passed
@siddk siddk deleted the decouple-dataloader branch August 26, 2024 14:45
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.

4 participants