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

All struct fields should be concrete types #22

Open
lassepe opened this issue Sep 24, 2019 · 5 comments
Open

All struct fields should be concrete types #22

lassepe opened this issue Sep 24, 2019 · 5 comments

Comments

@lassepe
Copy link
Collaborator

lassepe commented Sep 24, 2019

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

W::Symmetric{c}

is a problem because Symmetric has two template parameters, e.g.:

julia> Symmetric(A)
3×3 Symmetric{Int64,Array{Int64,2}}:
 1  2  3
 2  5  6
 3  6  8

julia> typeof(Symmetric(A))
Symmetric{Int64,Array{Int64,2}}

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.:

struct LinearDynamicsModel{TA<:AbstracArray, TA<:AbstractArray,
                TW<:Symmetric} <: DynamicsModel
    A::TA
    B::TB
    W::TW
end

Another instance of this problem is the use of Function as a field type, as Function is also an abstract type:

@lassepe
Copy link
Collaborator Author

lassepe commented Sep 25, 2019

@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.

@jamgochiana
Copy link
Collaborator

No, by all means, keep it coming this is great!

@MaximeBouton
Copy link
Member

Regarding the use of Function as a field type.
For the nonlinear models, what is the best way to define them such that the filtering algorithm are type stable?
Because right now calling model.f or model.h is not type stable since it depends on what model is.
Any suggestions? Are there other packages that use that kind of interface?

@lassepe
Copy link
Collaborator Author

lassepe commented Oct 14, 2019

POMDPs uses function fields quite a lot (e.g. for FunctionPolicy). Things should be type stable if you do:

struct Foo{F<:Function}
    f::F
end

or am I getting your question wrong?

@MaximeBouton
Copy link
Member

MaximeBouton commented Oct 14, 2019

Fantastic, I forgot this worked for functions :)

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