StructuredOptimization-related changes + new algorithms - #100
Draft
hakkelt wants to merge 12 commits into
Draft
Conversation
- separate default_iteration_summary function from default_display function and allow overriding them separately through general IterativeAlgorithm interface - improve default display function by showing header and automatically figuring out optimal column width - add override_parameters function - get_addumptions function return AssumptionGroup instead of a Tuple - improve ADMM type stability - fix errors in CG - introduce CGNR algorithm as a variation of CG algorithm - preallocate more in DavisYin, DouglasRachford, and FastForwardBackward - minor fixes in docstrings
Member
|
@hakkelt I see that you're attempting various changes in different packages, all motivated by StructuredOptimization. I think it might be fine to move some core definitions to ProximalCore, but I would encourage you to:
|
Author
|
Thanks for the feedback, and sorry for the messy PRs. Would it help if I discard this PR and break the changes included in it into multiple smaller pull requests, submitting them one by one? Concerning |
hakkelt
marked this pull request as draft
December 2, 2025 11:09
Author
|
Instead of this large PR, I'll make multiple smaller, more focused PRs, starting with #101 |
Member
|
Thanks @hakkelt, I’ll take a look at it asap |
…Order#103) The current compat of the package is ```txt julia 1.6 [47edcb42] ADTypes 1.5.3 [a0c0ee7d] DifferentiationInterface 0.6.2 [37e2e46d] LinearAlgebra 1.2 [de0858da] Printf 1.2 [dc4f5ac2] ProximalCore 0.1 ``` The restriction on DifferentiationInterface.jl precludes using version 0.7.x that is required by other packages (like version 2.x of Optimjl). I tested ProximalAlgorithms.jl, changing compat to ```txt julia 1.6 [47edcb42] ADTypes 1.5.3 [a0c0ee7d] DifferentiationInterface 0.6.2,0.7 [37e2e46d] LinearAlgebra 1.2 [de0858da] Printf 1.2 [dc4f5ac2] ProximalCore 0.1 ``` All tests passed. Hence, it seems possible to use 0.7.x version of DifferentiationInterface.jl.
The x-update solves (AᴴA + ∑ᵢ ρᵢ BᵢᴴBᵢ) x = rhs with an inner CG loop. The system operator was rebuilt from scratch as a sum of operators every time the adaptive penalty sequence changed a ρᵢ. That rebuilt operator was never written back into the state, so once ρ stabilised CG silently reverted to running against the initial ρ; and the rebuild allocated a fresh operator tree per iteration. Replace it with ADMMNormalOp, a small struct holding AᴴA, the tuple of BᵢᴴB, and a reference to penalty_sequence.rho. Its mul! computes the combined action directly into a single scratch buffer. Because every PenaltySequence mutates seq.rho in place and never reassigns the field, the operator tracks ρ live with nothing to rebuild and nothing to write back. Built once in ADMMState; the per-iteration rebuild branch in Base.iterate is removed, along with the now-unused rho_changed return value.
- WohlbergPenalty: the "otherwise" branch of Eq. (54) in Wohlberg (2017) never set tau to tau_max, leaving it at its stale value for badly imbalanced residuals (the case needing the biggest correction). - BarzilaiBorweinSpectralPenalty: the reliability safeguard (paper Eq. 29) is a cosine correlation and needs a sqrt to be scale-invariant; it was missing on both the alpha and beta checks, and the beta check compared the wrong inner product (copy-paste from the alpha check). This is very likely the cause of the "TODO: does not converge" notes on this penalty sequence in the test suite. - SpectralRadiusBoundPenalty: added the [rho_min, rho_max] projection that Lorenz & Tran-Dinh (2019) Theorem 5.1 eq. (25) requires before blending the spectral-radius estimate; without it the fixed-bounded-interval premise of the convergence guarantee (Theorem 3.1) doesn't hold. Verified against test/accel/test_penalty_sequence.jl (67/67 pass) and by hand on the small lasso ADMM problem: WohlbergPenalty and BarzilaiBorweinSpectralPenalty now converge (33 and 103 iterations) where they previously did not. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implement the proximal optimized gradient method (POGM, Kim & Fessler 2018) as its own POGMIteration/POGMState in src/algorithms/pogm.jl, rather than folding it into FastForwardBackwardIteration behind a `pogm` flag: the two update rules share no math, and a merged struct would carry dead fields either way (extrapolation_sequence/z_prev unused in POGM mode, theta/y_prev/w_prev/zeta_prev unused in FISTA mode) plus an `if iter.pogm` branch in every iterate call. Currently supports the mf == 0, fixed-stepsize, no-restart case. `POGM(; kwargs...)` is the entry point, mirroring `FastForwardBackward`. Verified bit-exact against a direct port of Kim & Fessler's reference pogm_restart (mom=:pogm, restart=:none, mu=0) from JeffFessler/MIRT.jl. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
3 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.
This PR is part of the effort to make StructuredOptimization as general as possible (see the PR on OperatorCore: JuliaFirstOrder/ProximalCore.jl#5).
Changes:
get_assumptionsfunction defined for all algorithms that allows querying the requirements for the algorithm's parameters (e.g.,fmust beis_smooth,gmust beis_proximable). The function returns a dict-like object that has the names of the parameters as keys and a tuple of functions that are expected to returntruefor all allowed inputs as values. The concept is to enable automatic decisions on how to feed functions of the ProximalOperators package and operators of AbstractOperators to the algorithms.default_iteration_summaryfunction fromdefault_displayfunction and allow overriding them separately through the generalIterativeAlgorithminterface.override_parametersfunction that allows overwriting values of fields inIterativeAlgorithmstruct by creating a copy with changed values. I use this in a high-level package to inject defaultmaxitandatolvalues into the user-provided algorithm.This PR contains no breaking changes (as far as I know), only introduces new features.