Skip to content

Simple affine units implementation #168

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 86 commits into from
Mar 16, 2025
Merged

Simple affine units implementation #168

merged 86 commits into from
Mar 16, 2025

Conversation

MilesCranmer
Copy link
Member

@Deduction42

Here is a reduced version of #159 with only the basic functionality. Writing out degC will auto-convert to dimensions when you multiply it with a number:

julia> x = 22ua"degC"
295.15 K

julia> x = 0ua"degC"
273.15 K

julia> 50ua"degC" - 40ua"degF"
45.55555555555554 K

That's basically it. I think this gets the main feature of unit parsing but without needing to have a separate AffineDimensions object that requires a lot of patching throughout the library.

Copy link
Contributor

github-actions bot commented Mar 16, 2025

Benchmark Results

main 9c0f1ab... main / 9c0f1ab...
Quantity/creation/Quantity(x) 3.1 ± 0.01 ns 3.1 ± 0.01 ns 0.997
Quantity/creation/Quantity(x, length=y) 3.43 ± 0.011 ns 3.73 ± 0.01 ns 0.919
Quantity/with_numbers/*real 3.11 ± 0.001 ns 3.11 ± 0.001 ns 1
Quantity/with_numbers/^int 8.68 ± 1.6 ns 8.68 ± 1.6 ns 1
Quantity/with_numbers/^int * real 8.98 ± 1.6 ns 8.99 ± 1.6 ns 0.999
Quantity/with_quantity/+y 4.35 ± 0.001 ns 4.35 ± 0.01 ns 1
Quantity/with_quantity//y 3.11 ± 0.001 ns 3.42 ± 0.001 ns 0.909
Quantity/with_self/dimension 3.11 ± 0.91 ns 2.79 ± 0.001 ns 1.11
Quantity/with_self/inv 3.11 ± 0.001 ns 3.11 ± 0 ns 1
Quantity/with_self/ustrip 3.72 ± 0.011 ns 2.79 ± 0 ns 1.33
QuantityArray/broadcasting/multi_array_of_quantities 0.0873 ± 0.0012 ms 0.0877 ± 0.0017 ms 0.996
QuantityArray/broadcasting/multi_normal_array 0.0528 ± 0.0003 ms 0.0496 ± 0.0019 ms 1.06
QuantityArray/broadcasting/multi_quantity_array 0.0592 ± 0.0061 ms 0.0559 ± 0.0031 ms 1.06
QuantityArray/broadcasting/x^2_array_of_quantities 13.7 ± 1.5 μs 14.9 ± 2.5 μs 0.921
QuantityArray/broadcasting/x^2_normal_array 2.1 ± 0.63 μs 2.35 ± 1.4 μs 0.894
QuantityArray/broadcasting/x^2_quantity_array 6.5 ± 0.31 μs 6.48 ± 0.23 μs 1
QuantityArray/broadcasting/x^4_array_of_quantities 0.0811 ± 0.00089 ms 0.0815 ± 0.0012 ms 0.995
QuantityArray/broadcasting/x^4_normal_array 0.0496 ± 0.0032 ms 0.0466 ± 0.0024 ms 1.07
QuantityArray/broadcasting/x^4_quantity_array 0.059 ± 0.0063 ms 0.0498 ± 0.0092 ms 1.19
time_to_load 0.201 ± 0.0062 s 0.202 ± 0.0042 s 0.995

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

@Deduction42
Copy link
Contributor

What kind of object does ua"degC" return? Does ua"m/s" work too?

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Mar 16, 2025

ua"degC" returns an AffineUnits object. It's outside of the dimensions hierarchy and thus can't be used inside a Quantity. Only when you multiply this against a number does it become a quantity.

ua"m/s" at the moment does in fact work; it triggers a fall-back method that calls uparse.

(Though I was thinking of deleting that, so users would need to be very explicit about getting an AffineUnits object)

@Deduction42
Copy link
Contributor

Deduction42 commented Mar 16, 2025

I'd actually prefer a string macro that produced units, so this looks like a very clever fix. As long 'ua"..."' and 'aff_uparse("...")' produces a consistent object (AffineUnits) and works on any units in the registry, this is probably good enough for most basic use cases. Are you able to convert a quantity back to affine units? I typically only use this worklow:

{customer units}->{si units}->{numbers}->{algorithm}->{si units}->{customer units}

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Mar 16, 2025

There’s no conversion to affine units, only from affine to SI. Right now there’s not even a display in affine units, because you would never use a naked AffineUnits for anything. (Edit: I added a display for the naked units, just because it was a one-line change, I think it's ok)

I feel the reverse from SI to affine + printing might be ok to do in user code, so I don't see a huge need to add it here.

Also, why do you need the other units accessible in ua_str? Since they are incompatible with affine anyways. Why not just do a

try
    uparse(s)
catch
    aff_uparse(s)
end

I don’t know why DQ would need to do this internally.

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Mar 16, 2025

Ok I am pretty happy with this. From 744 to 217 lines compared to #159, and gets the main functionality without the difficulties of treating affine units as regular Quantity. Will merge soon.

Note that I am labelling everything as "experimental" so there's the opportunity to roll things back or make changes if needed.

@MilesCranmer MilesCranmer merged commit 742caab into main Mar 16, 2025
8 checks passed
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.

3 participants