Skip to content

Fix Glorot/He uniform init docstrings to label the uniform bound, not sigma#3831

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

Fix Glorot/He uniform init docstrings to label the uniform bound, not sigma#3831
vineethsaivs wants to merge 1 commit into
ml-explore:mainfrom
vineethsaivs:fix/uniform-init-docstring-bound

Conversation

@vineethsaivs

Copy link
Copy Markdown

Proposed changes

The glorot_uniform and he_uniform docstrings in python/mlx/nn/init.py
write the sampling bound with the symbol \sigma, but both initializers draw
from a symmetric uniform distribution, uniform(-a, a), so the value shown is
the bound a, not a standard deviation. \sigma conventionally denotes the
standard deviation, and for uniform(-a, a) the standard deviation is
a / sqrt(3):

  • glorot_uniform: bound a = gain * sqrt(6 / (fan_in + fan_out)), so the
    actual standard deviation is gain * sqrt(2 / (fan_in + fan_out)), which is
    exactly the \sigma documented for glorot_normal.
  • he_uniform: bound a = gain * sqrt(3 / fan), so the actual standard
    deviation is gain / sqrt(fan), which is exactly the \sigma documented for
    he_normal.

So the documented \sigma for the uniform initializers is off by a factor of
sqrt(3) from the real standard deviation. It is also inconsistent with the
surrounding prose, which already describes the quantity as a "range", and with
the normal initializers in the same file, where \sigma correctly denotes the
standard deviation. For comparison, PyTorch's kaiming_uniform_ docstring
labels the same quantity bound and samples U(-bound, bound).

This relabels the quantity as the bound a and shows the sampling interval
[-a, a]. It is a documentation-only change with no effect on behavior; the
sampled values are unchanged.

Checklist

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes (docstring text only; verified it compiles and does not change black formatting)
  • I have added tests that prove my fix is effective or that my feature works (documentation-only change; no code path is affected)
  • I have updated the necessary documentation (if needed)

@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.

Confirmed correct against the implementation. Both glorot_uniform and he_uniform compute limit = gain * math.sqrt(6.0 / (fan_in + fan_out)) (resp. sqrt(3.0 / fan)) and then mx.random.uniform(-limit, limit, ...) — so that quantity is the half-width of the uniform interval, not a standard deviation. Labeling it \sigma overstates the spread by a factor of √3 (for U[-a, a] the actual std is a/√3), so the relabel is right. The explicit [-a, a] interval line you added makes the meaning unambiguous, which is the better framing.

Worth noting the normal siblings (glorot_normal / he_normal) correctly keep \sigma alongside mx.random.normal(scale=std) — so this reads like the \sigma label was carried over from the normal variants into the uniform ones by mistake.

One small thought: the implementation names this variable limit, so using \text{limit} as the symbol (as your #3835 does) ties the formula straight to the source. a with the explicit interval is also perfectly clear and arguably more conventional — either works.

Heads up that this overlaps with your own #3835 — same two functions, same lines — so they'll conflict and one probably wants closing. This is the more complete of the two (the interval prose is the useful part), so if you keep one, I'd keep this.

…igma

The glorot_uniform and he_uniform docstrings wrote the sampling bound as
sigma (the standard-deviation symbol), but the code samples from
uniform(-a, a) where a is that bound. The actual standard deviation of the
result is a / sqrt(3), so labeling the bound sigma is off by sqrt(3) and is
inconsistent with the prose (which calls it a range) and with the normal
initializers, where sigma correctly denotes the standard deviation. Relabel
the quantity as the bound a and show the sampling interval [-a, a].
@vineethsaivs vineethsaivs force-pushed the fix/uniform-init-docstring-bound branch from d0ffbbc to 769e1d8 Compare July 13, 2026 17:10
@vineethsaivs

Copy link
Copy Markdown
Author

Thanks for the careful review. Agreed the \sigma label looks carried over from the normal siblings. I have consolidated the two PRs into this one: it keeps the explicit [-limit, limit] interval wording and now uses \text{limit} as the symbol so the formula matches the code's variable name, as you suggested. Closing #3835 in favor of this PR.

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