Meshes.jl icon indicating copy to clipboard operation
Meshes.jl copied to clipboard

Make Vec a new type of StaticArrays.jl vector

Open juliohm opened this issue 3 years ago • 4 comments

Currently our Vec type is an alias to SVector and that helped us get off the ground and move fast with implementations. The only problem with SVector is that it doesn't force AbstractFloat coordinates. When the user types Vec(1,2) this will generate a vector with integer coordinates and many algorithms will break assuming that the vectors live in R^n.

We need a new definition for Vec that is simply a new type of StaticVector from StaticArrays.jl. They provide an interface and parent types to inherit most of the behavior of SVector. We just need to do that and add an inner constructor like we did for Point to force AbstractFloat coordinates.

juliohm avatar Nov 13 '21 10:11 juliohm

Why do you need a new type altogether? Don't we just need another constructor that converts to Float like with Point? Something like:

const Vec{Dim, T} = SVector{Dim,T}
Vec{Dim,T}(x::NTuple{Dim,T}) where {Dim,T} = Vec{Dim,T}(x)
Vec{Dim,T}(x::NTuple{Dim,T}) where {Dim,T<:Integer} = Vec{Dim,Float64}(x)

It seems to accomplish what we want here, but then again I am still trying to learn about structs, types, etc..

kylebeggs avatar Dec 22 '21 21:12 kylebeggs

It is because in Julia it is not good practice to define methods for types that are not defined in the same module. It is called type piracy. We are not the "owners" of SVector and SVector has a lot of behavior that is defined for 1-dimensional arrays but maybe not vectors in Rn (as in math)

On Wed, Dec 22, 2021, 18:42 Kyle Beggs @.***> wrote:

Why do you need a new type altogether? Don't we just need another constructor that converts to Float like with Point? Something like:

const Vec{Dim, T} = SVector{Dim,T}Vec{Dim,T}(x::NTuple{Dim,T}) where {Dim,T} = Vec{Dim,T}(x)Vec{Dim,T}(x::NTuple{Dim,T}) where {Dim,T<:Integer} = Vec{Dim,Float64}(x)

It seems to accomplish what we want here, but then again I am still trying to learn about structs, types, etc..

— Reply to this email directly, view it on GitHub https://github.com/JuliaGeometry/Meshes.jl/issues/207#issuecomment-999897807, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3JY52HV4LQVDJWIFKDUSJA3BANCNFSM5H6QBLUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

juliohm avatar Dec 22 '21 21:12 juliohm

The only problem with SVector is that it doesn't force AbstractFloat coordinates. When the user types Vec(1,2) this will generate a vector with integer coordinates and many algorithms will break assuming that the vectors live in R^n.

Hi! I would say that that is how usually dynamic types work in Julia, if a function only works with some subtypes then one restricts it at the function argument types, not at the object constructions level. Or else, even more common, one just lets the function break if the input types are not compatible.

I came here because I was wondering how the "small/static linear algebra" landscape in Julia was going. It seems to me that there is still no clear optimal solution. For instance, the Vec from GeometryBasics.jl which "is simply a new type of StaticVector from StaticArrays.jl" just lost an opportunity to automatically get a new feature defined for SVectors: https://github.com/JuliaGeometry/GeometryBasics.jl/issues/157

cdsousa avatar Jan 01 '22 13:01 cdsousa

@cdsousa we had some design iterations and thought that end users should see errors as early as possible with Integer coordinates. We assume that coordinates live in R^n, which is continuous. Discrete coordinates aren't supported.

Although some projects use <:Integer for exact representation, this is flawed. What we really want is Continuous<:X<:Real but Julia doesn't have this concept. It has AbstractFloat as the closest concept.

juliohm avatar Jan 04 '22 10:01 juliohm