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

[INF] Enable torch compile for inference #5612

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

oelayan7
Copy link
Contributor

@oelayan7 oelayan7 commented Jun 4, 2024

No description provided.

@delock
Copy link
Contributor

delock commented Jun 5, 2024

Hi @oelayan7 do you have sample code using this feature? Or does it turn on by default? I wonder if I can try it with CPU accelerator.

@oelayan7
Copy link
Contributor Author

oelayan7 commented Jun 9, 2024

Hi @delock,
No, it isn't run by default. The function .compile() needs to be called.
Torch compile was introduced in ZeRO and for training, which used CompileWrapper that was removed in #5581 and a new approach was introduced in this PR.
This change just adds this approach to inference as it was done in the other PRs.

@tjruwase tjruwase requested review from tohtana and umchand and removed request for arashb, awan-10 and mrwyattii June 9, 2024 22:45
@oelayan7
Copy link
Contributor Author

@tjruwase the failing tests are because an HTTP failure with HF.
Can you please retrigger? (couldn't find a way to do that)

@oelayan7
Copy link
Contributor Author

@tjruwase @tohtana @umchand Can anyone trigger the CI please?

Copy link
Contributor

@tohtana tohtana 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 @oelayan7 @delock !

I just found that nvtx decorator breaks the graph and has an impact on performance. Can you add deepspeed.utils.nvtx.enable_nvtx = False to compile() as in #569?

Overall this looks good to me. Let's merge after you add it.

Copy link
Contributor

@tohtana tohtana 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 @oelayan7 !

@tohtana tohtana enabled auto-merge June 25, 2024 08:37
@loadams loadams disabled auto-merge June 26, 2024 18:04
@oelayan7
Copy link
Contributor Author

@loadams Thanks!
I fixed the formatting issue, can you please retrigger?

@loadams loadams enabled auto-merge June 28, 2024 22:01
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.

None yet

6 participants