Merged
Conversation
Add an optional `callback` argument to the `GaussNewtonConjugateGradient` minimizer that gets called before yielding any model. The callback function takes a new `MinimizerResult` data class with some information about the minimization process.
Use the Log protocol in the Inversion class.
Revert this commit if not good. I pushed it by the end of the day.
The `InversionLog` now holds a `get_minimizer_callback` method that returns a function that can be passed to `Minimizer`'s as the `callback` argument. The `InversionLog` creates a new `MinimizerLog` every time the `get_minimizer_callback` is called, appends it to a running list of `minimizer_logs`, and returns the `update` method of the newly created one. The `Inversion` then passes this callable to the `Minimizer` when called. This way the logs of all minimization steps are being stored inside the `InversionLog`.
It's best to build the Rich renderables and store them in memory rather than rebuilding them every time the `__rich__` method is called. This is to avoid async issues that might happen when the `Live` wants to update the renderable, but some pieces of the code required to build it is still running (filling the log for example). So it's better to always have one renderable ready to be shown.
We better keep it flexible rather than having it as a dataclass. Update the MinimizerLog so it can add any column in the result to the logger.
Print the inversion log table first and then each of the minimizer logs in a tree structure. This way we simplify the `__rich__` method of the `InversionLogRich`: we don't need to worry about it having minimizer logs or not.
Keep models in the log, but hide them in the rich table.
santisoler
force-pushed
the
minimizer-log
branch
from
9b78821 to
f8dcb5c
Compare
santisoler
commented
Mar 25, 2026
santisoler
commented
Mar 25, 2026
Member
Author
|
I'll merge this PR as it is. I'm quite happy with it so far, despite of a few things I left for later (check TODO comments). I would also like to simplify the |
santisoler
commented
Mar 25, 2026
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.
Add
MinimizerLogclass that tracks information about minimizer calls. The minimizer logs are stored by theInversionobject itself. Improve the rich renders of the inversion log to include also the minimizer logs. Add a newMinimizerResultclass that will be used by minimizers to pass as argument to callbacks. Add number of conjugate gradient iterations toGaussNewtonConjugateGradientthat get passed to theMinimizerResult.