Skip to content

Conversation

@hakkelt
Copy link

@hakkelt hakkelt commented Nov 11, 2025

This PR is part of the effort to make StructuredOptimization as general as possible (see the PR on OperatorCore: JuliaFirstOrder/ProximalCore.jl#5).

Changes:

  • new get_assumptions function defined for all algorithms that allows querying the requirements for the algorithm's parameters (e.g., f must be is_smooth, g must be is_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 return true for 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.
  • Separate default_iteration_summary function from default_display function and allow overriding them separately through the general IterativeAlgorithm interface.
  • Improve the default display functionality by showing the header and automatically figuring out the optimal column width (it implements some of the wishes in Improve verbose mode #70 ).
  • Add override_parameters function that allows overwriting values of fields in IterativeAlgorithm struct by creating a copy with changed values. I use this in a high-level package to inject default maxit and atol values into the user-provided algorithm.
  • Introduce two new algorithms:
    • Alternating Directions of Multipliers Method (ADMM)
    • (Preconditioned) Conjugate Gradient (CG) -- I know that this is not a proximal algorithm, but ADMM uses it internally, so it seemed logical to also implement the full interface of ProximalAlgorithms.
  • Preallocate more in DavisYin, DouglasRachford, and FastForwardBackward
  • Minor fixes in docstrings

This PR contains no breaking changes (as far as I know), only introduces new features.

- 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
@lostella
Copy link
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:

  • Keep PRs small and focused, since reviewing 3000 line changes is very hard, especially if the changes are not always related
  • Keep the changes that belong to StructuredOptimization, into StructuredOptimization. Not everything needs to be pushed "upstream" from the get go, necessarily. For example, could you not define assumptions for the algorithms in ProximalAlgorithms, directly in StructuredOptimization? This is where the assumptions are needed, so it should be fine to define them there, without introducing inter-package design choices too early. Once things work well, one can think of exposing these interfaces in more "core" packages, and put the definitions next to the types. This way the wider plan is easier to understand & review

@hakkelt
Copy link
Author

hakkelt commented Nov 13, 2025

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 get_assumption: I'm ok with moving it to StructuredOptimization as it might too early have it here, but for maintainability, I would be glad if some machinery that allows querying is incorporated into AbstractAlgorithms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants