-
Notifications
You must be signed in to change notification settings - Fork 18
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
✨ Add optimization history to result and iteration column to parameter history #1134
Conversation
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 0.23%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Benchmark is done. Checkout the benchmark result page. Benchmark diff v0.6.0 vs. mainParametrized benchmark signatures: BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)
Benchmark diff main vs. PRParametrized benchmark signatures: BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)
|
Codecov ReportBase: 87.0% // Head: 87.2% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1134 +/- ##
=======================================
+ Coverage 87.0% 87.2% +0.1%
=======================================
Files 101 103 +2
Lines 5203 5274 +71
Branches 912 916 +4
=======================================
+ Hits 4531 4599 +68
- Misses 525 527 +2
- Partials 147 148 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
afa2dee
to
0a63eb8
Compare
@jsnel I triggered a binder build of the PR branch so you don't need to wait to try it out. |
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
The index is redundant information which is not needed
This prevent possible problems with the iteration column in dataframes created from ParameterHistory
1545bf0
to
a1ff956
Compare
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.
Nice functionality! Some small changes suggested.
Co-authored-by: Joris Snellenburg <[email protected]>
Co-authored-by: Joris Snellenburg <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
Looks good to me now.
This change allows inspecting the stdout output from
scipy.optimize.least_squares
during optimization and saving the current iteration to the parameter history, as well as creating apd.DataFrame
for plotting and inspection.The
OptimizationHistory
class is written in a way that allows using it like a view ofpd.DataFrame
e.g.However, since the wrapping functionality is dynamic at runtime you lose help information and autocompleting from static analyzers, which why the underlying
pd.DataFrame
is also exposed via thedata
property which gives you the static analyzer goodies.Change summary
Links to the changed sections
Checklist
Closes issues
Might allow #233 or one of the related issues to be closed