-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
refactor: add output dimensionality as a parameter in AbstractInterpolations #342
Conversation
6ab798f
to
59ada74
Compare
59ada74
to
1df1996
Compare
c6184f8
to
a79f1fe
Compare
function get_output_dim(u::AbstractVector) | ||
return (length(first(u)),) | ||
end | ||
|
||
function get_output_dim(u::AbstractArray) | ||
return size(u)[1:(end - 1)] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sathvikbhagavan @ChrisRackauckas This breaks type inference of the constructors which causes problems in my downstream package (see #389). One way around this would be to allow (require?) users to specify the dimension as a type parameter when constructing the interpolation method.
However, it's not clear to me why this type parameter should be included at all. Arguably it also doesn't make sense at all for nonnumeric types (that I actually encounter when working with ConstantInterpolation
). Would it be OK to revert this change?
If the output dimension is actually of interest to someone then it seems easy enough for them to use the logic of get_output_dim
directly. Possibly this could also be made official by adding something like output_dim
to the API (if actually necessary). This could also e.g. error for non-numeric types.
fixes: #328