-
Notifications
You must be signed in to change notification settings - Fork 47
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
Relative: Fix return of inner parameters on objective call #1333
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1333 +/- ##
========================================
Coverage 84.25% 84.26%
========================================
Files 152 152
Lines 12452 12441 -11
========================================
- Hits 10492 10483 -9
+ Misses 1960 1958 -2 ☔ View full report in Codecov by Sentry. |
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.
Thanks. Looks good to me.
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.
LGTM
There was a bad implementation of inner parameter returning on call of objective.
Previously, the
InnerCalculatorCollector
would take note ofself.best_fval
such that it returnsinner_parameters
only in cases a better objective function value was found. This was ok during optimization, but then if one wanted to get inner parameters for some other outer parameter vector for which the objective function would not be better, the inner parameters would not be returned.This is bad practice: to save current states in the calculator and objective objects. So now, after an optimization start is finished, the
hierarchical_decorator
instead calls the objective once more with the best outer parameters found to obtain the inner parameters for that outer vector.There some small update in the visualize of
model_fit.time_trajectory_model
. Was not working with the current implementation due to recent hierarchical structure changes.