Skip to content

Conversation

@nantonel
Copy link
Collaborator

Small modifications to printing output based on old solvers.

I think it would be nice to have also the cost function value! but I guess this requires adding a new field ProximalAlgorithm?

@coveralls
Copy link

coveralls commented Dec 11, 2017

Coverage Status

Coverage increased (+0.3%) to 97.213% when pulling 9d84ac3 on nantonel:master into ee8f26b on kul-forbes:master.

@codecov-io
Copy link

codecov-io commented Dec 11, 2017

Codecov Report

Merging #1 into master will increase coverage by 0.26%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #1      +/-   ##
==========================================
+ Coverage   96.94%   97.21%   +0.26%     
==========================================
  Files           7        7              
  Lines         262      287      +25     
==========================================
+ Hits          254      279      +25     
  Misses          8        8
Impacted Files Coverage Δ
src/algorithms/DouglasRachford.jl 100% <100%> (ø) ⬆️
src/ProximalAlgorithms.jl 100% <100%> (ø) ⬆️
src/algorithms/ZeroFPR.jl 99.05% <100%> (+0.07%) ⬆️
src/algorithms/ForwardBackward.jl 100% <100%> (ø) ⬆️
src/algorithms/Template.jl 100% <100%> (ø) ⬆️

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 ee8f26b...636b0ef. Read the comment docs.

Copy link
Member

@lostella lostella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some minor changes to be addressed, in my opinion, see the comments in the code (of course they apply to all the currently available algorithms).

# Constructor(s)

function DRSIterator(x0::BlockArray{R}; f=Zero(), g=Zero(), gamma::R=1.0, maxit::I=10000, tol::R=1e-4, verbose=1) where {I, R}
function DRSIterator(x0::BlockArray{R}; f=Zero(), g=Zero(), gamma::R=1.0, maxit::I=10000, tol::R=1e-4, verbose=2) where {I, R}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why verbose=2 by default? I would say 1, that is the minimum (nonzero) verbosity.


verbose(sol::DRSIterator, it) = sol.verbose > 0
verbose(sol::DRSIterator) = sol.verbose > 0
verbose(sol::DRSIterator, it) = sol.verbose > 0 && (sol.verbose == 1 ? true : (it == 1 || it%100 == 0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the above comment: should be sol.verbose == 2 ? true. Furthermore, I would make that 100 an option (output_freq maybe? Of course defaulting to 100).

@nantonel
Copy link
Collaborator Author

I've made the changes. Also I introduce the cost. However, in ZeroFPR the FBE is printed. I know FBE and f+g will coincide at minima but wouldn't it be more consistent to print the cost?

@coveralls
Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage increased (+0.3%) to 97.251% when pulling 20c0573 on nantonel:master into ee8f26b on kul-forbes:master.

x::T
f
g
cost::R
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forget about the cost for now, this we will add later. Let's not modify anything in the initialize and iterate methods in this PR, only visualization stuff.

@coveralls
Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage increased (+0.3%) to 97.213% when pulling 636b0ef on nantonel:master into ee8f26b on kul-forbes:master.

@lostella lostella merged commit ddd3599 into JuliaFirstOrder:master Dec 15, 2017
hakkelt added a commit to hakkelt/ProximalAlgorithms.jl that referenced this pull request Jul 2, 2025
hakkelt added a commit to hakkelt/ProximalAlgorithms.jl that referenced this pull request Nov 6, 2025
hakkelt added a commit to hakkelt/ProximalAlgorithms.jl that referenced this pull request Nov 11, 2025
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.

4 participants