SymbolicML / DynamicQuantities.jl

Efficient and type-stable physical quantities in Julia
https://symbolicml.org/DynamicQuantities.jl/dev/
Apache License 2.0
132 stars 17 forks source link

Add many other numeric methods #78

Closed MilesCranmer closed 10 months ago

MilesCranmer commented 11 months ago

This adds methods for the following functions. Most of these will throw an error if the quantity has nonzero dimensions. However some of the functions allow nonzero dimensions, and simply return an output with the same dimensions.

It also moves methods over to math.jl for:


TODO:

github-actions[bot] commented 11 months ago

Benchmark Results

main 40521a155e67d3... t[main]/t[40521a155e67d3...]
Quantity/creation/Quantity(x) 3.1 ± 0.01 ns 2.79 ± 0.01 ns 1.11
Quantity/creation/Quantity(x, length=y) 4.02 ± 0.011 ns 3.11 ± 0.01 ns 1.29
Quantity/with_numbers/*real 3.1 ± 0.01 ns 3.1 ± 0.01 ns 1
Quantity/with_numbers/^int 8.05 ± 1.9 ns 7.75 ± 1.8 ns 1.04
Quantity/with_numbers/^int * real 8.05 ± 1.9 ns 8.65 ± 1.9 ns 0.932
Quantity/with_quantity/+y 5.27 ± 0.01 ns 5.27 ± 0.01 ns 1
Quantity/with_quantity//y 3.42 ± 0.011 ns 3.42 ± 0.01 ns 1
Quantity/with_self/dimension 1.55 ± 0.01 ns 1.56 ± 0.01 ns 0.994
Quantity/with_self/inv 3.11 ± 0.01 ns 3.11 ± 0.01 ns 1
Quantity/with_self/ustrip 1.55 ± 0.01 ns 1.55 ± 0.01 ns 1
QuantityArray/broadcasting/multi_array_of_quantities 0.148 ± 0.012 ms 0.149 ± 0.014 ms 0.994
QuantityArray/broadcasting/multi_normal_array 0.0471 ± 0.00023 ms 0.0498 ± 0.00031 ms 0.946
QuantityArray/broadcasting/multi_quantity_array 0.158 ± 0.00058 ms 0.16 ± 0.0014 ms 0.99
QuantityArray/broadcasting/x^2_array_of_quantities 25.5 ± 2.1 μs 25.4 ± 2.5 μs 1
QuantityArray/broadcasting/x^2_normal_array 4.26 ± 0.82 μs 4.4 ± 0.96 μs 0.968
QuantityArray/broadcasting/x^2_quantity_array 5.98 ± 0.33 μs 5.93 ± 0.33 μs 1.01
QuantityArray/broadcasting/x^4_array_of_quantities 0.0816 ± 0.00052 ms 0.0785 ± 0.00068 ms 1.04
QuantityArray/broadcasting/x^4_normal_array 0.0436 ± 0.00017 ms 0.0467 ± 0.00017 ms 0.934
QuantityArray/broadcasting/x^4_quantity_array 0.0561 ± 0.00017 ms 0.0562 ± 0.0061 ms 0.999
time_to_load 0.161 ± 0.00063 s 0.161 ± 0.00075 s 1

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 commented 11 months ago

FYI the errors seem to be coming from this issue: https://github.com/JuliaPhysics/Measurements.jl/issues/162 so not related to this PR at all.

MilesCranmer commented 11 months ago

I think it's okay because one is supposed to return the exact same type, but a lot of these numerical operators are not required nor expected to. For example, your function f(q) would also be type unstable for Int64 input, as it would get converted to a float output for exp. But one's purpose is to return the exact same type representing a 1.

MilesCranmer commented 11 months ago

I guess the downside is that if someone did stuff like

x = 0.9u"km/s"
y = [x, x / x, exp(x / x * 0.3)]

it would be Vector{Number}. Which is bad and feels like it approaches Unitful type instabilities again...

So maybe you are right and we should just always keep to the original quantity type... to absolutely guarantee type stability.

MilesCranmer commented 11 months ago

I think I am 50/50 on this... So I'd welcome any opinions one way or the other.

MilesCranmer commented 11 months ago

it would be Vector{Number}. Which is bad and feels like it approaches Unitful type instabilities again...

I guess we could also fix this by simply defining a promotion rule for all the primitive numeric types in https://docs.julialang.org/en/v1/manual/integers-and-floating-point-numbers/. (Recalling that the reason I avoided a generic Number promotion rule is that it might conflict with other abstract types that are also <: Number)

gaurav-arya commented 11 months ago

I do feel like [3.0, 1u"m"] should promote in any case, yes.

gaurav-arya commented 11 months ago

My overall opinion is that the scalar-output approach for exp and friends seems very appealing, and I think we should take it unless we/others find an insurmountable obstable, e.g. type instability in a reasonable use case with no other fix.

MilesCranmer commented 11 months ago

Okay we could rebase this on #84 if you are on board with that change

MilesCranmer commented 11 months ago

My overall opinion is that the scalar-output approach for exp and friends seems very appealing, and I think we should take it unless we/others find an insurmountable obstable, e.g. type instability in a reasonable use case with no other fix.

Sounds good!