Merged
Conversation
plus a bunch of small documentation fixes and lints
…termination is not overwritten"
…for `Summary` This ensures different algorithms can provide different summaries without unused fields
…`process` (since not all algorithms minimize)
… create an ensemble via `on_status`
…` which checked termination conditions
This allows line searches to search the transformed space if the algorithm doesn't handle bounds internally
…e results are cleared when an algorithm is restarted
Warnings are on by default unless explicitly disabled (or re-enabled by a downward dependency) by the user
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #76 +/- ##
===========================================
+ Coverage 55.13% 87.03% +31.89%
===========================================
Files 26 32 +6
Lines 4097 5969 +1872
===========================================
+ Hits 2259 5195 +2936
+ Misses 1838 774 -1064 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #76 will improve performances by 18.79%Comparing 🎉 Hooray!
|
| Benchmark | BASE |
HEAD |
Change | |
|---|---|---|---|---|
| ⚡ | Rosenbrock (adaptive)[4] |
221.7 µs | 201.1 µs | +10.21% |
| ⚡ | Rosenbrock (adaptive)[5] |
949.2 µs | 799 µs | +18.79% |
| ⚡ | Rosenbrock[4] |
188.4 µs | 170 µs | +10.79% |
| ⚡ | Rosenbrock[5] |
240.5 µs | 208.5 µs | +15.37% |
…nus_g directly Not sure why I didn't have this before, maybe I forgot
denehoffman
force-pushed
the
refactor-unified-api
branch
from
64e511b to
3b51759
Compare
Owner
Author
|
Note: we expect the |
…` to PSO This also fixes a small bug in PSO where the boundary logic for Inf was flipped and the new points didn't get evaluated
The L-BFGS-B algorithm needs a Strong Wolfe line search anyway, so we shouldn't allow the user to make that mistake. This also makes it obvious that the user can modify the hyperparameters of the line search, and it completely gets rid of generics in the constructor, which is nice.
Also made it into a trait to ensure the signature is uniform wherever this is implemented.
Introduces the `process_default` method, which processes a problem using `default_callbacks` and configuration. Also updates the `Config` associated type to require `Default` so this method can work in the first place. That last part also makes it easier to use default configs without having to find the config struct.
Unfortunately, since we have some configs where the default x0 is a `DVector::zeros(0)`, the default config will always fail since we never set an initial position. I also reverted the `Default` trait bound on `Algorithm::Config` since it doesn't actually help us at all. I think I want to get rid of invalid defaujlt configs for L-BFGS-B and others in the next commit, but that'll wait for tomorrow.
commit 834d9e2 Author: Nathaniel D. Hoffman <36977879+denehoffman@users.noreply.github.com> Date: Fri Sep 5 21:00:35 2025 -0400 style: change `user_data` to `args` I think this is more inline with the intent of this signature commit ae3e763 Author: Nathaniel D. Hoffman <36977879+denehoffman@users.noreply.github.com> Date: Fri Sep 5 18:22:45 2025 -0400 refactor: change `user_data` field to be immutable I think the use cases of mutable user data are limited enough that we can probably rely on interior mutability or a mutex/rwlock to handle situations where we need it
also removed it in the signature of `process_default`
…void invalid state Previously, if you forgot to set an initial starting point or a set of walkers or a distribution of particles, some algorithms would just panic. We can instead prevent invalid state by refusing to provide a constructor for a config which is invalid, i.e. removing many of the `Default` implementations for configs. Since we always call `with_x0` or similar methods after `default()` anyway, this makes the syntax less verbose and makes it much more clear to the user what is required.
This also modifies the trait signatures of `LineSearch` to be a bit clearer on the output values, and it adds a curated enum of line searches which obey strong Wolfe conditions.
There are very few use cases where the problem needs to be mutable. In such cases, interior mutability with a `RefCell` or `Mutex` is recommended.
* feat: add `Transform` trait to differentiate explicit bounded algorithms from bounds transforms This is a potential feature which would simplify and clarify the difference between an algorithm which supports bounds "out of the box" like L-BFGS-B or (as I recently discovered) Nelder-Mead from those which support it via a MINUIIT-style change of coordinates. These changes of coordinates are now a separate trait, and `Bounds` implements this trait. Algorithms which don't natively support bounds can add a bounds transform with a different method from the `with_bounds` method: `with_transform`. Transformations are also nice because they can transform an entire problem space. For example, when fitting a multivariate normal distribution, symmetric positive-semidefinite covariance matrices are not only desirable, but required. However, with box bounds alone, there is no way to ensure this quality. A coordinate transform (`SymmetricPositiveSemidefiniteTransform`) ensures this property, allowing internal parameters to take any value given by the algorithm in the same way that bounds transforms allow algorithms to work on an unbounded space. * fix: remove binary reference in Cargo.toml * test: add tests for transforms/bounds * test: add tests for wrapped observers/terminators, fix minor inconsistency in MaxSteps, and add message to MaxSteps * test: remove extraneous code setting parameter names and add test for setting names in minimizations * style: rename transform methods * feat: add and methods as shorthand for getting owned versions of outputs * feat: many clippy lints plus an attempt at implementing transforms on L-BFGS-B. However, this will need a refactor, I'm running into some issues with how this currently is set up, but I wanted to commit this just to make sure I have it * feat: new draft of Transform using higher order derivatives * feat: make new generic cost/gradient for simulated annealing, remove Input on standard cost function, finish and test transforms, and add demo spherical coordinate transform * feat: change bounds implementation to one with better inverse, add optional to_internal_jacobian/hessian methods, remove SPD transform, and some perf lints * fix: accidentally transposed the inverse Jacobian twice * fix: revert to commented out code from debugging * feat: move DiffOps into Transform trait and update documentation with new transforms * feat: add new example generating and fitting multivariate normal data * feat: add some better styling to multivariate_normal_fit example * feat: overhaul bounds methods Big change to make it easier for users to not only implement their own bounds methods, but also to use legacy methods like Minuit/LMFIT bounds. Also a bunch of clippy lints and docstrings.
…ng solve where needed
This should probably never happen anyway
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Breaking Changes
This PR restructures the entire crate with a simple but powerful interface. The core ideas are as follows:
Algorithmtrait to be universal with generics/associated typesStatusobjects internalCallbacks, unifyingObservers with newTerminatorandCallbacktraitsMinimizerstruct in favor of a default method in theAlgorithmtraituser_datafield was renamed toargsand no longer needs to be mutable (in cases where the data needs mutability, opt for interior mutability or mutex/rwlock containers)Transformtrait to differentiateAlgorithms which natively handle bounds (like L-BFGS-B and Nelder-Mead) and those which require a change of coordinates.BoundLiketrait and use a faster set of transformations as default while providing Minuit/LMFIT-style transformations as an option.New Algorithms
In addition to the restructure, this PR provides some new algorithms:
Adam- used in stochastic gradient descentSimulatedAnnealing- a very versatile implementation that can handle non-standard cost functionsDocumentation
This PR increases the number of tests and enhances documentation. Tests now cover 87% of tracked lines.
Performance
Additionally, several optimizations were found, which can improve speeds by anywhere from 8% to 2.5x depending on the problem. In particular, the default implementation of the gradient/Hessian methods were improved, and some extraneous function calls were removed from the
L-BFGS-Bimplementation, both of which speed up the preferred gradient algorithm significantly. Additional improvements were made to the way the centroid was calculated in theNelder-Meadimplementation. See the benchmarks here to view how this PR compares to the current main branch at time of writing.Note
I believe this PR is a candidate for a v1.0.0 release, so I've removed the warning in
README.md.