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

Constructor with units

Open SebastianRuffert opened this issue 2 years ago • 4 comments

SebastianRuffert avatar Jun 15 '22 07:06 SebastianRuffert

This is a very straightforward way to implement units. Do we want to allow for units in both constructor functions or only the one I changed so far?

SebastianRuffert avatar Jun 15 '22 07:06 SebastianRuffert

Also I am unsure if we should convert all default values to the user-specified unit, if only one keyword argument is given

SebastianRuffert avatar Jun 15 '22 07:06 SebastianRuffert

I don't think we should do this like that.

Let's take just the CartesianPoint as an example (its the same for everything else including the primitives):

CartesianPoint(1u"m", 2u"m", 3u"m")

should return CartesianPoint{promote_type(typeof.(x, y, y))} (pseudo code) and definitely not convert to CartesianPoint{Int/Float}.

Instead we have to add methods to Unitful.ustrip and Unitful.uconvert for our types. uconvert might be a bit difficult as our types have different fields. I am not sure if there is a general syntax for that in Unitful.jl. Maybe there is also a function like unify_units in Unitful.jl for cases like that. Or this should be handled in the promotion. E.g. (1u"m", 2u"mm)" will be promoted to (1.0u"m", 0.002u"m"). We have to look into Unitful.jl if there are already some conventions for this.

If not, we could (in SSD but not in the submodule CSG), define some _ssd_uconvert-methods for our types and then just ustrip them with the ustrip methods which we can define in the CSG submodule.

lmh91 avatar Jun 28 '22 12:06 lmh91

I just tested on the current main branch:

julia> CartesianPoint(1u"m", 2u"m", 3u"m")
3-element CartesianPoint{Quantity{Int64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}} with indices SOneTo(3):
 1 m
 2 m
 3 m

That works already out of the box as we do not limit T for AbstractGeometry.

julia> CartesianPoint(1u"m", 2u"m", 3u"mm")
3-element CartesianPoint{Quantity{Rational{Int64}, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}} with indices SOneTo(3):
    1//1 m
    2//1 m
 3//1000 m

And there seems also already to be some promotion rules defined in Unitful.jl as the second case also works :)

lmh91 avatar Jun 28 '22 12:06 lmh91