Skip to content

Add some Unitful unit names that were missing #81

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 2 commits into from
Nov 9, 2023

Conversation

MichaelHatherly
Copy link
Contributor

There are a couple of differences between the naming of units in Unitful and this package that makes it not quite a direct swap when changing between the two packages. This adds a couple of aliases for current units as well as a new wk.

The only one that we're really needing here is wk since we can deal with aliasing the others internally where needed, so I'm fine with dropping those if there are reasons that they weren't included already?


How would you like tests written for these kinds of additions? If they are needed?

There's a couple of differences between the naming of units in
`Unitful` and this package that makes it not quite a direct swap when changing between the two packages. This adds a couple of aliases for current units as well as a new `wk`.
Copy link
Contributor

github-actions bot commented Nov 9, 2023

Benchmark Results

main 2178c8d... t[main]/t[2178c8d...]
Quantity/creation/Quantity(x) 3.4 ± 0 ns 3.7 ± 0.1 ns 0.919
Quantity/creation/Quantity(x, length=y) 3.4 ± 0 ns 3.6 ± 0.1 ns 0.944
Quantity/with_numbers/*real 3.1 ± 0.1 ns 3 ± 0.1 ns 1.03
Quantity/with_numbers/^int 12 ± 4.1 ns 11.8 ± 4 ns 1.02
Quantity/with_numbers/^int * real 12.2 ± 4.7 ns 12.1 ± 4.8 ns 1.01
Quantity/with_quantity/+y 7.1 ± 0 ns 7.1 ± 0.1 ns 1
Quantity/with_quantity//y 3.7 ± 0 ns 3.7 ± 0 ns 1
Quantity/with_self/dimension 1.7 ± 0 ns 1.7 ± 0 ns 1
Quantity/with_self/inv 3.5 ± 0.2 ns 3.4 ± 0.2 ns 1.03
Quantity/with_self/ustrip 1.7 ± 0 ns 1.7 ± 0 ns 1
QuantityArray/broadcasting/multi_array_of_quantities 0.233 ± 0.028 ms 0.232 ± 0.03 ms 1.01
QuantityArray/broadcasting/multi_normal_array 0.0772 ± 0.0027 ms 0.0769 ± 0.0026 ms 1
QuantityArray/broadcasting/multi_quantity_array 0.255 ± 0.0017 ms 0.255 ± 0.0016 ms 1
QuantityArray/broadcasting/x^2_array_of_quantities 0.0639 ± 0.017 ms 0.0645 ± 0.016 ms 0.991
QuantityArray/broadcasting/x^2_normal_array 12.5 ± 3.4 μs 12.5 ± 3.7 μs 1
QuantityArray/broadcasting/x^2_quantity_array 13.3 ± 1.3 μs 13.4 ± 1.2 μs 0.993
QuantityArray/broadcasting/x^4_array_of_quantities 0.15 ± 0.0075 ms 0.148 ± 0.0066 ms 1.01
QuantityArray/broadcasting/x^4_normal_array 0.0719 ± 0.0029 ms 0.0708 ± 0.003 ms 1.02
QuantityArray/broadcasting/x^4_quantity_array 0.108 ± 0.0015 ms 0.107 ± 0.0026 ms 1.01
time_to_load 0.214 ± 0.00081 s 0.216 ± 0.0031 s 0.991

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

@jkrumbiegel jkrumbiegel mentioned this pull request Nov 9, 2023
@MilesCranmer
Copy link
Member

Looks good! The one suggestion I have is to mention these units and aliases in this line:

    "Time in seconds. Available variants: `fs`, `ps`, `ns`, `μs` (/`us`), `ms`, `min`, `h` (/`hr`), `day`, `yr`, `kyr`, `Myr`, `Gyr`.",

@MilesCranmer
Copy link
Member

Thanks for the addition 👍

@MilesCranmer MilesCranmer merged commit aa40730 into SymbolicML:main Nov 9, 2023
@MichaelHatherly
Copy link
Contributor Author

Thanks Miles.

@MichaelHatherly MichaelHatherly deleted the mh/unitful-compats branch November 9, 2023 12:46
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.

2 participants