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

Intended behavior for dimension errors on SymbolicDimensions #39

Open
MilesCranmer opened this issue Sep 11, 2023 · 0 comments
Open

Intended behavior for dimension errors on SymbolicDimensions #39

MilesCranmer opened this issue Sep 11, 2023 · 0 comments

Comments

@MilesCranmer
Copy link
Member

MilesCranmer commented Sep 11, 2023

I would like to hear the opinions of @odow @trulsf for JuMP and maybe @ChrisRackauckas or @YingboMa for ModelingToolkit.jl.

What would you prefer the behavior to be when you try to add two quantities that have incompatible SymbolicDimensions?

First, recall that SymbolicDimensions is a way to avoid automatically converting to SI base units: For example:
julia> q = 4us"kPa * Constants.c"
4.0 kPa c

julia> sqrt(q)
2.0 kPa¹ᐟ² c¹ᐟ²

Only when you call expand_units would it convert from SymbolicDimensions to Dimensions:

julia> expand_units(q)
1.199169832e12 kg s⁻³

Right now when you try to add two incompatible quantities, the behavior is:

julia> 1us"km/s" + 1us"m/s"
ERROR: DimensionError: 1.0 s⁻¹ km and 1.0 m s⁻¹ have incompatible dimensions

whereas with regular Dimensions, you get:

julia> 1u"km/s" + 1u"m/s"
1001.0 m s⁻¹

Questions for you: is this behavior good as-is? Or should there be some way to automatically convert to SI units when this happens, but stay in SymbolicDimensions (to avoid type instability)?


Another option would be to perform all computations with Dimensions, and then display by converting back to SymbolicDimensions. For example:

julia> output_units = us"km/hr";

julia> a = 15us"km/hr"; b = 30us"m/s";

julia> raw_output = expand_units(a) + expand_units(b)
34.166666666666664 m s⁻¹

julia> as_units(q, units) = Quantity(ustrip(q / expand_units(units)) * ustrip(units), dimension(units));

julia> as_units(raw_output, output_units)
122.99999999999999 km hr⁻¹
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

1 participant