Closed lostella closed 2 years ago
Merging #62 (20dda99) into master (3cd2cca) will decrease coverage by
0.74%
. The diff coverage is95.72%
.
@@ Coverage Diff @@
## master #62 +/- ##
==========================================
- Coverage 89.33% 88.59% -0.75%
==========================================
Files 22 22
Lines 966 894 -72
==========================================
- Hits 863 792 -71
+ Misses 103 102 -1
Impacted Files | Coverage Δ | |
---|---|---|
src/algorithms/li_lin.jl | 75.55% <80.00%> (-3.30%) |
:arrow_down: |
src/algorithms/primal_dual.jl | 61.73% <85.71%> (-2.20%) |
:arrow_down: |
src/algorithms/drls.jl | 94.52% <90.00%> (-0.48%) |
:arrow_down: |
src/algorithms/sfista.jl | 83.33% <90.90%> (ø) |
|
src/algorithms/davis_yin.jl | 92.30% <92.85%> (-1.64%) |
:arrow_down: |
src/ProximalAlgorithms.jl | 90.00% <100.00%> (+90.00%) |
:arrow_up: |
src/algorithms/douglas_rachford.jl | 93.33% <100.00%> (-2.32%) |
:arrow_down: |
src/algorithms/fast_forward_backward.jl | 96.96% <100.00%> (-0.54%) |
:arrow_down: |
src/algorithms/forward_backward.jl | 96.29% <100.00%> (-0.77%) |
:arrow_down: |
src/algorithms/panoc.jl | 97.70% <100.00%> (-0.13%) |
:arrow_down: |
... and 3 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 3cd2cca...20dda99. Read the comment docs.
@aldma since you contributed PANOCplus, I would kindly ask you to take a look to the changes I’m doing to it here (see PR description), whenever you are able to, and confirm that they look OK.
Feel free to focus on the changes to PANOCplus, of course :-)
In particular, I’m changing the behavior when the maximum number of backtracking steps on tau is exceeded, to have the “nominal” forward-backward step accepted instead of the iteration halting.
@wwkong I’m proposing some breaking changes to the SFISTA method you contributed, to uniformize the naming across algorithms: the proximable term h
becomes g
, the initial iterate y0
becomes x0
, the convexity modulus of f
becomes mf
. So this PR is simply renaming things, and the algorithm is unaffected. Please feel free to look into the changes (only the SFISTA ones!) and confirm I didn’t do anything silly there :-)
@lostella Your changes to PANOCplus look good. Nice refactoring and docs, indeed!
Merging, I’ll address further comments separately, if there are any 🙃
This PR touches lots things, but the changes are not that many, with several concerning documentation. However, a few breaking changes are there. All of the changes were done working backwards from the documentation process, trying to simplify and uniformize the interface to algorithms.
Breaking changes
DavisYin
now hasf
as smooth term (wash
) to align it with all other algorithms; the smoothness constant was renamed toLf
as a consequence.DRLS
now usesmf
as strong convexity parameter (wasmuf
).SFISTA
arguments were uniformized to other algorithms:x0
is the initial iterate,g
is the proximable term,mf
is the convexity modulus off
.Other changes
IterativeAlgorithm
type, which encodes at once how to construct and iterate a given iterator type, with stopping criterion and verbosity and so on; specific algorithms construct an object of this type now, with a particular iterator type, default stopping criterion (which can now be replaced with anything if one wants, not just tuned via the tolerance) and default values for other options.iterate
method to make it type stable (a new state is always returned, nevernothing
).Documentation changes
Tests
Except for the changes to the algorithms interface (described above), test scripts are unchanged.