Generalise _interpolate! to allow mixed dimensions
#33
Conversation
rafaqz
changed the title
_interpolate! to allow mixed dimensions_interpolate! to allow mixed dimensions
src/interpolation_methods.jl
Outdated
| all(map(check_derivative_order, dims, derivative_orders)) | ||
| check_derivative_order(::LinearInterpolationDimension, d_o) = d_o <= 1 | ||
| check_derivative_order(::ConstantInterpolationDimension, d_o) = d_0 <= 0 | ||
| check_derivative_order(::AbstractInterpolationDimension, d_o) = true |
There was a problem hiding this comment.
I feel like this name can be more descriptive, something like is_nonzero_derivative. For BSplineInterpolationDimension it would be is_nonzero_derivative(interp_dim::BSplineInterpolationDimension, d_o) = d_0 < interp_dim.degree + 1. That is only the case when no weights are defined; if weights are defined the interpolation is a NURBS geometry defined by rational functions which generally have all nonzero derivatives
There was a problem hiding this comment.
Totally, my names are likely pretty bad this PR is coming in cold not knowing much about interpolation vernacular
You can see them more as placeholder tags for a structure that works rather than clear or correct descriptors
src/interpolation_methods.jl
Outdated
| iteration_space(::LinearInterpolationDimension) = (false, true) | ||
| iteration_space(::ConstantInterpolationDimension) = 1 | ||
| iteration_space(::NoInterpolationDimension) = 1 | ||
| iteration_space(d::BSplineInterpolationDimension) = 1:d.degree + 1 |
There was a problem hiding this comment.
| iteration_space(d::BSplineInterpolationDimension) = 1:d.degree + 1 | |
| iteration_space(d::BSplineInterpolationDimension) = 1:(d.degree + 1) |
src/interpolation_methods.jl
Outdated
| iszero(derivative_order) ? t₂ - t : -one(t) | ||
| end * t_vol_inv | ||
| end | ||
| scale(::ConstantInterpolationDimension, prep::NamedTuple, i) = 1 |
There was a problem hiding this comment.
Perhaps just make a fallback method that returns true (I've seen that internally used in some places so it's probably a bit faster than multiplication with an integer).
| # end | ||
|
|
||
| denom = zero(eltype(t)) | ||
| function prepare(d::LinearInterpolationDimension, derivative_order, multi_point_index, t, i) |
There was a problem hiding this comment.
Can you motivate the need for these preparations? I'm generally not a fan of passing around NamedTuples.
There was a problem hiding this comment.
We need a way to do out of loop precalulation of arbitrary things for each dimension, and different interpolatiom methods need different fields.
We can of course use structs specific to each interpolation type, NamedTuple is just easy for iterative development of a new idea.
|
FYI there is a lot left to do, the tests that pass are because there are no missing dimensions but this will break if there are any. |
| sze = map(interp.interp_dims, size(interp.u)) do d, s | ||
| d isa NoInterpolationDimension ? s : length(d.t_eval) | ||
| end | ||
| # TODO: do we need to promote the type here, e.g. for eltype(u) <: Integer ? |
There was a problem hiding this comment.
Good point. I believe this is sometimes done by obtaining the type of a dummy calculation. Alternatively the output type can be passed as a kwarg
|
This should be working now. |
|
There are a few outstanding questions not - like #38 on what dimensions the weights matrix should have now. But more importantly, what should Currently Please chime in here this should be finalised and tested in this PR. |
2 participants
This PR is an exploration of #31 and #32 to see whats possible.
Probably quite slow in current form but this largely works already for the current tests.The idea is to break each dimension into a preparation step, a definition of the iteration space, and then scaling and index generation functions for the inner loop, then call all of these function from the same shared
_interpolate!method.Will need to add some tests for mixed dimension types.Closes #31 and closes #32