Closed MilesCranmer closed 10 months ago
main | 0609463b8f1863... | t[main]/t[0609463b8f1863...] | |
---|---|---|---|
Quantity/creation/Quantity(x) | 3.1 ± 0.01 ns | 2.79 ± 0.01 ns | 1.11 |
Quantity/creation/Quantity(x, length=y) | 3.11 ± 0.01 ns | 3.11 ± 0.01 ns | 1 |
Quantity/with_numbers/*real | 3.1 ± 0.01 ns | 3.1 ± 0.01 ns | 1 |
Quantity/with_numbers/^int | 8.37 ± 2.5 ns | 8.05 ± 2.2 ns | 1.04 |
Quantity/with_numbers/^int * real | 8.37 ± 2.5 ns | 8.05 ± 2.2 ns | 1.04 |
Quantity/with_quantity/+y | 5.27 ± 0.01 ns | 5.27 ± 0.01 ns | 1 |
Quantity/with_quantity//y | 3.11 ± 0.001 ns | 3.41 ± 0.01 ns | 0.912 |
Quantity/with_self/dimension | 1.55 ± 0.01 ns | 1.55 ± 0.01 ns | 1 |
Quantity/with_self/inv | 3.11 ± 0.001 ns | 3.11 ± 0.01 ns | 1 |
Quantity/with_self/ustrip | 1.55 ± 0.01 ns | 1.56 ± 0.01 ns | 0.994 |
QuantityArray/broadcasting/multi_array_of_quantities | 0.147 ± 0.016 ms | 0.15 ± 0.14 ms | 0.985 |
QuantityArray/broadcasting/multi_normal_array | 0.0499 ± 0.003 ms | 0.053 ± 0.00026 ms | 0.941 |
QuantityArray/broadcasting/multi_quantity_array | 0.158 ± 0.00092 ms | 0.158 ± 0.00072 ms | 0.999 |
QuantityArray/broadcasting/x^2_array_of_quantities | 26.5 ± 3.3 μs | 27.2 ± 1.9 μs | 0.976 |
QuantityArray/broadcasting/x^2_normal_array | 4.35 ± 0.85 μs | 5.19 ± 0.83 μs | 0.838 |
QuantityArray/broadcasting/x^2_quantity_array | 6.97 ± 0.27 μs | 6.03 ± 0.4 μs | 1.16 |
QuantityArray/broadcasting/x^4_array_of_quantities | 0.0788 ± 0.00078 ms | 0.0787 ± 0.00055 ms | 1 |
QuantityArray/broadcasting/x^4_normal_array | 0.0498 ± 0.00015 ms | 0.0498 ± 0.00018 ms | 0.998 |
QuantityArray/broadcasting/x^4_quantity_array | 0.0592 ± 0.00016 ms | 0.0593 ± 0.00019 ms | 0.998 |
time_to_load | 0.162 ± 0.006 s | 0.238 ± 0.0058 s | 0.681 |
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).
Actually I think we shouldn't make all default quantities Real
. We can just have these as quantities that someone can use if they want that particular dispatch.
@gaurav-arya I'm wondering if we should just make Quantity <: Real
, and NumberQuantity <: Number
. Having the default numeric type be RealQuantity
rather than Quantity
could be a bit awkward...
Actually I think we shouldn't make all default quantities Real. We can just have these as quantities that someone can use if they want that particular dispatch.
Why not choose the most specific type given the value type? E.g. 1.0 * u"m"
would be <: Real
but (1.0 + 1.0 * im) * u"m"
would be <: Number
for now, etc. Is there a disadvantage, i.e. a reason the user would have preferred Quantity{<:Real}
as opposed to RealQuantity{<:Real}
?
In this scenario, I wonder if we even need a single DEFAULT_QUANTITY_TYPE
? Instead, we have a default mapping from value type -> quantity type?
Why not choose the most specific type given the value type? E.g.
1.0 * u"m"
would be<: Real
but(1.0 + 1.0 * im) * u"m"
would be<: Number
for now, etc. Is there a disadvantage, i.e. a reason the user would have preferredQuantity{<:Real}
as opposed toRealQuantity{<:Real}
?
Yeah good point. The current behavior in this PR is actually (1.0 * u"m")::Real
and ((1.0 + 1.0 * im) * u"m")::Number
already. But I'm wondering if we should change this...
In this scenario, I wonder if we even need a single
DEFAULT_QUANTITY_TYPE
? Instead, we have a default mapping from value type -> quantity type?
Maybe do you mean like #66? Although I don't think we necessarily need an AbstractFloatQuantity
yet, unless a specific need arises, since it will increase load time. But AbstractRealQuantity
seems necessary for many packages.
Cool, coverage is now back to 100%. Feel free to take a look when you want. I'll keep RealQuantity
as the default, but am still open to changing the name to be Quantity
and NumberQuantity
rather than RealQuantity
and Quantity
, respectively.
Also:
Why not choose the most specific type given the value type? E.g.
1.0 * u"m"
would be<: Real
Is this semantically correct? One could indeed argue that 1.0 meters is a type of number... But is it a real?
Let me give an example. Would you agree with the statement
$(0.5 \text{ km} \text{ s}^{-1}) \in \mathbb{R}$
? I think it kind of seems incorrect... It's more like:
$(0.5 \text{ km} \text{ s}^{-1}) \in \mathbb{R} \times \mathbb{U}$
for some abstract space $\mathbb{U}$ of physical dimensions. In which case it might not be the best option to set the default type as ::Real
.
Whereas Number
is much more abstract and I could totally say that a physical quantity is a type of number. i.e., $\mathbb{R} \times \mathbb{U}$ is a space of "numbers."
This semantic ambiguity leads me to want to keep Quantity <: Number
as the default, but allow RealQuantity
as a wrapper for users who know what they are doing. Wdyt?
Thanks!
This adds a new
AbstractRealQuantity <: Real
and a concrete typeRealQuantity
. Users can wrap quantities withRealQuantity(0.3u"km/s")
to pass to functions that require real input. But the default would still beQuantity <: Number
for reasons discussed here.I also introduce a new function
promote_quantity
which one can overload as a sort of custom type hierarchy, so thatRealQuantity -> Quantity
if it interacts with a non-real, and{RealQuantity,Quantity} -> GenericQuantity
if they interact with non-numeric.However this is a bit of a mess right now due to promotion complexities, makingFixed!(0.3+2im)u"km/s"
promote to aComplex{<:Quantity}
instead of aQuantity
... Anybody interested in having a go?