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

Warn user about precompilation of symbolic units #58

Closed MilesCranmer closed 8 months ago

MilesCranmer commented 11 months ago

As noted in #51 by @cadojo, SymbolicDimensions parsing is not compatible with precompilation because the unit and constant variables are only created at runtime. (The reason for this is that it can take about a second to generate all the symbols. So, to keep startup time small for performant libraries, the symbolic dimensions are generated at runtime.)

This PR:

  1. Prevents the creation of the unit symbols/constants if the user happens to trigger this, by exiting _generate_unit_symbols is precompilation is active.
  2. Prints a helpful warning to the user:

Creating SymbolicDimensions objects during precompilation is not allowed, as symbolic units are not created until the first runtime call. You should use regular Dimensions instead for computations, and generally use SymbolicDimensions for display purposes. If needed, you can manually create SymbolicDimensions by calling the constructor explicitly, such as Quantity(1.5, SymbolicDimensions; km=1, s=-1), which is equivalent to 1.5us"km/s".

What do you think @cadojo?

cc @gaurav-arya

github-actions[bot] commented 11 months ago

Benchmark Results

main 85203629883240... t[main]/t[85203629883240...]
Quantity/creation/Quantity(x) 3.1 ± 0.01 ns 3.1 ± 0.01 ns 1
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.05 ± 1.9 ns 8.05 ± 1.9 ns 1
Quantity/with_numbers/^int * real 8.05 ± 1.9 ns 8.05 ± 1.8 ns 1
Quantity/with_quantity/+y 5.27 ± 0.01 ns 5.27 ± 0.01 ns 1
Quantity/with_quantity//y 3.42 ± 0.01 ns 3.42 ± 0.01 ns 1
Quantity/with_self/dimension 1.55 ± 0.01 ns 1.55 ± 0.01 ns 1
Quantity/with_self/inv 3.11 ± 0.01 ns 3.11 ± 0.01 ns 1
Quantity/with_self/ustrip 1.56 ± 0.01 ns 1.55 ± 0.01 ns 1.01
QuantityArray/broadcasting/multi_array_of_quantities 0.148 ± 0.0046 ms 0.148 ± 0.0068 ms 1
QuantityArray/broadcasting/multi_normal_array 0.0472 ± 0.00023 ms 0.0473 ± 0.0047 ms 0.998
QuantityArray/broadcasting/multi_quantity_array 0.159 ± 0.00067 ms 0.158 ± 0.00056 ms 1
QuantityArray/broadcasting/x^2_array_of_quantities 27 ± 2.4 μs 28.4 ± 3.1 μs 0.952
QuantityArray/broadcasting/x^2_normal_array 5.35 ± 0.93 μs 4.94 ± 0.71 μs 1.08
QuantityArray/broadcasting/x^2_quantity_array 5.98 ± 0.34 μs 6.27 ± 0.33 μs 0.954
QuantityArray/broadcasting/x^4_array_of_quantities 0.0819 ± 0.00066 ms 0.0819 ± 0.00086 ms 0.999
QuantityArray/broadcasting/x^4_normal_array 0.0437 ± 0.00021 ms 0.0437 ± 0.00016 ms 1
QuantityArray/broadcasting/x^4_quantity_array 0.059 ± 0.003 ms 0.0562 ± 0.00021 ms 1.05
time_to_load 0.162 ± 0.0051 s 0.162 ± 0.00099 s 0.998

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).

cadojo commented 11 months ago

What do you think @cadojo?

Looks good to me! Thanks for tagging me this patch. I hadn't considered the manual constructer either.

MilesCranmer commented 8 months ago

This is no longer needed because the feature has been implemented! 😄 #101

But u"..." will still fail during precompilation until #106 is merged.