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

Remove SparseArrays dependency #56

Closed devmotion closed 11 months ago

devmotion commented 12 months ago

This PR removes the SparseArrays dependency completely.

Only the implementation of == and +/- required a bit more code, otherwise not too many changes were needed.

Fixes #53.

github-actions[bot] commented 12 months ago

Benchmark Results

main 0bcc13487593d0... t[main]/t[0bcc13487593d0...]
Quantity/creation/Quantity(x) 4 ± 0.2 ns 3 ± 0.1 ns 1.33
Quantity/creation/Quantity(x, length=y) 4.2 ± 0.2 ns 3.9 ± 0.4 ns 1.08
Quantity/with_numbers/*real 3.6 ± 0.5 ns 3.8 ± 0.3 ns 0.947
Quantity/with_numbers/^int 12.3 ± 3.9 ns 11.9 ± 2.7 ns 1.03
Quantity/with_numbers/^int * real 13.3 ± 4 ns 12.5 ± 4.3 ns 1.06
Quantity/with_quantity/+y 6.3 ± 0.3 ns 6 ± 0.6 ns 1.05
Quantity/with_quantity//y 4.7 ± 0.2 ns 4.2 ± 0.4 ns 1.12
Quantity/with_self/dimension 1.8 ± 0.1 ns 1.9 ± 0.3 ns 0.947
Quantity/with_self/inv 4.5 ± 0.2 ns 3.9 ± 0.3 ns 1.15
Quantity/with_self/ustrip 1.9 ± 0.1 ns 1.9 ± 0.1 ns 1
QuantityArray/broadcasting/multi_array_of_quantities 0.351 ± 0.17 ms 0.348 ± 0.16 ms 1.01
QuantityArray/broadcasting/multi_normal_array 0.105 ± 0.022 ms 0.103 ± 0.029 ms 1.02
QuantityArray/broadcasting/multi_quantity_array 0.349 ± 0.041 ms 0.349 ± 0.043 ms 1
QuantityArray/broadcasting/x^2_array_of_quantities 0.0719 ± 0.014 ms 0.0713 ± 0.017 ms 1.01
QuantityArray/broadcasting/x^2_normal_array 12.7 ± 1.2 μs 12.2 ± 1.5 μs 1.04
QuantityArray/broadcasting/x^2_quantity_array 13 ± 1.2 μs 13 ± 10 μs 1
QuantityArray/broadcasting/x^4_array_of_quantities 0.184 ± 0.048 ms 0.185 ± 0.048 ms 0.995
QuantityArray/broadcasting/x^4_normal_array 0.0953 ± 0.018 ms 0.097 ± 0.025 ms 0.982
QuantityArray/broadcasting/x^4_quantity_array 0.119 ± 0.021 ms 0.116 ± 0.026 ms 1.03
time_to_load 0.226 ± 0.0087 s 0.29 ± 0.066 s 0.778

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 12 months ago

Nice work!! Really appreciate your effort on this.

It will take me a bit longer to read through this one but I'll try to get to it soon.

MilesCranmer commented 12 months ago

I'm wondering (maybe I missed it) if there could be some filter! or deleteat! somewhere? Otherwise it seems like the vector of dimensions could keep growing.

e.g., each operation on SymbolicDimensions could end with something like

remove_zeros(v) = filter!(e->!iszero(e), v)

(but adapted to the two vector version).

I'm not sure where in the operations SparseArrays performs this.

Edit: wait, it seems like you do this at the stage of creating a new vector (by simply not pushing zeros), so its fine?

devmotion commented 12 months ago

wait, it seems like you do this at the stage of creating a new vector (by simply not pushing zeros), so its fine?

When performing multiplications and divisions (i.e., addition and subtraction of dimensions) only non-zero dimensions of the result are stored. Similarly, when constructing SymbolicDimensions and when converting from Dimension only non-zero dimensions are stored.

devmotion commented 11 months ago

I added more tests that should improve coverage.

MilesCranmer commented 11 months ago

One more TODO item (I think the last one, we're almost done) is to benchmark QuantityArray broadcasting for SymbolicDimensions. I am curious to see if the compiler inlining on the == changes at all with the custom sparse array implementation. [I will try to do this soon]

MilesCranmer commented 11 months ago

Awesome. I have no idea how, but this PR makes the broadcasting on SymbolicDimensions ~18x as fast as it was before!

Before:

julia> f(v) = v^2 * 1.5;

julia> @btime $f.(qa) setup=(xa = randn(10000) .* us"km/s"; qa = QuantityArray(xa));
  17.795 ms (160019 allocations: 13.05 MiB)

after:

julia> @btime $f.(qa) setup=(xa = randn(10000) .* us"km/s"; qa = QuantityArray(xa));
  1.053 ms (40007 allocations: 2.06 MiB)

(Still several orders of magnitude slower than with normal Dimensions, which has an advantage by being immutable, but I think this is pretty good)


Also, I went ahead and fixed the bug I noted here.

MilesCranmer commented 11 months ago

Hm, I found another bug:

> convert(Quantity{Float64,SymbolicDimensions}, u"kg")
1.0 

not sure why this happens.