Support scalar parameters in scale_by_sm3 - #1736
Open
TheSaiEaranti wants to merge 1 commit into
Open
Conversation
scale_by_sm3's init built the per-axis accumulator list with range(p.ndim), which is empty for a 0-d leaf, so the first update raised IndexError from v[0] deep inside _new_accum. Any parameter pytree containing a scalar leaf, such as a temperature or a scalar gain, made optax.sm3 unusable while every other transformation accepts the same pytree. Treat a 0-d leaf as having one virtual axis: build one accumulator for it, make _expanded_shape return an empty shape for rank 0 via slicing, and iterate max(g.ndim, 1) axes in the update. A scalar leaf now behaves exactly like the same value as a one-element vector, the algorithm's natural degenerate case, which the regression test asserts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
optax.sm3crashes withIndexError: list index out of rangeon the firstupdate()whenever the parameter pytree contains a scalar (0-d) leaf — a temperature, a scalar gain — while every other transformation in the library (adam,adagrad,rmsprop, ...) accepts the identical pytree:initbuilds the per-axis accumulator list withrange(p.ndim)— the empty list for a 0-d leaf — and the first update then indexesv[0]inside_new_accum. Neither the docstring nor an error message surfaces any shape restriction;initsucceeding andupdatethrowing an unrelatedIndexErroris the worst version of the failure.Fix
Treat a 0-d leaf as having one virtual axis, the algorithm's natural degenerate case: one accumulator covering the whole (scalar) parameter. Three
range(max(ndim, 1))sites plus making_expanded_shaperank-0-safe via slicing. Behavior for all existing shapes is unchanged (shape[axis:axis+1]equals[shape[axis]]for every rank ≥ 1).The regression test asserts the crash is gone and the semantics are right: a scalar leaf's update equals the same value's update as a one-element vector.
Tests
test_scale_by_sm3_scalar_paramsfails onmainwith theIndexErrorand passes with the fix. Fulltransform_test.py+alias_test.py: 560 passed, 643 subtests.ruffclean.