feat: add is_parameter_timeseries trait, improve parameter timeseries indexing
#75
Conversation
This file contains 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
Merged
5 tasks
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #75 +/- ##
===========================================
+ Coverage 28.07% 92.82% +64.75%
===========================================
Files 11 12 +1
Lines 513 1018 +505
===========================================
+ Hits 144 945 +801
+ Misses 369 73 -296
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
AayushSabharwal
force-pushed
the
as/param-timeseries
branch
from
6c9c517 to
9130c4b
Compare
5 tasks
baggepinnen
reviewed
May 23, 2024
docs/src/api.md
Outdated
| #### Parameter timeseries | ||
|
|
||
| If the index provider contains parameters that change during the course of the simulation | ||
| at discrete intervals, it must implement the following methods to ensure correct |
There was a problem hiding this comment.
Suggested change
| at discrete intervals, it must implement the following methods to ensure correct | |
| at discrete time points, it must implement the following methods to ensure correct |
There was a problem hiding this comment.
Fixed in 9046cd7
docs/src/api.md
Outdated
| at discrete intervals, it must implement the following methods to ensure correct | ||
| functioning of [`getu`](@ref) and [`getp`](@ref) for value providers that save the parameter | ||
| timeseries. Note that there can be multiple parameter timeseries, in case different parameters | ||
| change at different times. |
There was a problem hiding this comment.
Suggested change
| change at different times. | |
| may change at different times. |
There was a problem hiding this comment.
Fixed in 9046cd7
docs/src/complete_sii.md
Outdated
| @@ -305,6 +313,164 @@ integrator.ps[:b] = 3.0 | |||
| setp(integrator, :b)(integrator, 3.0) # functionally the same as above | |||
| ``` | |||
|
|
|||
| ## Parameter Timeseries | |||
|
|
|||
| If a solution object saves modified parameter values (such as through callbacks) during the | |||
There was a problem hiding this comment.
Suggested change
| If a solution object saves modified parameter values (such as through callbacks) during the | |
| If a solution object includes modified parameter values (such as through callbacks) during the |
There was a problem hiding this comment.
Fixed in 9046cd7
AayushSabharwal
force-pushed
the
as/param-timeseries
branch
3 times, most recently
from
c228ec4 to
59fe65b
Compare
- some new, some deleted interface methods - extensive validation for input values - consistency with getu and getp - proper error messages
AayushSabharwal
force-pushed
the
as/param-timeseries
branch
from
59fe65b to
f9c500f
Compare
|
I'll reopen to run CI once MTK and SciMLBase are tagged (closed it a bit prematurely) |
|
RAT downstream failure is an adjoint bug in SciMLBase |
|
SciMLBase downstream failure is an MTK bug |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.