Repository navigation
fix: Average MoE expert K-FAC covariances over their routed positions - #565
Open
flyingdoubleG wants to merge 1 commit into
Open
flyingdoubleG wants to merge 1 commit into
flyingdoubleG wants to merge 1 commit into
Conversation
An expert's covariances only sum the positions routed to it, but the eigendecomposition divided every module's sums by total_processed, the dataset's position count. That scaled each expert's K-FAC eigenvalues by the fraction of positions routed to it. CovarianceCollector now counts the positions each module summed, and fit_factored_hessians divides by that count in the eigendecomposition and the uncorrected eigenvalues. Dense modules are unchanged, their count being total_processed.
This branch has not been deployed
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.
An expert's K-FAC covariances (
CovarianceCollector) only sum the positions routed to it:collection_mask(module)selects the expert's_positionsgrid. The eigendecomposition then divides every module's sums bytotal_processed, which counts all gradient-carrying positions in the dataset. As a result, each expert's activation and gradient "means" are too small by a factor of p = (positions routed to it) /total_processed. Its uncorrected eigenvalue grid,λ_G ⊗ λ_A · total_processed / num_documents, ends up p times too small.damped_inverseand the other inversions damp relative to each module's own mean eigenvalue, so this does not wash out. An expert's preconditioned gradient comes out roughly 1/p times too large compared with the dense modules. With top-2 of 64 experts that is about 32×. EK-FAC is unaffected: the eigenvectors don't depend on the scale, and the corrected eigenvalues are per-document sums divided bynum_documents.Fix
CovarianceCollectorcounts the positions each module's covariances summed (num_positions). In a process group it all-reduces them at teardown.fit_factored_hessianspasses those counts toeigendecompose_ownedandsave_uncorrected_eigenvalues. Each module is now divided by its own count instead oftotal_processed.total_processed.Check
Each document has 2 tokens, so it has one gradient-carrying position. A module's per-document gradient there is
g aᵀ, which makes the EK-FAC corrected eigenvalues sum to exactlyE[|a|² |g|²]. The K-FAC grid sums toE[|a|²] E[|g|²], so the two should roughly agree. K-FAC / exact trace on a tiny Mixtral (4 experts, top-1, 64 docs), from the new test:The "before" ratios match each expert's routed fraction.
Tests
tests/test_moe.py::test_kfac_normalizes_experts_by_their_routed_positionsrunsfit_factored_hessianson that model and asserts the ratio is in (0.7, 1.4) for every module. It fails onmainand passes with this change.test_gloo_ranks_match_one_process_on_cpunow also checks that the all-reducednum_positionsequal the single-process counts.Not covered
compute_eigendecomposition(the file-based path used bytkfac/shampooand approximate unrolling's segment aggregation) still divides bytotal_processed. Those collectors would need to save their counts alongside the covariance shards; I can follow up if you'd like.Tested:
pytest tests/test_moe.py tests/ekfac_tests: 56 passed, 15 skipped on CPU (torch 2.9.1).