-
-
Notifications
You must be signed in to change notification settings - Fork 713
I4545-remove-explicit-sens #4975
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4975 +/- ##
===========================================
- Coverage 98.60% 98.57% -0.04%
===========================================
Files 304 304
Lines 23809 23641 -168
===========================================
- Hits 23478 23305 -173
- Misses 331 336 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MarcBerliner
left a comment
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 for working on this @martinjrobins! It makes the code so much cleaner :)
Just a few minor comments
| sens_idaklu = np.interp( | ||
| solutions[1].t[:-2], | ||
| solutions[3].t, | ||
| solutions[3]["Voltage [V]"] | ||
| t, | ||
| solutions[1].t, | ||
| solutions[1]["Voltage [V]"] | ||
| .sensitivities[input_param_name] | ||
| .full() | ||
| .flatten(), | ||
| ) |
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.
Can we change this to a higher order interpolator
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.
I tried this but it doesn't work very well due to the discontinuities in the experiment, linear interp behaves better
MarcBerliner
left a comment
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 Martin! Looks great
Description
Fixes #4545
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)
Important checks:
Please confirm the following before marking the PR as ready for review:
nox -s pre-commitnox -s testsnox -s doctests