Skip to content

Refactor symbolic units module, make all symbols available at import #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 4, 2024

Conversation

MilesCranmer
Copy link
Member

@MilesCranmer MilesCranmer commented Dec 3, 2023

Previously, units were created the first time the user used sym_uparse, because otherwise it required creating ~150 arrays at import inside an @eval. This also means that you could not precompile unit calculations done with symbolic units.

With this change, we now eagerly create symbolic units as immutable objects. For example, h would be created with

Quantity(1.0, SymbolicDimensionsSingleton(:h))

and this would represented in memory as the lightweight (1.0, 85), where 85 is the index of hours in the constant tuple of units and quantities.

  • This creates the abstract type AbstractSymbolicDimensions <: AbstractDimensions and defines several of the symbolic dimensions methods on it.
  • This uses a new type SymbolicDimensionsSingleton that stores a single integer corresponding to the index of the unit/constant in a constant tuple.
  • This also exports modules SymbolicUnits and SymbolicConstants, so you can do things like:
julia> using DynamicQuantities

julia> using DynamicQuantities.SymbolicUnits: km, h

julia> km |> typeof
Quantity{Float64, SymbolicDimensionsSingleton{DynamicQuantities.FixedRational{Int32, 25200}}}

julia> km/h |> typeof
Quantity{Float64, SymbolicDimensions{DynamicQuantities.FixedRational{Int32, 25200}}}

julia> km/h |> uexpand
0.2777777777777778 m s⁻¹

julia> km/h |> uexpand |> uconvert(SymbolicConstants.pc / SymbolicUnits.Myr)
0.00028408671251269307 Myr⁻¹ pc

@devmotion would you be up for reviewing this?


cc @gaurav-arya

Fixes #51 by cadojo (and prevents the need for #58)

@SymbolicML SymbolicML deleted a comment from sweep-ai bot Dec 3, 2023
Copy link
Contributor

github-actions bot commented Dec 3, 2023

Benchmark Results

main 212626f... t[main]/t[212626f...]
Quantity/creation/Quantity(x) 2.79 ± 0.009 ns 3.41 ± 0.01 ns 0.82
Quantity/creation/Quantity(x, length=y) 3.41 ± 0.01 ns 3.11 ± 0.01 ns 1.1
Quantity/with_numbers/*real 3.1 ± 0.01 ns 3.1 ± 0.01 ns 1
Quantity/with_numbers/^int 8.05 ± 1.9 ns 8.05 ± 2.2 ns 1
Quantity/with_numbers/^int * real 8.36 ± 1.9 ns 8.67 ± 2.2 ns 0.964
Quantity/with_quantity/+y 4.04 ± 0.001 ns 4.04 ± 0.001 ns 1
Quantity/with_quantity//y 3.11 ± 0.001 ns 3.42 ± 0.01 ns 0.909
Quantity/with_self/dimension 3.1 ± 0.01 ns 3.1 ± 0.01 ns 1
Quantity/with_self/inv 3.11 ± 0.01 ns 3.11 ± 0.001 ns 1
Quantity/with_self/ustrip 2.79 ± 0.009 ns 2.79 ± 0.01 ns 1
QuantityArray/broadcasting/multi_array_of_quantities 0.149 ± 0.0016 ms 0.145 ± 0.00089 ms 1.03
QuantityArray/broadcasting/multi_normal_array 0.0529 ± 0.00015 ms 0.0528 ± 0.00017 ms 1
QuantityArray/broadcasting/multi_quantity_array 0.158 ± 0.00054 ms 0.155 ± 0.00078 ms 1.02
QuantityArray/broadcasting/x^2_array_of_quantities 22.9 ± 1.5 μs 23.1 ± 1.4 μs 0.992
QuantityArray/broadcasting/x^2_normal_array 4.37 ± 1.3 μs 4.06 ± 1.3 μs 1.08
QuantityArray/broadcasting/x^2_quantity_array 6.95 ± 0.25 μs 6.92 ± 0.24 μs 1
QuantityArray/broadcasting/x^4_array_of_quantities 0.0786 ± 0.00047 ms 0.0785 ± 0.00038 ms 1
QuantityArray/broadcasting/x^4_normal_array 0.0498 ± 0.00016 ms 0.0467 ± 0.00017 ms 1.07
QuantityArray/broadcasting/x^4_quantity_array 0.0499 ± 0.00015 ms 0.0501 ± 0.0031 ms 0.995
time_to_load 0.124 ± 0.00053 s 0.129 ± 0.0036 s 0.956

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
Copy link
Member Author

@gaurav-arya I wonder if there are any type instabilities that could potentially come from this...? Although the promotion rules seem to be doing their job:

julia> [km]
1-element Vector{Quantity{Float64, SymbolicDimensionsSingleton{DynamicQuantities.FixedRational{Int32, 25200}}}}:
 1.0 km

julia> [km, km/h]
2-element Vector{Quantity{Float64, SymbolicDimensions{DynamicQuantities.FixedRational{Int32, 25200}}}}:
     1.0 km
 1.0 km h⁻¹

julia> [km, km/h, uexpand(km)]
3-element Vector{Quantity{Float64, Dimensions{DynamicQuantities.FixedRational{Int32, 25200}}}}:
                 1000.0 m
 0.2777777777777778 m s⁻¹
                 1000.0 m

I guess you could create an instability if you do something like output=km; output /= h, but in that case you would be asking for type instabilities anyways...

@gaurav-arya gaurav-arya self-requested a review December 14, 2023 12:22
@gaurav-arya
Copy link
Collaborator

It'll take me a bit of time since I'm not quite done with school yet, but I'll give this a review:)

@MilesCranmer
Copy link
Member Author

Thanks very much for the suggestions @devmotion. Everything implemented!

@MilesCranmer
Copy link
Member Author

Let me know if there are any other comments! Otherwise I will try to merge this soon.

MilesCranmer and others added 7 commits January 1, 2024 07:19
- Now eagerly creates symbolic units as immutable objects, rather than at first call
- This uses a new type `SymbolicDimensionsSingleton` that stores a single dimension
- Exports `SymbolicUnits` and `SymbolicConstants` for direct imports
@MilesCranmer MilesCranmer enabled auto-merge January 4, 2024 23:28
@MilesCranmer MilesCranmer merged commit 0913ba2 into main Jan 4, 2024
@MilesCranmer MilesCranmer deleted the built-in-symbolic-units branch January 5, 2024 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apparent Issue with Precompilation for Dependent Packages
3 participants