Repository navigation
Conversation
Signed-off-by: Hiroki Naganuma <hiroki11x@gmail.com>
|
|
Thanks for the contribution, I should probably update the document. We are still in the process of refactoring. This won't be merged and not necessary:
|
|
@skyw To clarify, I tested I may be misunderstanding the intended state semantics, but I do not think that keeping the eigenbasis sorted is sufficient by itself. The basis may be sorted for I reproduced this directly with the current Could you clarify whether v3 intentionally defines |
anyway, let me check your code again and compare what it does exactly. Could be I misunderstood it. |
|
Thanks for clarifying. I now understand that My earlier test assumed that history should follow the directions. It demonstrates a difference between the two policies, but does not establish a correctness bug. I rechecked both implementations, and each matches its respective update rule. The PR is therefore better viewed as an alternative state-update policy. Documenting the sorted-position semantics would help avoid this ambiguity. Thanks for taking another look. |
I check the code and math again. Although there is no proof, I still believe the algorithm only works if eigen bases don't change much. The math in the paper needs fixed eigen bases to work. EMA and rest of it were completely ignored. It is not very obvious, we have a Adam warm up period to skip the frequent eigen bases change early in the training. And if we used eigh all alone, there won't be anything to realign because no such information existed. The main question I think is whether power iteration approximated eigen bases represent some soft of permutation of previous eigen bases when they change by large amount. I can see argument goes both ways although not very strong. |
What
Keep SOAP-family diagonal second moments aligned with eigenbasis columns when QR-based orthogonal iteration sorts those columns.
This restores the required permutation in:
The existing 2-tuple return from
orthogonal_iterationandget_eigenbasis_qrremains the default. Callers that own coordinate-wise state can opt in to receive the sort indices.Why this is a correctness bug
Let the post-QR bases be sorted as
for permutation matrices
P_LandP_R. A tensor represented in that basis changes coordinates asso SOAP's coordinate-wise second moment must follow exactly:
The current implementation sorts
Qbut leavesV = exp_avg_sqin the old slot order. This makes the physical Adam update depend on a pure basis-label permutation.The original SOAP implementation applies the same
sort_idxto the basis andexp_avg_sq. This regression was introduced when #274 moved eigenvalue sorting after QR and removed the previous joint permutation helper. Sorting after QR is retained here; the minimal fix is to apply that actual post-QR permutation toexp_avg_sqtoo.Deterministic oracle
A new rectangular 2x3 test forces different permutations on the two axes (
[1, 0]and[1, 2, 0]). It verifies both the exact second-moment permutation and that a pure relabeling leaves the represented physical update unchanged. Separate tests cover the low-level sort indices, both MOSO axes, and KL-SOAP v3.For the simple 2x2 state
M=[[1,2],[3,4]],V=M^2, with both axes swapped, the correct physical update remains all ones. LeavingVstale instead gives[[0.25, 0.6667], [1.5, 4.0]].Validation
4.77e-7mypy emerging_optimizers: 63 source files passedgit diff --checkpassedThe GPU reference and multistream paths are left to CI.
Scope
This PR intentionally does not change Emerging-Optimizers' current-gradient inclusion or per-step QR behavior. Those differ from the ICLR 2025 algorithm but are explicit design choices in arXiv:2607.20548, rather than this permutation bug.