Fix SCCA_PMD's bisection to be scale-invariant - #230
Merged
Conversation
_bisect_threshold compared the unnormalised power-iteration update's raw L1 norm directly against l1_bound (tau*sqrt(p)), a bound that is only meaningful for a unit-L2-norm vector. Since the raw update's magnitude depends on the data's scale (not on tau), this made the effective sparsity constraint depend on the data's scale: in the common case where the raw update's L1 norm exceeds l1_bound, tau bound far more aggressively than intended, up to and including tau=1 (nominally "no sparsity constraint") still collapsing to near-total sparsity. Fixed by bisecting on the L1/L2 ratio of the thresholded vector instead, which is invariant to the input's overall scale. Added regression tests for scale-invariance and tau-monotonicity.
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
2 participants
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.
Summary
_bisect_thresholdcompared the unnormalised power-iteration update's raw L1 norm directly againstl1_bound(tau * sqrt(p)), a bound that is only meaningful for a unit-L2-norm vector (||w||_1 <= sqrt(p)for||w||_2 = 1is the Cauchy-Schwarz boundtau in (0, 1]relies on).tau, this made the effective sparsity constraint depend on the data's scale: whenever the raw update's L1 norm exceededl1_bound(the common case for reasonably-sizedn),taubound far more aggressively than intended -- up to and includingtau=1("no sparsity constraint") still collapsing to near-total sparsity.Test plan
test_scca_pmd_invariant_to_input_scale: fits on the same data scaled by 37x and checks the recovered direction is unchanged (up to sign).test_scca_pmd_tau_controls_sparsity_monotonically: checks nnz is non-decreasing intau, and thattau=1(nominally unconstrained) recovers the fully dense solution.511 passed, 7 skipped.Generated by Claude Code