-
-
Notifications
You must be signed in to change notification settings - Fork 57
Convert z
in CubicSpline
for performance
#457
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
It looks reasonable enough to me. Though what does it do to the arrays of arrays case? allocate the views? |
I think so, results for If you meant the |
Covers vector of vector input and vector of matrices input
Can support `Vector{Vector}` and `Vector{Matix}` input as well; tests added to confirm behavior.
Need to remove value `push!`ed onto `u` prior to new tests.
With this PR, the following methods work with
Not all of these methods have tests confirming this behavior. I had to relax the type signatures of the The remaining methods that don't seem to work with
I am interested in trying to get PCHIPInterpolation and AkimaInterpolation working, so I may look into that in another PR. For now I think this PR is ready for review. |
We should test this via AllocCheck.jl so that it doesn't regress. |
I have pushed an example for what tests checking |
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
This hopes to address the performance issues of multi-valued interpolation discussed in #431 by using dependent variable arrays with elements of
StaticArrays.SArray
to remove allocations and improve performance. Onmaster
this still allocates when evaluating the interpolants, hurting performance.For the example of
CubicSpline
, I isolated the allocations to come from thez
array. By converting the elements ofz
to matcheltype(u)
, we are able to remove allocations when evaluating the interpolant and greatly improve the performance. Example usage and performance comparison is given below.All tests pass for me locally on Julia 1.11.6. If this seems like a reasonable solution, I can look into modifying the other relevant methods as well.