Add test cases, disable flatness check in HZ #180
Draft
Conversation
This file contains 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
The flatness check in HagerZhang was intended to prevent excessive iteration in cases where the objective function is flat to within numerical precision. This test is a poor attempt at capturing the issue. HZ takes more iterations than any other linesearch algorithm, but it's still a far cry from real-world cases I've seen where the linesearch exits from the iteration limit.
|
Aha, it looks like CI caught a case where the new flatness test (without the flatness check) spent 32 iterations in a flat basin. That's a little closer to the behavior I've seen. I'll leave this open and wait for some feedback. |
|
Oh interesting, it's also the diff --git a/test/issues.jl b/test/issues.jl
index 7b40677..b37a851 100644
--- a/test/issues.jl
+++ b/test/issues.jl
@@ -57,14 +57,15 @@ end
BackTracking(; cache=cache), BackTracking(; order=2, cache=cache) )
n = 0
- while n < 10
+ while n < 1000
ϕ, dϕ, ϕdϕ = makeϕdϕ(randn(2))
ϕ0, dϕ0 = ϕdϕ(0)
dϕ0 < -eps(abs(ϕ0)) || continue # any "slope" is just roundoff error, but we want roundoff that looks like descent
n += 1
for ls in lsalgs
res = ls(ϕ, dϕ, ϕdϕ, 1.0, ϕ(0.0), dϕ(0.0))
- @test length(cache.alphas) < 10 # really should be < 5
+ length(cache.alphas) >= 10 && println(typeof(ls), ": ", length(cache.alphas))
+ # @test length(cache.alphas) < 10 # really should be < 5
end
end
endI get the following printout: |
|
In trying this locally, I got this many passes (out of 1000 attempts):
|
|
OK, this was more successful in capturing the purpose of the flatness check than I initially thought. Maybe best to wait to merge this for an actual fix. |
2 tasks
This follows the notation in the paper and is much more readable. It also introduces an external parameter, `epsilonk`, that "solves" the flatness problem (though it requires external specification).
|
Are there any ideas from here that @avikpal should consider for the newer LineSearch.jl? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 may cause problems of its own, but for the time being it's better
than the status quo.
Fixes #173
Closes #174
Fixes #175
I am pretty sure that without the flatness check, I've seen HZ iterate until the
linesearchmaxlimit is hit, which is rather inefficient. This comes nowhere close to hitting that limit, though. So until we have a better test case we should probably disable the flatness check.Note: this PR should probably not be squash-merged, the three commits are crafted to be independent.