Skip to content

OKLS optimizer - #265

Draft
mkhona-nvidia wants to merge 10 commits into
NVIDIA-NeMo:mainfrom
mkhona-nvidia:okls-optimizer
Draft

OKLS optimizer#265
mkhona-nvidia wants to merge 10 commits into
NVIDIA-NeMo:mainfrom
mkhona-nvidia:okls-optimizer

Conversation

@mkhona-nvidia

Copy link
Copy Markdown
Contributor

Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
Signed-off-by: mkhona <mkhona@nvidia.com>
@mkhona-nvidia
mkhona-nvidia requested a review from skyw July 28, 2026 23:57
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Comment on lines +152 to +153
if ridge_eps < 0.0:
raise ValueError(f"Invalid ridge epsilon: {ridge_eps}")

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.

P1 Zero epsilon produces NaN state

When ridge_eps=0 and the first gradient is all zero, initialization computes sqrt(rows / 0) and multiplies the zero covariance by infinity, causing NaN factors, inverse roots, and parameter values on the first step.

Suggested change
if ridge_eps < 0.0:
raise ValueError(f"Invalid ridge epsilon: {ridge_eps}")
if ridge_eps <= 0.0:
raise ValueError(f"Invalid ridge epsilon: {ridge_eps}")

Comment on lines +106 to +112
grad_right_preconditioned = grad @ inverse_root_right
factor_left.lerp_(grad_right_preconditioned @ grad_right_preconditioned.T / cols, 1 - shampoo_beta)
factor_left.copy_((factor_left + factor_left.T) / 2.0)
factor_left.diagonal().add_(ridge_eps)

grad_left_preconditioned = inverse_root_left @ grad
factor_right.lerp_(grad_left_preconditioned.T @ grad_left_preconditioned / rows, 1 - shampoo_beta)

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.

P2 Factor matmuls inherit global precision

The covariance and final preconditioning matmuls execute outside fp32_matmul_precision, so a process-wide medium setting silently reduces their precision and makes persistent optimizer factors depend on unrelated global configuration.

Knowledge Base Used: SOAP: Shampoo-style Preconditioning

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds the OKLS optimizer and its scaled-CANS inverse-root implementation.

  • Registers and publicly exports OKLS.
  • Adds API documentation for OKLS and the matrix inverse-root utility.
  • Adds optimizer, numerical utility, and registry tests.

Confidence Score: 3/5

The PR is not yet safe to merge because zero epsilon can corrupt optimizer state with NaNs and configured matmul precision is not applied throughout the OKLS update.

The current implementation still permits a zero epsilon that makes zero-gradient initialization evaluate zero times infinity, while factor and final preconditioning matmuls remain controlled by unrelated process-wide precision.

Files Needing Attention: emerging_optimizers/soap/okls.py

Important Files Changed

Filename Overview
emerging_optimizers/soap/okls.py Implements OKLS state initialization, factor updates, inverse-root refreshes, momentum, and parameter updates.
emerging_optimizers/soap/matrix_root_inverse_utils.py Implements the fixed-schedule scaled-CANS approximation for FP32 matrix inverse square roots.
tests/test_okls.py Adds CUDA optimizer smoke, state-initialization, and dimensionality-validation coverage.
tests/test_matrix_root_inverse_utils.py Adds shape, precision-restoration, accuracy, and dtype-validation coverage for inverse roots.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    G[Gradient] --> I[Initialize or update Kronecker factors]
    I --> C[Scaled CANS inverse roots]
    G --> M[Nesterov momentum]
    C --> P[Two-sided preconditioning]
    M --> P
    P --> U[Parameter update]
Loading

Reviews (2): Last reviewed commit: "fix: use decoupled weight decay in OKLS" | Re-trigger Greptile

@skyw
skyw marked this pull request as draft July 29, 2026 00:28
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.

1 participant