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

the new version (1.22) broke interactions with IntervalArithmetic #758

Open
aplavin opened this issue Jan 4, 2025 · 3 comments
Open

the new version (1.22) broke interactions with IntervalArithmetic #758

aplavin opened this issue Jan 4, 2025 · 3 comments

Comments

@aplavin
Copy link
Contributor

aplavin commented Jan 4, 2025

Unitful 1.21.1:

julia> using IntervalArithmetic, Unitful

julia> y = interval(1, 2)u"°"
[1.0, 2.0]_com°

julia> exp(y)
[1.0176, 1.03553]_com_NG

Unitful 1.22.0:

julia> using IntervalArithmetic, Unitful

julia> y = interval(1, 2)u"°"
[1.0, 2.0]_com°

julia> exp(y)
ERROR: ArgumentError: only irrationals from MathConstants are supported
Stacktrace:
  [1] _round(::Type{Float64}, ::Unitful.UnitConversionFactor{Float64}, ::RoundingMode{:Down})
    @ IntervalArithmetic ~/.julia/packages/IntervalArithmetic/GSwKs/src/intervals/construction.jl:120

From my cursory understanding, uconvert now involves multiplication with a UnitConversionFactor object, that is surpsisingly marked as Irrational here.

@sostock
Copy link
Collaborator

sostock commented Jan 6, 2025

This was introduced in #754 in order to preserve the floating-point precision when converting units (so that a Float32-based quantity would not turn into a Float64-based quantity on conversion because the conversion factor is a Float64). Since picking the correct floating-point precision is the purpose of the AbstractIrrational type, it was used for this. It made the implementation very convenient and I assumed it would work correctly with custom number types (since they probably support multiplying by irrationals like π). I tested some packages to make sure it does not break anything, but I didn’t test IntervalArithmetic.

I will investigate how to fix this. If the behavior cannot easily be preserved without breaking IntervalArithmetic, we will have to implement the other approach in #754 (comment) where packages that define their own number types must opt-in to the new behavior.

@sostock
Copy link
Collaborator

sostock commented Jan 20, 2025

My current plan is to make the UnitConversionFactor type <: Real instead of <: AbstractIrrational. This will hopefully work, we just have to implement a bunch of methods (so that it promotes like an AbstractIrrational but otherwise behaves just like the underlying number). I will test more packages (especially ones that have specialized AbstractIrrational behavior) so that it (hopefully) doesn't break anything this time.

@eliascarv FYI

@eliascarv
Copy link
Contributor

I think that is a good idea @sostock. However, the Number or Real API is not well documented.
Maybe implementing the same methods implemented by the AbstractIrrational type cold be a viable option.

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

3 participants