Skip to content

Fix uniform-initializer docstrings to use the range limit, not sigma#3835

Closed
vineethsaivs wants to merge 1 commit into
ml-explore:mainfrom
vineethsaivs:fix/init-uniform-limit-docstring
Closed

Fix uniform-initializer docstrings to use the range limit, not sigma#3835
vineethsaivs wants to merge 1 commit into
ml-explore:mainfrom
vineethsaivs:fix/init-uniform-limit-docstring

Conversation

@vineethsaivs

Copy link
Copy Markdown

Proposed changes

nn.init.glorot_uniform and nn.init.he_uniform sample from a uniform distribution on [-limit, limit]: the code computes limit = gain * math.sqrt(...) and calls mx.random.uniform(-limit, limit, ...). Their docstring math, however, labels that bound as \sigma, the standard-deviation symbol:

\sigma = \gamma \sqrt{\frac{6.0}{\text{fan\_in} + \text{fan\_out}}}   # glorot_uniform
\sigma = \gamma \sqrt{\frac{3.0}{\text{fan}}}                        # he_uniform

The standard deviation of U(-limit, limit) is limit / sqrt(3), not limit, so using \sigma for the bound is misleading. This renames the symbol to \text{limit} in both docstrings so the documented quantity matches the code variable and its true meaning.

glorot_normal and he_normal are left unchanged: they sample from a normal distribution, where \sigma is the correct symbol for the standard deviation.

Documentation-only change; no code paths are affected.

Checklist

  • I have read the CONTRIBUTING document
  • The change is docstring text only (inside a .. math:: block), so black formatting is unchanged
  • I have added tests (not applicable: documentation-only change, no behavior changed)
  • I have updated the necessary documentation

glorot_uniform and he_uniform sample from a uniform distribution on
[-limit, limit], but their docstring math labeled that bound as sigma,
the standard-deviation symbol. The standard deviation of U(-limit, limit)
is limit / sqrt(3), so the symbol was misleading. The code already names
the value 'limit'; use the same name in the math so the docs match the
implementation. glorot_normal and he_normal keep sigma, which is correct
for a normal distribution.

@Pablosinyores Pablosinyores left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The relabel is correct. glorot_uniform / he_uniform sample mx.random.uniform(-limit, limit, ...) where limit = gain * math.sqrt(6.0 / (fan_in + fan_out)) (resp. sqrt(3.0 / fan)), so that value is the uniform bound, not a standard deviation — \sigma was wrong here (for U[-a, a], σ is a/√3). Using \text{limit} is a nice touch since it matches the code's actual variable name.

This overlaps with your own #3831, which makes the same \sigma-is-wrong fix and additionally rewrites the prose to state the [-a, a] interval explicitly. Since both edit the same glorot/he docstring lines they'll conflict, so it's worth consolidating into one — I'd lean toward #3831 for the extra interval clarity, ideally keeping this PR's \text{limit} symbol. Flagging so one doesn't end up rotting behind the other.

@vineethsaivs

Copy link
Copy Markdown
Author

Closing in favor of #3831, which makes the same relabel with clearer wording: it states the [-limit, limit] interval explicitly. I have folded this PR's \text{limit} symbol into #3831 so nothing here is lost. Thanks for flagging the overlap.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants