Skip to content
This repository was archived by the owner on Aug 1, 2025. It is now read-only.

Conversation

mlazos
Copy link
Contributor

@mlazos mlazos commented Jul 14, 2022

This functionality is needed to trace through optimizers.

Summary of changes

  • Update dictionaries to store weakrefs to parameter keys in f_globals
  • Update DICT_KEY guard to check that parameter id's match

@anijain2305
Copy link
Contributor

Overall, looks good to me. Once you have optimizers working, would you be interested in adding optimizers in benchmarks/common.py so that we can test torchbench and huggingface models with optimizers?

@mlazos mlazos self-assigned this Jul 18, 2022
@mlazos mlazos requested a review from jansel July 20, 2022 01:28
@mlazos
Copy link
Contributor Author

mlazos commented Jul 20, 2022

Overall, looks good to me. Once you have optimizers working, would you be interested in adding optimizers in benchmarks/common.py so that we can test torchbench and huggingface models with optimizers?

Let me see if there are other higher priority follow ups that are needed from the distributed team but I think this would be a good continuation of this.

@mlazos mlazos merged commit b2e85a8 into main Jul 20, 2022
@desertfire
Copy link
Contributor

@mlazos , this PR triggers an assertion in several torchbench tests, e.g. ./benchmarks/torchbench.py -d cuda --backend eager -k hf_Albert. If it is not an easy fix, we should revert this first.

@mlazos
Copy link
Contributor Author

mlazos commented Jul 20, 2022

@mlazos , this PR triggers an assertion in several torchbench tests, e.g. ./benchmarks/torchbench.py -d cuda --backend eager -k hf_Albert. If it is not an easy fix, we should revert this first.

I'll take a look at the asserts and and either have a followup fix or revert if necessary

desertfire added a commit that referenced this pull request Jul 20, 2022
Summary: In #579, the
torchbench CI run hits an ssertion but didn't report failure on that. It
will be helpful to raise AssertionError at this active development
stage.
desertfire added a commit that referenced this pull request Jul 20, 2022
Summary: In #579, the
torchbench CI run hits an ssertion but didn't report failure on that. It
will be helpful to raise AssertionError at this active development
stage.
desertfire added a commit that referenced this pull request Jul 21, 2022
Summary: In #579, the
torchbench CI run hits an ssertion but didn't report failure on that. It
will be helpful to raise AssertionError at this active development
stage.
desertfire added a commit that referenced this pull request Jul 21, 2022
Summary: In #579, the
torchbench CI run hits an ssertion but didn't report failure on that. It
will be helpful to raise AssertionError at this active development
stage.
@mlazos mlazos deleted the mlazos/param-keys branch July 22, 2022 05:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use TorchDynamo to capture optimizers

5 participants