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

Revise type parameters of LinearMap #451

Closed
schillic opened this issue Aug 1, 2018 · 6 comments
Closed

Revise type parameters of LinearMap #451

schillic opened this issue Aug 1, 2018 · 6 comments
Assignees
Labels
fix 🤕 Fix to a problem that is not too serious

Comments

@schillic
Copy link
Member

schillic commented Aug 1, 2018

struct LinearMap{NM, N} <: LazySet{N}

Proposal:

struct LinearMap{N, NM} <: LazySet{N}

Reason: All other set types have the numeric type as their first argument. With this change we can do this:

function foo(X::LazySet{N}) where {N<:AbstractFloat}
    ...
    zero(N)
    ...
end

(We might actually do this already.)

@schillic schillic added discussion 🗣️ Requires human input fix 🤕 Fix to a problem that is not too serious labels Aug 1, 2018
@schillic
Copy link
Member Author

schillic commented Aug 1, 2018

While we are at it, should we add another type parameter for the set type, as we do for the other lazy operations?

@mforets
Copy link
Member

mforets commented Aug 3, 2018

yes, i agree. while we are at it, we can also add the parametric type of the linear map.

@schillic
Copy link
Member Author

schillic commented Aug 3, 2018

We could also change the second type argument from the matrix numeric type NM (which should usually be identical to N anyway) to the actual matrix type:
struct LinearMap{N<:Real, MAT<:AbstractMatrix{N}, S<:LazySet{N}} <: LazySet{N}

EDIT: We can also use M for MAT, but then we need to rename the field of the same name.

@mforets
Copy link
Member

mforets commented Aug 3, 2018

i think we should keep MN possibly different to N. for example if you have an interval matrix the coefficients are intervals.

@schillic
Copy link
Member Author

schillic commented Aug 3, 2018

Good point. Then, would it be
struct LinearMap{N<:Real, MAT<:AbstractMatrix{MN}, S<:LazySet{N}} <: LazySet{N}
or just
struct LinearMap{N<:Real, AbstractMatrix{MN}, S<:LazySet{N}} <: LazySet{N}
as before?
Motivation: Julia can create more efficient code if it sees the matrix type in advance, but I am not completely sure if it cannot see it in the other case as well.
Or do we even need MN then?

@mforets
Copy link
Member

mforets commented Aug 3, 2018

i would typically write out the full one:

julia> abstract type LazySet{N} end

julia> struct LinearMap{N<:Real, MN, MAT<:AbstractMatrix{MN}, S<:LazySet{N}} <: LazySet{N}
       end

julia> struct LinearMap2{N<:Real, MAT<:AbstractMatrix{MN}, S<:LazySet{N}} <: LazySet{N}
       end
ERROR: UndefVarError: MN not defined

@schillic schillic changed the title Change order of LinearMap's type parameters Revise type parameters of LinearMap Aug 4, 2018
@schillic schillic removed the discussion 🗣️ Requires human input label Aug 4, 2018
@schillic schillic self-assigned this Aug 5, 2018
schillic added a commit that referenced this issue Aug 5, 2018
#451 - Revise type parameters of LinearMap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix 🤕 Fix to a problem that is not too serious
Projects
None yet
Development

No branches or pull requests

2 participants