OptimizationGradientError

[jduerholt]

Motivation

I am currently experimenting with some custom kernels, which exhibit some numeric instabilities, which lead in some situations during optimization to NaNs in the gradient. To better examine these situations, I thought it could be nice if the error which is thrown in this situation also contains the current x which leads to the NaNs in the gradient. For this purpose, I implemented an OptimizationGradientError like the OptimizationTimeoutError which holds the current x.

Im also thinking about catching the error in timeout.py as the OptimizationTimeoutError, throwing a warning and returning a result of the optimization. What do you think about this? I think this could be a useful and elegant way of dealing with it.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Unit tests.

[Balandat]

Thanks for putting this up - this makes sense to me.

Im also thinking about catching the error in timeout.py as the OptimizationTimeoutError, throwing a warning and returning a result of the optimization. What do you think about this? I think this could be a useful and elegant way of dealing with it.

Yeah I think this generally make sense. One issue with warnings is that they're often overlooked or ignored - so I wouldn't want to change the default behavior that something that would previously throw an error would now just return a result. But maybe it could be gated by some raise_on_nan flag?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.98%. Comparing base (c895a8d) to head (81fbc3a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2537   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         193      193           
  Lines       17000    17005    +5     
=======================================
+ Hits        16998    17003    +5     
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[facebook-github-bot]

@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[esantorella]

Thanks! I'm all for making it easier to debug optimization errors :)

[jduerholt]

I think it is also fine as it is now, advanced users can now catch the error and see which is the x that creates the NaNs and further inspect them. From my side, no urgent need to further catch in minimize_with_timeout, based on an argument to the function. Maybe, I add this in a seperate PR, but from my side, this is good to go now. Thanks for the quick review!

[facebook-github-bot]

@Balandat merged this pull request in 974fa26.