Support dispatches on `Number` via union type

[MilesCranmer]

This fixes #44 and refactors much of the internals to support two abstract quantities:

• AbstractQuantity{T,D} <: Number. Same name, but now only Number
• AbstractGenericQuantity{T,D}. New name, supports Any

It does this by creating UnionAbstractQuantity which is a union of these two types:

const UnionAbstractQuantity{T,D} = Union{AbstractQuantity{T,D},AbstractGenericQuantity{T,D}}

There is also now GenericQuantity as a built-in AbstractGenericQuantity, similar to how Quantity is the built-in AbstractQuantity.

In principle one could also add RealQuantity <: AbstractRealQuantity and so on. I chose to not add those yet until someone needs it.

@gaurav-arya what do you think?

---

The downside of this PR is that it hurts readability due to the use of @eval (required to avoid method ambiguities, and if we don't want to manually write a hundred extra methods...)

[github-actions[bot]]

Benchmark Results

	main	2a1c7ad3976e24...	t[main]/t[2a1c7ad3976e24...]
Quantity/creation/Quantity(x)	4.3 ± 0 ns	4.3 ± 0 ns	1
Quantity/creation/Quantity(x, length=y)	4.3 ± 0 ns	4.3 ± 0 ns	1
Quantity/with_numbers/*real	3.9 ± 0 ns	3.9 ± 0 ns	1
Quantity/with_numbers/^int	13.2 ± 3.9 ns	13.2 ± 3.9 ns	1
Quantity/with_numbers/^int * real	14.3 ± 4.1 ns	14.1 ± 4.3 ns	1.01
Quantity/with_quantity/+y	6.5 ± 0 ns	6.5 ± 0 ns	1
Quantity/with_quantity//y	4.7 ± 0.1 ns	4.9 ± 0 ns	0.959
Quantity/with_self/dimension	2 ± 0.1 ns	2 ± 0.1 ns	1
Quantity/with_self/inv	4.3 ± 0 ns	4.7 ± 0 ns	0.915
Quantity/with_self/ustrip	2 ± 0.1 ns	2 ± 0.1 ns	1
QuantityArray/broadcasting/multi_array_of_quantities	0.355 ± 0.18 ms	0.371 ± 0.2 ms	0.956
QuantityArray/broadcasting/multi_normal_array	0.104 ± 0.017 ms	0.0987 ± 0.029 ms	1.05
QuantityArray/broadcasting/multi_quantity_array	0.343 ± 0.062 ms	0.341 ± 0.06 ms	1.01
QuantityArray/broadcasting/x^2_array_of_quantities	0.0753 ± 0.034 ms	0.0749 ± 0.026 ms	1.01
QuantityArray/broadcasting/x^2_normal_array	12.9 ± 1.2 μs	12.9 ± 1.1 μs	1
QuantityArray/broadcasting/x^2_quantity_array	13.9 ± 1.1 μs	13.8 ± 1.3 μs	1.01
QuantityArray/broadcasting/x^4_array_of_quantities	0.184 ± 0.095 ms	0.185 ± 0.088 ms	0.995
QuantityArray/broadcasting/x^4_normal_array	0.0936 ± 0.016 ms	0.0907 ± 0.013 ms	1.03
QuantityArray/broadcasting/x^4_quantity_array	0.118 ± 0.016 ms	0.111 ± 0.019 ms	1.06
time_to_load	0.248 ± 0.031 s	0.312 ± 0.047 s	0.797

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

[MilesCranmer]

Awesome, thanks so much @gaurav-arya!!

Relatedly, should AbstractQuantity <: Number really be AbstractNumberQuantity <: Number. I'm fine with your choice here too: special casing Number as the shortest name / default type seems reasonable, as that's what Unitful worked with. Presumably future additions would look like AbstractRealQuantity <: Real etc.

Exactly my thoughts as well. i.e., keeping it AbstractQuantity just for Unitful compatibility. We could even have AbstractNumberQuantity as a synonym for it if needed, in case someone is confused why there is AbstractRealQuantity but no Number equivalent.

Further, I hope in a future PR to substantially reduce the code size of e.g.math.jl by classifying the overloads into their maybe 3 or 4 different kinds and then having a simple loop over functions for each (or a convenience macro to define the overload).

Great idea! We could use TypedDelegation.jl for this too if needed.

Unit parsing: Right now, AFAIU unit strings are always parsed as an instance of Quantity. This will be an interesting thing to revisit if were to want e.g. an AbstractRealQuantity in the future, as we may want 1.0 * u"m" <: Real for example. But I don't think there's any issue right now.

Sounds good, we can bring this part up again later!

[MilesCranmer]

Okay I added a ton of unittests for GenericQuantity. This should be ready to go now. Do you want to take a look @gaurav-arya?

[MilesCranmer]

Do we want this as a fallback method?

function Base.promote_rule(::Type{<:AbstractQuantity}, ::Type{<:Number})
    return Number
end

then you could have

julia> [0.5, 0.5u"km"]
2-element Vector{Number}:
       0.5
 500.0 m

which seems reasonable.

[gaurav-arya]

Sorry, ignore any notifications so far, I accidentalyl submitted some pending comments.

[gaurav-arya]

Overall I only had minor comments:) I made a bit of a mess of the review though, sorry about that..

I'd be happy to take another pass once there's documentation:)

[MilesCranmer]

Thanks!

I just added some more documentation examples. Let me know what you think?

[MilesCranmer]

Not sure what’s going on with the automated benchmarks, but if I run the benchmarks locally it seems to have no effect on the performance. Although I’m using 1.10 locally so perhaps there’s some difference on 1.9...

[gaurav-arya]

The performance regression on 1.9 appears to be real:

julia> using DynamicQuantities

julia> q = 1u"m"
1.0 m

julia> @btime inv($q)
  254.961 ns (8 allocations: 768 bytes)

[MilesCranmer]

I fixed the performance regression. Looks like it was just something that the Julia compiler was able to inline on Julia 1.10 but not the earlier versions. I just moved it to a generated function now.

[MilesCranmer]

Thanks for the review! Here are the changes since your last pass over: https://github.com/SymbolicML/DynamicQuantities.jl/compare/3bb945261b0a983d40ad6ea1d8b68b77ef53cecf...union-type

Regarding test coverage, it is good to worry about but I'm not that worried because GenericQuantity should be the exact same as Quantity. The only cases we need to worry about are: (1) where both GenericQuantity and Quantity can be used as inputs, like where there are two AbstractUnionQuantity arguments, and (2) where Quantity is the default for something (we should have a GenericQuantity alternative). I just added another field to ABSTRACT_QUANTITY_TYPES to set the default concrete type in such cases so its easier to loop.

Also, just to be safe, I'm looping over both quantity types in a lot of the unittests now, particularly the basic tests and the QuantityArray tests.

[MilesCranmer]

Btw, should it be UnionAbstractQuantity instead? Because we want to have AbstractXQuantity be subtyped to X, it might have better semantics, since it’s not an abstract type itself.

Also, maybe AbstractAnyQuantity and AnyQuantity instead of Generic - wdyt?

[MilesCranmer]

Okay I changed it to UnionAbstractQuantity. However I'm going to leave the generic one as GenericQuantity since AnyQuantity makes me think it is an Any, which is bad.

Any last comments or do you think it's good to go?

[MilesCranmer]

Thanks for the reviews!!