Expose AA trust_factor in ScsSettings (opt-in, default off)#396
Open
bodono wants to merge 2 commits into
Open
Conversation
Adds `acceleration_trust_factor` to ScsSettings. When set to a finite
positive value, the AA solve in include/aa.h activates two coupled
mechanisms:
1. Trust region: each AA solve rejects steps with ||D γ||_2 >
trust_factor * ||g||_2.
2. Adaptive regularization: the Tikhonov term r adapts via accept /
reject feedback (start large = AA ≈ DRS, shrink on accept, grow on
rejection).
Together these recover the type-II broken regime on slow-contraction
maps (ADMM/DRS) — on the Maros-Meszaros + Netlib benchmark, type-II's
penalized geomean drops 32.8 → 14.5 with trust_factor=10 and 31 of 33
previously-failed problems recover with no working-set regressions.
The default value AA_TRUST_FACTOR is INFINITY (= "no cap"), which keeps
the existing behavior unchanged: the original ε·||S||_F·||Y||_F
regularization path is used and no trust region is applied. Callers
who want to opt in can sweep the field; the parameter accepts INFINITY
and any positive finite value (NaN, 0, negative are rejected).
The vendored aa.c / aa.h are updated to the cvxgrp/aa aa-trust-factor
branch which adds the trust_factor argument to aa_init.
rw.c writes acceleration_trust_factor at the END of the settings block
and the reader tolerates EOF there, so data files written by older scs
versions still load (the field defaults to INFINITY = current behavior).
Tests: 57 → 59. New sweep test covers INFINITY (default) and a finite
value; new validation test rejects NaN/0/negative. All 59 tests pass on
both direct and indirect builds.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The EOF-tolerant read for acceleration_trust_factor was raising a warning on gcc with -Werror builds because the return value of fread was being ignored. Capture it and explicitly no-op on short read: the default INFINITY has already been written into the struct.
4 tasks
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.
Summary
Adds
acceleration_trust_factortoScsSettings. When set to a finite positive value, the AA solve activates two coupled mechanisms (introduced in cvxgrp/aa#54):‖D γ‖_2 > trust_factor · ‖g‖_2.radapts via accept / reject feedback (start large = AA ≈ DRS, shrink on accept, grow on rejection).Together these recover the type-II broken regime on slow-contraction maps. The default is
INFINITY(= "no cap") — existing callers see no behavior change.Why opt-in?
Setting
acceleration_trust_factor = 10on the Maros-Meszaros + Netlib benchmark:trust_factor=10The trust region helps type-II dramatically but slightly hurts type-I, and the historical best (type-I at 13.7) is still the global best — so flipping the default would be a small regression for users on type-I. Leaving
INFINITYas the default preserves current performance for everyone while letting type-II users (and anyone willing to sweep) opt in.What's in this PR
include/scs.h:acceleration_trust_factorfield onScsSettings.include/glbopts.h:AA_TRUST_FACTORdefault =INFINITY.src/util.c: default set inscs_set_default_settings.src/scs.c:validate()rejects NaN / non-positive (INFINITY accepted);aa_initcall now passestrust_factor.src/rw.c: serialized at the END of the settings block; reader tolerates EOF there (data files written by older scs versions load fine, with the field defaulting to INFINITY).src/aa.c,include/aa.h: vendored from cvxgrp/aa#54 — addstrust_factorparameter toaa_initplus the trust-region + adaptive-r mechanisms.test/problems/test_solver_options.h,test/run_tests.c: sweep test (INFINITY + 10.0) and validation test (NaN / 0 / negative rejected).Test plan
make test: 59/59 pass on both direct and indirect builds (was 57; +2 new tests)acceleration_trust_factoris left at default (INFINITY) — random_prob and other read-from-file tests still load and solve correctlyDependency
Depends on cvxgrp/aa#54 landing first (the vendored aa.c / aa.h in this PR matches that branch).
🤖 Generated with Claude Code