-
Notifications
You must be signed in to change notification settings - Fork 11
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
All struct fields should be concrete types #22
Comments
@jamgochiana By the way: Don't feel obligated to do all of this. These are just some hints as I really like the idea of this package. If it seriously bothers me when using this package, I'll just open a PR one day. |
No, by all means, keep it coming this is great! |
Regarding the use of |
POMDPs uses function fields quite a lot (e.g. for struct Foo{F<:Function}
f::F
end or am I getting your question wrong? |
Fantastic, I forgot this worked for functions :) |
If you want to maximize performance, having structs immutable is a good first step. However, to get even better performance, it should be avoided to have non-concrete fields. These things are sometimes quite subtle, e.g. this
GaussianFilters.jl/src/kf_classes.jl
Line 27 in bc24efe
is a problem because
Symmetric
has two template parameters, e.g.:Therefore
Symmetric{a} where {a}
is not a concrete type and makes uses of the corresponding struct unnecessarily slow. A better thing to do here is to have a bounded type parameter, e.g.:Another instance of this problem is the use of
Function
as a field type, asFunction
is also an abstract type:GaussianFilters.jl/src/kf_classes.jl
Line 87 in bc24efe
The text was updated successfully, but these errors were encountered: