Skip to content
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

Units through unitful? #113

Open
aramirezreyes opened this issue Jun 8, 2022 · 2 comments
Open

Units through unitful? #113

aramirezreyes opened this issue Jun 8, 2022 · 2 comments

Comments

@aramirezreyes
Copy link

aramirezreyes commented Jun 8, 2022

Hi!

I am working on a couple packages. In particular this one. I was thinking on extracting my physicsfunctions.jl and adding them to a new package, but I discovered Thermodynamics.jl and now I am thinking on using it instead. One think that I am missing is support for units (in my package I am achieving that through Unitful.jl but this currently does not play nice with Thermodynamics.jl:

julia> using Thermodynamics, Unitful
julia> import CLIMAParameters as CP
julia> struct EarthParameterSet <: CP.AbstractEarthParameterSet end
julia> param_set = EarthParameterSet()
EarthParameterSet()
julia> saturation_vapor_pressure(param_set,300u"K",Liquid())
ERROR: MethodError: no method matching saturation_vapor_pressure(::EarthParameterSet, ::Quantity{Int64, 𝚯, Unitful.FreeUnits{(K,), 𝚯, nothing}}, ::Liquid)
Closest candidates are:

I usually use my functions as this:

julia> using TropicalCyclonePotentialIntensity, Unitful
julia> get_saturation_vapor_pressure(310.0u"K")
62.355321818976755 hPa

And I use units in my tests to ensure consistency and some other things.
Considering that this package is being intended for wider use that only whithin CLIMA.
Is accepting unitful units in the plans for Thermodynamics.jl? Would it be desirable?

@charleskawczynski
Copy link
Member

Hi @aramirezreyes, good question. We put some effort into doing this a while back in ClimateMachine but I think that it didn't quite work for some reason.

We currently enforce pretty strict type diagonalization in Thermodynamics to avoid unwanted type promotions. However, if we fix #99, then we'll need to relax this restriction, and I do like the idea of having units. Perhaps we could do this, but it will require some work / thought.

@charleskawczynski
Copy link
Member

I just realized that a nice alternative/compromise might be to add constructors that accept unitfull inputs, strip the units, compute necessary default constructor args, and re-wrap the default constructor args with the appropriate units.

one issue with this, however, is that we’ll need new types, since unitful stirs units in the type space. Alternatively we could use dynamic quantities (https://github.com/SymbolicML/DynamicQuantities.jl). However, that would require carrying around additional data, which. Perhaps we could do wrapping/unwrapping with that too?

wrapping and unwrapping kind of defeats the purpose of unitful internally, however, adding units generally speaking is not great for performance (or latency).

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

No branches or pull requests

2 participants