Skip to content

Propose some functions and macros for unitful integration. #11

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

Closed
wants to merge 3 commits into from

Conversation

j-fu
Copy link
Contributor

@j-fu j-fu commented Jun 8, 2023

In particular this PR provides a q_str macro which accesses u_str to define quantities via the corresponding data in Unitful.

I don't know how to define the macros in the extension, so at this stage it would make Unitful a full dependency. IMHO would be ok, but of course this is open for discussion.

See my proposal in #10 - this PR could be an implementation.

In particular this PR provides a `q_str` macro which accesses
`u_str` to define qunatities via the corresponding data in Unitful.

I don't know how to define the macros in the extension, so
at this stage it would make Unitful a full dependency. IMHO this is ok,
but of course this is open for discussion.
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

Benchmark Results

main 3d99928... t[main]/t[3d99928...]
creation/Quantity(x) 3.4 ± 0.4 ns 3.8 ± 0.3 ns 0.895
creation/Quantity(x, length=y) 4.2 ± 0.4 ns 3.8 ± 0.4 ns 1.11
time_to_load 0.0762 ± 0.0069 s 0.562 ± 0.092 s 0.136
with_numbers/*real 3.8 ± 0.3 ns 3.4 ± 0.3 ns 1.12
with_numbers/^int 0.235 ± 0.026 μs 0.203 ± 0.02 μs 1.16
with_numbers/^int * real 0.236 ± 0.023 μs 0.203 ± 0.02 μs 1.17
with_quantity/+y 7.6 ± 0.6 ns 7.1 ± 0.6 ns 1.07
with_quantity//y 0.262 ± 0.023 μs 0.255 ± 0.022 μs 1.03
with_self/dimension 2 ± 0.1 ns 1.9 ± 0.1 ns 1.05
with_self/inv 9.9 ± 1.8 ns 9.6 ± 0.7 ns 1.03
with_self/ustrip 1.9 ± 0.1 ns 1.9 ± 0.4 ns 1

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

MilesCranmer commented Jun 8, 2023

Thanks for the PR! I am in principle on board with this, just need to consider things a bit more. I am wondering whether it makes sense to make Unitful.jl a hard dependency or not. According to the benchmark above, it bumps the load time to 0.6s. After #9, the load time would go down to 0.07s, so this would be a big increase if we do choose that route.

@oscardssmith thoughts?

I definitely like the idea of exporting a q_str though. I think you could export it from an extension by defining a macro that calls some function _q_str. Then overload that function inside the extension.

If we break the target audience into two groups, I see the following use-cases:

  1. Internal use by package maintainers.
  • They might want to eliminate reliance on Unitful.jl completely, and just manipulate dimensions. Here, perhaps we just want to reduce load time of the package, and simplify things as much as possible.
  • I think this will be the main user base?
  1. End users (people doing scientific calculations/analysis).
  • They probably want to use units.
  • They may want to try DynamicQuantities.jl for faster calculations.
  • Having them install both DynamicQuantities.jl and Unitful.jl (as well as importing both) would make things slightly less user-friendly.

In talking through this, I wonder if it makes the most sense to just have people import both packages, and convert between them. In other words, maybe it makes sense to keep Unitful.jl as an extension, without the q_str macro. Then they can still rely on the Unitful.jl documentation and other unit libraries, rather than treating DynamicQuantities.jl as a front-end API and expecting the relevant docs to be here. What do you think?

@MilesCranmer
Copy link
Member

@odow I'd be interested to hear your thoughts as well

@j-fu
Copy link
Contributor Author

j-fu commented Jun 8, 2023

Interesting considerations!

My opinion here is that Unitful.jl is too rigid to be used by end users (who ase a rule are inexperienced programers and don't see programming as their foremost purpose). km and cm being different types feels IMHO is not very intuitive. I am working with others on a PDE simulation stack for applicationsm and I really would like to provide the possibility to do calculations with units to end users and modelers. With Unitful one needs to strip units before going into the solver (and not forget upreferred - I once tried to get some convenience calls which should have made it easier for end users into Unitful).
For me this ended up in LessUnitful.jl which is just a band aid.

For DyanamicQuantities I indeed see the potential that this package can work well for less experienced end users. I see the potential that quanties can be passed through e.g. a PDE solver if it is well designed. This opens up great possibilities for simulation work, teaching etc.

I see the need for being able to expose end users to quantities with units in an easy to handle consistent way, Julia and its ecosystem certainly have the potential, and DynamicQuanties could make it possible. So indeed I would put them first...

@odow
Copy link
Contributor

odow commented Jun 8, 2023

Personally, I don't think DynamicQuantities should support Unitful or have any conversions. I'd also stay away from helper functions and macros for as long as possible. See if people can get used to the Dimension and Quantity API initially. Don't inflate the API surface right at the start because that locks you into the decision long-term.

@j-fu
Copy link
Contributor Author

j-fu commented Jun 9, 2023

I feel suggesting that everyone writing their own list of units in their codes using the bare metal API provided by the Quantity/Dimension API would violate the DRY mantra.

But in a way this is a matter of taste and the functionality I am proposing could as well be provided by a glue package.

@j-fu
Copy link
Contributor Author

j-fu commented Jun 14, 2023

Ok the wind blew the other direction, so I think we can close this.

1 similar comment
@j-fu
Copy link
Contributor Author

j-fu commented Jun 14, 2023

Ok the wind blew the other direction, so I think we can close this.

@j-fu j-fu closed this Jun 14, 2023
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