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

Run only a subset for optim benchmarks, attempt to fix #2392

Closed
wants to merge 4 commits into from

Conversation

janeyx99
Copy link
Contributor

@janeyx99 janeyx99 commented Jul 23, 2024

A first attempt to fix https://github.com/pytorch/benchmark/actions/workflows/userbenchmark-regression-detector.yml by only running a subset of models

Impact of this change:
This workflow is green: https://github.com/pytorch/benchmark/actions/runs/10066723358/job/27828703722

It also takes so much shorter. At one point, the benchmark was 7 hours! Now it's 7 minutes.

Test Plan

Test run: https://github.com/pytorch/benchmark/actions/runs/10065401507

Final test run: https://github.com/pytorch/benchmark/actions/runs/10066723358/job/27828703722

@@ -579,8 +592,6 @@ def run_model(modelName, device, Optim, defaults, maybe_pt2_):
len(params),
params[0].device,
)
if Optim.__name__ == "SGD":
defaults["lr"] = 1e-2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no longer necessary as we have a default LR for SGD now

@janeyx99 janeyx99 temporarily deployed to docker-s3-upload July 23, 2024 19:47 — with GitHub Actions Inactive
@facebook-github-bot
Copy link
Contributor

@janeyx99 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@janeyx99 janeyx99 temporarily deployed to docker-s3-upload July 23, 2024 21:34 — with GitHub Actions Inactive
@janeyx99 janeyx99 temporarily deployed to docker-s3-upload July 23, 2024 21:35 — with GitHub Actions Inactive
@janeyx99 janeyx99 temporarily deployed to docker-s3-upload July 23, 2024 21:35 — with GitHub Actions Inactive
@facebook-github-bot
Copy link
Contributor

@janeyx99 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@janeyx99 janeyx99 requested a review from xuzhao9 July 23, 2024 21:36
@@ -412,7 +412,7 @@ def get_metrics_by_date(
if not start_date:
raise RuntimeError(
f"No start date in previous JSONS found to compare towards the end date {end_date}. User specified start date: {args.start_date}. "
+ f"Available JSON dates: {available_metrics_jsons.keys()}. No regression info has been generated."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

available_metrics_jsons doesn't have keys anymore as it's a List[str] and not a dict. It's unclear whether there were ever keys. Just remove the printout for now as it's not needed for the error message.

@facebook-github-bot
Copy link
Contributor

@janeyx99 merged this pull request in 9ad6639.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants