JuliaGeo / Geodesy.jl

Work with points defined in various coordinate systems.
MIT License
112 stars 24 forks source link

ENU / LLA parameterization too lax? #26

Closed garborg closed 7 years ago

garborg commented 7 years ago

I'm glad the signatures were loosened -- what do you think about limiting to {T <: AbstractFloat}?

julia> LLA('a', 'a', 'a')
LLA(lat=a°, lon=a°, alt=a)

julia> ENU('a', 'a', 'a')
3-element Geodesy.ENU{Char}:
 'a'
 'a'
 'a'
andyferris commented 7 years ago

I would prefer Number (we use dual numbers for autodifferentiation)

garborg commented 7 years ago

That makes sense. I'd seen seen Real used in Distributions to allow for the same -- whatever's the tightest signature that works for you sounds great -- easy to loosen later if need be.

andyferris commented 7 years ago

Right - many in the community use Real to mean NotComplex but that is IMHO wrong. Dual numbers are neither real nor complex. Similarly for quaternions, etc.

Unfortunately, Julia doesn't support typediff in the spirit of setdiff.

andyferris commented 7 years ago

Done on master

garborg commented 7 years ago

Sounds good. I noticed some of the differentiation packages worked with subtypes of Real even though DualNumber isn't, but that's the extent of my knowledge of that part of the ecosystem.

c42f commented 7 years ago

As far as I know, DualNumbers.Dual isn't used by the "serious" numerical autodiff packages, and the subtyping is one of the reasons. ForwardDiff.Dual made the compromise to subtype Real so that it would work with more user defined functions.

garborg commented 7 years ago

That's what I understood.

Off topic, really, but to preach to the choir, I too hope a better hierarchy or hierarchy + traits emerges for numbers. Ecosystem-wide labeling of Number, or even Real, for methods written with floats in mind is unfortunate. If ForwardDiff.Dual, etc., were out of the picture, my preference would be to start with the narrower signature the author had in mind when writing, just until other types of numbers were tested and the person with a use case with for widening validated the algorithm's suitability for new types.