Skip to content

Commit

Permalink
Update base for Update on "[BE][SparseAdam] cleaner way to verify no …
Browse files Browse the repository at this point in the history
…sparse params"

Context:

#47724 fixed the problem that SparseAdam could not handle generators by using the `list(...)` construct. However, this meant that SparseAdam deviated from other optimizers in that it could _accept_ a raw Tensors/Parameter vs requiring a container of them. This is not really a big deal.

So why this PR?

I do think this PR is cleaner. It uses the fact that the Optimizer parent class already containerizes parameters into parameter groups, so we could reuse that here by calling `super().__init__` first and then filter the param_groups after. This change would also make SparseAdam consistent with the rest of our optimizers in that only containerized params are accepted, which technically is BC breaking but would be minor, I believe.




[ghstack-poisoned]
  • Loading branch information
janeyx99 committed Nov 28, 2023
1 parent 5622adb commit 24cd0a0
Showing 0 changed files with 0 additions and 0 deletions.

0 comments on commit 24cd0a0

Please sign in to comment.