Skip to content

Add uconvert method for QuantityArray #61

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 15, 2023
Merged

Add uconvert method for QuantityArray #61

merged 2 commits into from
Oct 15, 2023

Conversation

MilesCranmer
Copy link
Member

@MilesCranmer MilesCranmer commented Oct 14, 2023

cc @gaurav-arya

(For what it's worth, it seems like using uconvert(us"km").(array) works, but it relies on compiler inlining a lot of stuff to be as efficient. Also, expand_units has a dedicated method, so I think it makes sense to do it here as well.)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2023

Benchmark Results

main 3333cd9... t[main]/t[3333cd9...]
Quantity/creation/Quantity(x) 3.4 ± 0 ns 3.4 ± 0 ns 1
Quantity/creation/Quantity(x, length=y) 3.7 ± 0 ns 3.7 ± 0 ns 1
Quantity/with_numbers/*real 3.4 ± 0 ns 3.4 ± 0 ns 1
Quantity/with_numbers/^int 11.5 ± 4.1 ns 11.5 ± 4.1 ns 1
Quantity/with_numbers/^int * real 11.8 ± 4 ns 12 ± 4 ns 0.983
Quantity/with_quantity/+y 6.8 ± 0.1 ns 6.8 ± 0.1 ns 1
Quantity/with_quantity//y 3.7 ± 0.001 ns 3.7 ± 0.001 ns 1
Quantity/with_self/dimension 1.7 ± 0 ns 1.7 ± 0.001 ns 1
Quantity/with_self/inv 3.4 ± 0 ns 3.4 ± 0.001 ns 1
Quantity/with_self/ustrip 1.7 ± 0 ns 1.7 ± 0 ns 1
QuantityArray/broadcasting/multi_array_of_quantities 0.225 ± 0.007 ms 0.226 ± 0.023 ms 0.999
QuantityArray/broadcasting/multi_normal_array 0.0764 ± 0.0022 ms 0.0761 ± 0.0021 ms 1
QuantityArray/broadcasting/multi_quantity_array 0.255 ± 0.0014 ms 0.254 ± 0.0013 ms 1
QuantityArray/broadcasting/x^2_array_of_quantities 0.0426 ± 0.0045 ms 0.0414 ± 0.0028 ms 1.03
QuantityArray/broadcasting/x^2_normal_array 8.7 ± 1.5 μs 9 ± 1.7 μs 0.967
QuantityArray/broadcasting/x^2_quantity_array 10.2 ± 0.8 μs 10.2 ± 1.4 μs 1
QuantityArray/broadcasting/x^4_array_of_quantities 0.136 ± 0.0074 ms 0.135 ± 0.0058 ms 1.01
QuantityArray/broadcasting/x^4_normal_array 0.0688 ± 0.0014 ms 0.069 ± 0.0013 ms 0.997
QuantityArray/broadcasting/x^4_quantity_array 0.104 ± 0.0018 ms 0.103 ± 0.0021 ms 1.01
time_to_load 0.165 ± 0.0031 s 0.165 ± 0.00031 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).

Copy link
Collaborator

@gaurav-arya gaurav-arya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. One nit: the @assert isone(... is missing for the array method.

@MilesCranmer
Copy link
Member Author

Good catch! Added.

@MilesCranmer MilesCranmer merged commit bfc8efb into main Oct 15, 2023
@MilesCranmer MilesCranmer deleted the uconvert-array branch October 15, 2023 02:13
MilesCranmer added a commit that referenced this pull request Oct 16, 2023
[Diff since v0.7.3](v0.7.3...v0.7.4)

**Merged pull requests:**
- Add `uconvert` and chemistry example (#48) (@gaurav-arya)
- Add `uconvert` method for `QuantityArray` (#61) (@MilesCranmer)
- Deprecate `expand_units` -> `uexpand` (#62) (@MilesCranmer)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants