-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add unbounded objective tests #268
Add unbounded objective tests #268
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/fomo.jl
Outdated
set_status!(stats, :unbounded) | ||
break | ||
end | ||
unbounded = isinf(fck) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would go for a test like fx < min(-one(T), f0) / eps(T)
instead of testing Inf.
…OSolvers.jl into unbounded-below-test
add objective value test in unbounded tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @d-monnet
* update find_beta for the nonmonotone case * nonmonotone extension, remove useless norm computation * Add unbounded optimality in lbfgs * Add unbounded optimality for TRUNK * Add unbounded objective tests (#268) * add unbounded below obj test. Fix unbounded test in fomo. * add unbounded below obj test. Fix unbounded test in fomo. * standardize fomo :unbounded condition, add objective value test in unbounded tests. * rename: fk -> f0 * 🤖 Format .jl files (#270) Co-authored-by: d-monnet <[email protected]> * fomatting Co-authored-by: Tangi Migot <[email protected]> * update find_beta for the nonmonotone case * nonmonotone extension, remove useless norm computation * fomatting Co-authored-by: Tangi Migot <[email protected]> * fix rebase errors * add tests, replace `circshift` by index * fix allocs test * Update src/fomo.jl Co-authored-by: Tangi Migot <[email protected]> * update docstring * update docstring * Update src/fomo.jl Co-authored-by: Tangi Migot <[email protected]> --------- Co-authored-by: tmigot <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: d-monnet <[email protected]>
@@ -288,6 +289,9 @@ function SolverCore.solve!( | |||
solver.α = init_alpha(norm_∇fk, step_backend) | |||
|
|||
# Stopping criterion: | |||
fmin = min(-one(T), f0) / eps(T) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you work in Float64, this isn’t an especially large number in absolute value. Float64 can represent values up to about 1.0e+308. It would be better to use floatmax()
here. Effectively, when f0 < -floatmax(Float64)
, it means that f0 == Inf
. So you could declare unboundedness either if f0 == Inf
or f0 < floatmax(eltype(x)) / 2
(for example).
Looks like lbfgs doesn't detect unbounded objective.
Trunk subsolver errors (not a descent direction) before unbounded happen (if it happen). Maybe the subsolver crashes because under/overflow due to large value of the gradient.
fomo
set status to unbounded whenever objective value overflow (-Inf
) whiletron
usesfx < min(-one(T), f0) / eps(T)
conditions.I would be nice to have the same condition.