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

Enable tolerance for ExtrapolationError checks #270

Closed
mleseach opened this issue Jun 16, 2024 · 3 comments
Closed

Enable tolerance for ExtrapolationError checks #270

mleseach opened this issue Jun 16, 2024 · 3 comments

Comments

@mleseach
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When dealing with pre-computed interpolations but with a different time parameterization, mapping from one parameterization to another can cause precision errors, leading to an extrapolation error if extrapolate=false.

using DataInterpolations

t1 = range(0.000000000000001, 10, length=10)
u = rand(10)

itp = LinearInterpolation(u, t1)

f(t) = t * 2 + 1
f⁻¹(t) = (t - 1) / 2 

t2 = f⁻¹.(f.(t1))

itp(t2[1]) # ExtrapolationError
# note that itp(t2) doesn't check properly for extrapolation

Describe the solution you’d like

I think the best solution is to allow some tolerance when checking for extrapolation error (e.g. something like t < itp.t[1] - eps(t) instead of t < itp.t[1] for the lower bound)

Additionally, the tolerance could be parameterized, allowing users to specify a custom tolerance level if the default isn't ideal for their use case. But in this case maybe extrapolate=true could be sufficient.

If you think this feature is within the scope of this package, I can create a draft PR and discuss the details.

@ChrisRackauckas
Copy link
Member

To me, extrapolate=true is sufficient and the cost of maintaining some odd boundary behavior likely is not worth it. @sathvikbhagavan can have an opinion

@sathvikbhagavan
Copy link
Member

Yes, I don't think it is useful supporting this in this package. extrapolate=true should be sufficient.

@ChrisRackauckas
Copy link
Member

Closing as not planned, but thank you for the input @mleseach as it is an interesting suggestion but we don't think it's the right idea at this time. As mentioned in the OP, we currently think extrapolate=true should be sufficient for most users, but if this is a frequently asked for change we'd be willing to reconsider it.

@ChrisRackauckas ChrisRackauckas closed this as not planned Won't fix, can't repro, duplicate, stale Jun 23, 2024
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

No branches or pull requests

3 participants