-
-
Notifications
You must be signed in to change notification settings - Fork 669
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:
|
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
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-commit
nox -s tests
nox -s doctests