Skip to content
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

Upgrade to DataInterpolations v7 #2019

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Upgrade to DataInterpolations v7 #2019

wants to merge 2 commits into from

Conversation

visr
Copy link
Member

@visr visr commented Jan 27, 2025

Instead of adding extra dummy data points to the arrays, we can now directly specify the extrapolation behavior in the DataInterpolations API:

https://docs.sciml.ai/DataInterpolations/stable/extrapolation_methods/

Fixes #1621.

Instead of adding extra dummy data points to the arrays, we can now directly specify the extrapolation behavior in the DataInterpolations API:

https://docs.sciml.ai/DataInterpolations/stable/extrapolation_methods/

Fixes #1621.
@visr
Copy link
Member Author

visr commented Jan 27, 2025

Oh this still fails a few tests, might need some work in DataInterpolations:

ERROR: LoadError: UndefVarError: `extrapolate_integral_left` not defined in `DataInterpolations`
Suggestion: check for spelling errors or missing imports.
Stacktrace:
  [1] integral(A::DataInterpolations.LinearInterpolation{Vector{Float64}, Vector{Float64}, Vector{Float64}, Vector{Float64}, Float64, (1,)}, t1::Float64, t2::Float64)
    @ DataInterpolations a:\.julia\packages\DataInterpolations\5Uosd\src\integrals.jl:27
  [2] formulate_storage!(current_storage::Vector{Float64}, tprev::Float64, t::Float64, flow_boundary::Ribasim.FlowBoundary{@NamedTuple{node_id::Vector{Int32}, time::Vector{Dates.DateTime}, substance::Vector{String}, concentration::Vector{Float64}}})
    @ Ribasim A:\repo\ribasim\Ribasim\core\src\solve.jl:182

@visr visr marked this pull request as draft January 27, 2025 16:06
@visr
Copy link
Member Author

visr commented Jan 31, 2025

The Int() calls in this PR can be removed if we update to v7.1, SciML/DataInterpolations.jl#379.

@SouthEndMusic
Copy link
Collaborator

Oh this still fails a few tests, might need some work in DataInterpolations:
SciML/DataInterpolations.jl#386

@SouthEndMusic
Copy link
Collaborator

I gave it a try with the current master branch of DataInterpolations, and it still breaks, although differently than what @visr mentioned above. Now it (still) breaks on integrating an interpolation with cache_parameters = true where the upper integration bound coincides with the first data point.

SciML/DataInterpolations.jl#390

@SouthEndMusic
Copy link
Collaborator

Yep, with this fix nothings breaks anymore, the only failing tests are due to some slightly different simulation results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup of Interpolation in qh_interpolation contains an "arbitrary" offset of -1.0
2 participants