search

JuliaNLSolvers / Optim.jl

Optimization functions for Julia
Other
1.1k stars 214 forks source link

Only warn for early stop if `options.show_trace` #1049

Closed MilesCranmer closed 6 months ago

MilesCranmer commented 10 months ago

Addresses discussion in https://github.com/JuliaNLSolvers/Optim.jl/pull/1046 with @mohamed82008

Users of PySR and SymbolicRegression.jl were suddenly getting spammed with warning messages (e.g., https://github.com/MilesCranmer/PySR/discussions/416) so I had to hotfix things to use an earlier Optim.jl.

This PR addresses this so that the warning only shows up if the user requested that level of information with show_trace = true. Otherwise the warning does not show. But it will still quit early which I agree is the correct behavior.

MilesCranmer commented 10 months ago

Pinging @pkofod

pkofod commented 9 months ago

I'm wondering if this is the right approach. If you have optimize in a loop where you don't want logging information, have you considered using with_logger(NullLogger) do .... end ?

MilesCranmer commented 9 months ago

I don’t want to skip all logging information, just the NaNs in the Optim loop. I want all logging info from any user-specified objective, or other optimization warnings, to be displayed.

Throwing a warning for NaNs in all optimization loops is a subjective design decision. For some applications where NaNs are frequently observed (like my symbolic regression use cases), it results in a bad user experience with tons of irrelevant warnings being printed. So I’d like to be able to turn it off.

pkofod commented 9 months ago

Okay. I personally think the options struct is way overloaded as it is, but maybe it's better to introduce a show_warnings keyword here instead? I'm not sure I agree the two are similar in nature.

MilesCranmer commented 9 months ago

Sounds good. I didn't want to add yet another option which is why I included it in show_trace. But happy to change it.

MilesCranmer commented 9 months ago

Let me know what your decision is?

MilesCranmer commented 8 months ago

Ping @pkofod. What do you prefer?

MilesCranmer commented 7 months ago

Ping @pkofod. This is holding up SymbolicRegression.jl and PySR from upgrading.

MilesCranmer commented 7 months ago

Okay I still haven't heard any response so I'm just going to submit a second PR with show_warnings instead and you can pick the PR you want.

MilesCranmer commented 6 months ago

Ping @mohamed82008 @pkofod. Sorry for being annoying but I've been waiting for this since August and it is still blocking me from upgrading.

pkofod commented 6 months ago

went with #1058