-
Notifications
You must be signed in to change notification settings - Fork 918
Enhanced Levenberg-Marquardt Algorithm with Residual Function Support #1116
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
base: master
Are you sure you want to change the base?
Conversation
…r reflect its purpose in modeling optimization problems
…elated interfaces
- Add configurable accuracy with orders 1-6 for both model and residual functions - Use NumericalJacobian class for more reliable derivative approximation
…objective function
- TStatistics and PValues for parameter significance testing - ConfidenceIntervalHalfWidths for parameter precision - Dependencies to measure parameter correlations - Goodness-of-fit statistics
Lastly, I've added a new Ready for review and merge. |
I've just added the Please review and merge when ready. Thank you! |
In the meantime, I found a critical bug in the Trust Region optimization implementation. The subproblem solvers were using unscaled gradient and Hessian values directly from the IObjectiveModel, while they should have been using the scaled values that NonlinearMinimizerBase.EvaluateJacobian produces. The last commit fixes this issue by modifying the ITrustRegionSubproblem interface to accept the correctly scaled gradient and Hessian as parameters. |
Summary of Key Changes
|
Hi @diluculo, Please could this bit be pulled out to a separate PR for now?
|
Hi @febkor Thanks for your review. You're right, this PR contains several independent changes that should be separated. I've split it into 3 separate PRs:
I initially planned to separate Edit:
Hi @cdrnet, All PRs are now ready for review. |
Regarding the failing test in AppVeyor: This is unrelated to the Levenberg-Marquardt enhancements - it's a floating-point precision issue in the statistics tests for .NET 8.0 (off by ~1.03E-09). |
Hi @febkor Here is the mathematical background document I've prepared to explain the residual function support. It illustrates the relationship between the objective function (χ²) and the residual vector (R). I hope this helps in understanding the purpose and implementation of this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a brief review, more from a performance angle.
Hope you find them useful.
public NonlinearMinimizationResult(IObjectiveModel modelInfo, int iterations, ExitCondition reasonForExit) | ||
{ | ||
ModelInfoAtMinimum = modelInfo; | ||
Iterations = iterations; | ||
ReasonForExit = reasonForExit; | ||
|
||
EvaluateCovariance(modelInfo); | ||
EvaluateGoodnessOfFit(modelInfo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both EvaluateCovariance, EvaluateGoodnessOfFit seem like they should be optional.
The user might not need these, but they are relatively expensive to calculate.
On the master branch, EvaluateCovariance for example shows up in profiles as being quite significant (esp due to its allocations).
while (iterations < maximumIterations && exitCondition == ExitCondition.None) | ||
{ | ||
iterations++; | ||
|
||
while (true) | ||
{ | ||
// Store current Hessian diagonal for restoration if step is rejected | ||
var savedDiagonal = Hessian.Diagonal().Clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the separate Clone()
required?
Matrix.Diagonal() produces a new vector with values copied in from the matrix.
var mu = initialMu * Hessian.Diagonal().Max(); // Damping parameter μ | ||
var nu = 2.0; // Multiplication factor ν for mu updates | ||
|
||
// Counters for successful and failed iterations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ncsuc
counter seems to get incremented, but is not used for any logic?
|
||
// Create adapter function that returns the model value for current observation | ||
// when given parameters array | ||
double funcAdapter(double[] p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems it'd be more natural to extend the NumericalJacobian to support modelFunctions of the shape needed here?
The usage here seems more complex than needed and some inefficiencies:
- _modelFunction returns a whole vector, but only a single value is used
- this funcAdapter lambda creates an allocation per observation (minor in the context of the rest of the code, but still avoidable)
weights = weights.PointwiseAbs(); | ||
} | ||
|
||
Weights = (weights == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the Weights
matrix get used anywhere?
Apart from some trivial null checks, the usages I noticed all only use the diagonal, i.e. the weights
vector. Are there any I might have missed?
It can be quite expensive in memory terms to create it.
This PR implements:
NonlinearMinimizationResult
NumericalJacobian
classCloses #1114