Skip to content
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

RFC: catch linesearch errors #275

Merged
merged 8 commits into from
Dec 15, 2016
Merged

RFC: catch linesearch errors #275

merged 8 commits into from
Dec 15, 2016

Conversation

anriseth
Copy link
Contributor

@anriseth anriseth commented Sep 7, 2016

I've just added this as a WIP proposal for how to treat linesearch errors, to add to #263. Probably no reason to fully develop this until #266 is sorted.

I'm using this at the moment to get around the Error throw, as I'm only calling Optim in an inner loop of a solve that does not require very high tolerance.

TODO:

  • Deal with minimum linesearch step

@codecov-io
Copy link

codecov-io commented Sep 7, 2016

Current coverage is 90.64% (diff: 91.86%)

Merging #275 into master will increase coverage by 0.01%

@@             master       #275   diff @@
==========================================
  Files            27         27          
  Lines          1569       1635    +66   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1422       1482    +60   
- Misses          147        153     +6   
  Partials          0          0          

Powered by Codecov. Last update 4427a7e...a6adbc7

@pkofod
Copy link
Member

pkofod commented Nov 9, 2016

@anriseth do you have any plans here?

@anriseth
Copy link
Contributor Author

anriseth commented Nov 9, 2016

I certainly use it sometimes, so let me update this to work with LineSearches.

@anriseth
Copy link
Contributor Author

anriseth commented Nov 9, 2016

Oh, actually I had already done that (JuliaNLSolvers/LineSearches.jl#19). I'll just have to tag a new version of LineSearches

@anriseth anriseth changed the title Example linesearch error throw RFC: catch linesearch errors Nov 9, 2016
@pkofod
Copy link
Member

pkofod commented Nov 9, 2016

cg seems to be hitting this, huh?

Anyway, I have another question, simply because I don't know. What's the effect on performance of doing try-catch-blocks? Is there any?

@anriseth
Copy link
Contributor Author

anriseth commented Nov 9, 2016

Not sure what you mean by cg hitting this?

And I don't know what the try-catch effect on performance is..

By the way, the "minimum linesearch step = 1e-12" is arbitrary(just using the same defaults as PETSc), and currently not enforced on successful linesearch steps. We should either not enforce it on failure, or enforce it everywhere (that would be a LineSearches task)

@pkofod
Copy link
Member

pkofod commented Nov 9, 2016

Not sure what you mean by cg hitting this?

No, sorry, it was unclear. What I mean is that tests seem to fail because one of the Conjugate Gradient tests end up in situation where the limit is reached, right?

@anriseth
Copy link
Contributor Author

anriseth commented Nov 9, 2016

What I mean is that tests seem to fail because one of the Conjugate Gradient tests end up in situation where the limit is reached, right?

Ah, no it failed because of a typo: cg uses df instead of d to refer to the DifferentiableFunction.
We have to wait for LineSearches 0.1.1 to be tagged for the tests to pass JuliaLang/METADATA.jl#6991

@anriseth
Copy link
Contributor Author

I've added some tests. Is there a way to verify text in a warn statement?

@pkofod
Copy link
Member

pkofod commented Nov 10, 2016

I've added some tests. Is there a way to verify text in a warn statement?

Hmm, my best bet is something like

_STDERR = STDERR
f() = warn("what, don't do that")
eR, eW = redirect_stderr()
f()
close(eW)
warning = readavailable(eR)
close(eR)
redirect_stdout(_STDERR)
String(warning)
# outputs 
# "\e[1m\e[31mWARNING: what, don't do that\e[0m\n"

@anriseth
Copy link
Contributor Author

Hmm. That works fine from the REPL, but if I put it in a loop and do a comparison on warning, I get a "Aborted (core dumped)" or "Segmentation fault" error, depending on the order of calls.

So can we just leave the test as it is?

@pkofod
Copy link
Member

pkofod commented Dec 13, 2016

Sorry for making this comment so late in the party, but since all the block are identical, maybe we should make a more general perform_linesearch_step-ish function to replace all that duplicate code. Maybe in another PR. Restarting tests.

@anriseth anriseth mentioned this pull request Dec 13, 2016
@pkofod
Copy link
Member

pkofod commented Dec 14, 2016

I know this is my fault for being slow, but can you move the news item to v0.7.2 ?

@pkofod
Copy link
Member

pkofod commented Dec 14, 2016

Thanks. Merging after tests (I know the changes can't break anything, just want to see something.)

@anriseth
Copy link
Contributor Author

anriseth commented Dec 14, 2016 via email

@pkofod pkofod merged commit 557a696 into JuliaNLSolvers:master Dec 15, 2016
@anriseth anriseth deleted the anriseth/lsexception branch December 25, 2017 14:03
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.

3 participants