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

[torchtitan] Fix test runner fused optim tests #384

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

wz337
Copy link
Contributor

@wz337 wz337 commented Jun 6, 2024

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 6, 2024
@wz337 wz337 requested a review from tianyu-l June 6, 2024 21:32
test_runner.py Outdated
Comment on lines 229 to 230
f"--optimizer.name Adam --optimizer.fused --job.dump_folder",
f"--optimizer.name AdamW --optimizer.fused --job.dump_folder",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's already specified by https://github.com/pytorch/torchtitan/blob/main/test_runner.py#L252
Can we just remove this arg locally, following other tests?

@wz337 wz337 force-pushed the fix_test_runner branch 2 times, most recently from 296f371 to 9d929af Compare June 6, 2024 21:42
Copy link
Contributor

@tianyu-l tianyu-l left a comment

Choose a reason for hiding this comment

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

looks good, thx for the fix!

@wz337 wz337 merged commit baa678c into pytorch:main Jun 6, 2024
5 checks passed
tianyu-l pushed a commit to tianyu-l/torchtitan_intern24 that referenced this pull request Aug 16, 2024
philippguevorguian pushed a commit to YerevaNN/YNNtitan that referenced this pull request Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants