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

Change default tolerances to be a bit more realistic #44

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

denehoffman
Copy link
Owner

The default epsilon values for basically all the methods were a bit too precise. While they work fine for the Rosenbrock demos, most real-world applications don't start with that level of precision (scipy uses 1e-4 for both tolerances on their Nelder-Mead and esp * 1e7 for their 64-bit float tolerance for L-BFGS-B, for example). I've tried to scale these and use values that can work with either 64- or 32-bit floats and approximate those used by the 64-bit only existing libraries (scipy and the FORTRAN implementation of L-BFGS-B). As a result, tests needed to be updated (in precision only) and benchmarks will probably run faster simply because it will take fewer steps to converge.

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.70%. Comparing base (63b6e11) to head (f9bc385).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   70.37%   70.70%   +0.32%     
==========================================
  Files           8        8              
  Lines        1826     1826              
==========================================
+ Hits         1285     1291       +6     
+ Misses        541      535       -6     

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

@denehoffman
Copy link
Owner Author

This seems to improve coverage too, but only because assert!(approx_eq!(...)) doesn't seem to provide the same coverage as assert_approx_eq!(...)! I guess the LLVM coverage thinks there's a path where the inside fails but the outside doesn't? Strange.

Copy link
Contributor

Benchmark for 7b83eef

Click to view benchmark
Test Base PR %
nelder-mead: rosenbrock/Adaptive/2 32.6±0.77µs 21.4±0.74µs -34.36%
nelder-mead: rosenbrock/Adaptive/3 83.6±1.17µs 56.9±1.13µs -31.94%
nelder-mead: rosenbrock/Adaptive/4 165.5±1.63µs 37.7±0.71µs -77.22%
nelder-mead: rosenbrock/Adaptive/5 267.6±5.52µs 186.9±4.27µs -30.16%
nelder-mead: rosenbrock/Standard/2 32.6±0.31µs 21.3±0.24µs -34.66%
nelder-mead: rosenbrock/Standard/3 69.6±1.01µs 47.4±0.58µs -31.90%
nelder-mead: rosenbrock/Standard/4 144.9±3.95µs 32.1±0.50µs -77.85%
nelder-mead: rosenbrock/Standard/5 238.4±4.30µs 43.5±0.93µs -81.75%

@denehoffman denehoffman merged commit 02a62fe into main Oct 24, 2024
6 checks passed
@denehoffman denehoffman deleted the epsilon-updates branch October 24, 2024 15:47
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.

1 participant