-
Notifications
You must be signed in to change notification settings - Fork 1
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
Type Floats
cannot support Unitful vectors
#4
Comments
I'd be very happy to enlarge the possible types for the "values" manipulated in
For efficiency reasons (large scale problems) value types should be homogeneous (the same concrete type for all values in a given variable or mapping). Values may be real/complex but const LazyReals = Union{AbstractFloat,AbstractQuantity{<:AbstractFloat}}
const LazyComplexes = Complex{<:LazyReals} Maybe something like that is sufficient (after all, using Unitful
x = 1.0u"kg"
y = 1.0u"g"
x.val # yields 1.0
y.val # yields 1.0
(x/y).val # yields 1.0
uconvert(Unitful.NoUnits, x/y) # corretly yields 1000.0
uconvert(Unitful.NoUnits, y/x) # corretly yields 0.001 which (for me) means that we may have to explicitely call H = alpha*A + beta*B with So perhaps we can just give a try to allow for values with units. Methods |
Thanks for the detailed notes. I too am interested in inverse problems and I agree with nearly everything you wrote above for the same reasons. (The only exception is that in MRI work I truly use complex numbers rather than thinking of them as pairs of reals and I use the usual complex-valued inner product most of the time. But that is a minor point here.) I especially agree that all values in an array should have the same I have never used The |
You are right, it is only in optimization code that complexes should be considered as pairs of floats. The inner product ( I really like your duck-typing approach, type assertions should be relaxed to exploit one of the power of Julia that the same code can serve to many different argument types (I was probably too much influenced by my experience in C coding). I agree that it makes sense to let Julia complain when argument types are incompatible. Below is a demonstration that relaxing types to any function vdot1(x::AbstractArray{Tx,N}, y::AbstractArray{Ty,N}) where {Tx,Ty,N}
s = zero(promote_type(Tx, Ty))
@inbounds @simd for i in eachindex(x, y); s += x[i]*y[i]; end
return s
end
function vdot2(x::AbstractArray{Tx,N}, y::AbstractArray{Ty,N}) where {Tx,Ty,N}
s = zero(typeof(oneunit(Tx)*oneunit(Ty))) # there are probably simpler expressions that yield this
@inbounds @simd for i in eachindex(x, y); s += x[i]*y[i]; end
return s
end
using Unitful
T, n = Float32, 5_000
x=rand(T,n).*1u"kg"
y=rand(T,n).*1u"m"
vdot1(x,y) # throws "ERROR: ArgumentError: zero(Quantity{Float32, D, U} where {D, U}) not defined."
vdot2(x,y) # yields a correct result in "kg⋅m" The only thing to adapt is the initiallization of the result (here the accumulation variable I benchmarked that the The only remaining issue I can anticipate is the case of multipliers. To not penalize optimizations, these factors are supposed to not enforce their precision but instead follow that of the quantities multipled by them. We have to design a new implementation of |
All looks good! |
The
Floats
type used here is a union of AbstractFloats and Complex and is a very sensible type for the goals of this package. Unfortunately that union is not quite general enough to include Unitful vectors because such vector's elements are subtypes ofNumber
not ofAbstractFloat
. So simple methods likevscale!
do not apply as currently written. MWE:It probably would be a major undertaking to support Unitful vectors here, so I am not really advocating for that.
But I am just reporting the issue here because it is relevant to another PR elsewhere:
emmt/LinearInterpolators.jl#9
The text was updated successfully, but these errors were encountered: