From 13aae793af71e7317f48e1da756c4519d2ac06d5 Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Thu, 5 Dec 2024 11:11:14 -0300 Subject: [PATCH 01/11] Avoid converting the float type of quantities --- src/conversion.jl | 15 ++++++++++++++- test/runtests.jl | 6 ++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/conversion.jl b/src/conversion.jl index 8a32ac63..50729762 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -46,6 +46,19 @@ Returns 1. (Avoids effort when unnecessary.) """ convfact(s::Units{S}, t::Units{S}) where {S} = 1 +""" + convfact(T::Type, s::Units{S}, t::Units{S}) +Returns a appropriate conversion factor from unit `t` to unit `s` for number type `T`. +""" +function convfact(::Type{T}, s::Units, t::Units) where {T<:Number} + cf = convfact(s, t) + if cf isa AbstractFloat + convert(float(real(T)), cf) + else + cf + end +end + """ uconvert(a::Units, x::Quantity{T,D,U}) where {T,D,U} Convert a [`Unitful.Quantity`](@ref) to different units. The conversion will @@ -69,7 +82,7 @@ function uconvert(a::Units, x::Quantity{T,D,U}) where {T,D,U} elseif (a isa AffineUnits) || (x isa AffineQuantity) return uconvert_affine(a, x) else - return Quantity(x.val * convfact(a, U()), a) + return Quantity(x.val * convfact(T, a, U()), a) end end diff --git a/test/runtests.jl b/test/runtests.jl index cfc39070..ce33732d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -228,6 +228,12 @@ end # Issue 647: @test uconvert(u"kb^1000", 1u"kb^1001 * b^-1") === 1000u"kb^1000" @test uconvert(u"kOe^1000", 1u"kOe^1001 * Oe^-1") === 1000u"kOe^1000" + # Issue 753: + # avoid converting the float type of quantities + @test Unitful.numtype(uconvert(m, 100f0cm)) === Float32 + @test Unitful.numtype(uconvert(cm, (1f0π + im) * m)) === ComplexF32 + @test Unitful.numtype(uconvert(rad, 360f0°)) === Float32 + @test Unitful.numtype(uconvert(°, (2f0π + im) * rad)) === ComplexF32 # Floating point overflow/underflow in uconvert can happen if the # conversion factor is large, because uconvert does not cancel # common basefactors (or just for really large exponents and/or From d1605dc67ea97bcae0cdcc4f3455879e64ea896c Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Thu, 5 Dec 2024 13:29:12 -0300 Subject: [PATCH 02/11] Fix typo --- src/conversion.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conversion.jl b/src/conversion.jl index 50729762..e968e89d 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -47,8 +47,8 @@ Returns 1. (Avoids effort when unnecessary.) convfact(s::Units{S}, t::Units{S}) where {S} = 1 """ - convfact(T::Type, s::Units{S}, t::Units{S}) -Returns a appropriate conversion factor from unit `t` to unit `s` for number type `T`. + convfact(T::Type, s::Units, t::Units) +Returns the appropriate conversion factor from unit `t` to unit `s` for the number type `T`. """ function convfact(::Type{T}, s::Units, t::Units) where {T<:Number} cf = convfact(s, t) From 90596204935d1ee816f1b1106e8bcec4682aafcd Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Tue, 10 Dec 2024 14:50:35 -0300 Subject: [PATCH 03/11] Apply suggestions --- src/conversion.jl | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/conversion.jl b/src/conversion.jl index e968e89d..86702b79 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -1,3 +1,18 @@ +""" + struct UnitConversionFactor{x} <: AbstractIrrational end +Conversion factor with value `x`. + +Used by the [`convfact`](@ref) function in floating point conversion factors +to preserve the precision of quantities. +""" +struct UnitConversionFactor{x} <: AbstractIrrational end + +Base.:(==)(::UnitConversionFactor{a}, ::UnitConversionFactor{b}) where {a, b} = a == b +Base.hash(x::UnitConversionFactor, h::UInt) = 3 * objectid(x) - h +Base.BigFloat(::UnitConversionFactor{x}) where {x} = BigFloat(x) +Base.Float64(::UnitConversionFactor{x}) where {x} = Float64(x) +Base.Float32(::UnitConversionFactor{x}) where {x} = Float32(x) + """ convfact(s::Units, t::Units) Find the conversion factor from unit `t` to unit `s`, e.g., `convfact(m, cm) == 1//100`. @@ -30,13 +45,14 @@ Find the conversion factor from unit `t` to unit `s`, e.g., `convfact(m, cm) == ex = numerator(ex) end - result = (inex ≈ 1.0 ? 1 : inex) * ex - if fp_overflow_underflow(inex_orig, result) + ex = (inex ≈ 1.0 ? 1 : inex) * ex + if fp_overflow_underflow(inex_orig, ex) throw(ArgumentError( "Floating point overflow/underflow, probably due to large " * "exponents and/or SI prefixes in units" )) end + result = ex isa AbstractFloat ? UnitConversionFactor{ex}() : ex return :($result) end @@ -46,19 +62,6 @@ Returns 1. (Avoids effort when unnecessary.) """ convfact(s::Units{S}, t::Units{S}) where {S} = 1 -""" - convfact(T::Type, s::Units, t::Units) -Returns the appropriate conversion factor from unit `t` to unit `s` for the number type `T`. -""" -function convfact(::Type{T}, s::Units, t::Units) where {T<:Number} - cf = convfact(s, t) - if cf isa AbstractFloat - convert(float(real(T)), cf) - else - cf - end -end - """ uconvert(a::Units, x::Quantity{T,D,U}) where {T,D,U} Convert a [`Unitful.Quantity`](@ref) to different units. The conversion will @@ -82,7 +85,7 @@ function uconvert(a::Units, x::Quantity{T,D,U}) where {T,D,U} elseif (a isa AffineUnits) || (x isa AffineQuantity) return uconvert_affine(a, x) else - return Quantity(x.val * convfact(T, a, U()), a) + return Quantity(x.val * convfact(a, U()), a) end end From a20b45a1f84da8c59b36a0eb744440fde58584b7 Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Wed, 11 Dec 2024 09:02:44 -0300 Subject: [PATCH 04/11] Apply suggestions --- src/conversion.jl | 23 ++++++++++++----------- test/runtests.jl | 12 ++++++++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/conversion.jl b/src/conversion.jl index 86702b79..2e2d7b49 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -1,17 +1,19 @@ """ - struct UnitConversionFactor{x} <: AbstractIrrational end + UnitConversionFactor(x::AbstractFloat) Conversion factor with value `x`. Used by the [`convfact`](@ref) function in floating point conversion factors to preserve the precision of quantities. """ -struct UnitConversionFactor{x} <: AbstractIrrational end +struct UnitConversionFactor{T<:AbstractFloat} <: AbstractIrrational + x::T +end -Base.:(==)(::UnitConversionFactor{a}, ::UnitConversionFactor{b}) where {a, b} = a == b -Base.hash(x::UnitConversionFactor, h::UInt) = 3 * objectid(x) - h -Base.BigFloat(::UnitConversionFactor{x}) where {x} = BigFloat(x) -Base.Float64(::UnitConversionFactor{x}) where {x} = Float64(x) -Base.Float32(::UnitConversionFactor{x}) where {x} = Float32(x) +Base.:(==)(a::UnitConversionFactor, b::UnitConversionFactor) = a.x == b.x +Base.hash(x::UnitConversionFactor, h::UInt) = hash(x.x, h) +Base.BigFloat(x::UnitConversionFactor) = BigFloat(x.x) +Base.Float64(x::UnitConversionFactor) = Float64(x.x) +Base.Float32(x::UnitConversionFactor) = Float32(x.x) """ convfact(s::Units, t::Units) @@ -45,15 +47,14 @@ Find the conversion factor from unit `t` to unit `s`, e.g., `convfact(m, cm) == ex = numerator(ex) end - ex = (inex ≈ 1.0 ? 1 : inex) * ex - if fp_overflow_underflow(inex_orig, ex) + result = (inex ≈ 1.0 ? 1 : inex) * ex + if fp_overflow_underflow(inex_orig, result) throw(ArgumentError( "Floating point overflow/underflow, probably due to large " * "exponents and/or SI prefixes in units" )) end - result = ex isa AbstractFloat ? UnitConversionFactor{ex}() : ex - return :($result) + return :($(result isa AbstractFloat ? UnitConversionFactor(result) : result)) end """ diff --git a/test/runtests.jl b/test/runtests.jl index ce33732d..ddd9d3f2 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -230,10 +230,22 @@ end @test uconvert(u"kOe^1000", 1u"kOe^1001 * Oe^-1") === 1000u"kOe^1000" # Issue 753: # avoid converting the float type of quantities + @test Unitful.numtype(uconvert(m, BigFloat(100)cm)) === BigFloat + @test Unitful.numtype(uconvert(cm, (BigFloat(1)π + im) * m)) === Complex{BigFloat} + @test Unitful.numtype(uconvert(rad, BigFloat(360)°)) === BigFloat + @test Unitful.numtype(uconvert(°, (BigFloat(2)π + im) * rad)) === Complex{BigFloat} + @test Unitful.numtype(uconvert(m, 100.0cm)) === Float64 + @test Unitful.numtype(uconvert(cm, (1.0π + im) * m)) === ComplexF64 + @test Unitful.numtype(uconvert(rad, 360.0°)) === Float64 + @test Unitful.numtype(uconvert(°, (2.0π + im) * rad)) === ComplexF64 @test Unitful.numtype(uconvert(m, 100f0cm)) === Float32 @test Unitful.numtype(uconvert(cm, (1f0π + im) * m)) === ComplexF32 @test Unitful.numtype(uconvert(rad, 360f0°)) === Float32 @test Unitful.numtype(uconvert(°, (2f0π + im) * rad)) === ComplexF32 + @test Unitful.numtype(uconvert(m, Float16(100)cm)) === Float16 + @test Unitful.numtype(uconvert(cm, (Float16(1)π + im) * m)) === ComplexF16 + @test Unitful.numtype(uconvert(rad, Float16(360)°)) === Float16 + @test Unitful.numtype(uconvert(°, (Float16(2)π + im) * rad)) === ComplexF16 # Floating point overflow/underflow in uconvert can happen if the # conversion factor is large, because uconvert does not cancel # common basefactors (or just for really large exponents and/or From e156e51e91f7baababa16fb62d3641a57f556529 Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Wed, 11 Dec 2024 09:15:12 -0300 Subject: [PATCH 05/11] Fix Ambiguity tests --- src/conversion.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conversion.jl b/src/conversion.jl index 2e2d7b49..7716eeda 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -7,6 +7,7 @@ to preserve the precision of quantities. """ struct UnitConversionFactor{T<:AbstractFloat} <: AbstractIrrational x::T + UnitConversionFactor(x::T) where {T<:AbstractFloat} = new{T}(x) end Base.:(==)(a::UnitConversionFactor, b::UnitConversionFactor) = a.x == b.x From d4e1e428ffbae87759a657dc5532351f7c75b090 Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Wed, 11 Dec 2024 10:27:59 -0300 Subject: [PATCH 06/11] Apply suggestions --- src/conversion.jl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/conversion.jl b/src/conversion.jl index 7716eeda..8acb5eed 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -1,13 +1,12 @@ """ - UnitConversionFactor(x::AbstractFloat) + UnitConversionFactor(x::Float64) Conversion factor with value `x`. Used by the [`convfact`](@ref) function in floating point conversion factors to preserve the precision of quantities. """ -struct UnitConversionFactor{T<:AbstractFloat} <: AbstractIrrational - x::T - UnitConversionFactor(x::T) where {T<:AbstractFloat} = new{T}(x) +struct UnitConversionFactor <: AbstractIrrational + x::Float64 end Base.:(==)(a::UnitConversionFactor, b::UnitConversionFactor) = a.x == b.x From de56f8c2f7124ef4c03d69b147ef8b429f4c3195 Mon Sep 17 00:00:00 2001 From: Elias Carvalho <73039601+eliascarv@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:16:37 -0300 Subject: [PATCH 07/11] Update src/conversion.jl --- src/conversion.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conversion.jl b/src/conversion.jl index 8acb5eed..fb428d51 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -54,7 +54,7 @@ Find the conversion factor from unit `t` to unit `s`, e.g., `convfact(m, cm) == "exponents and/or SI prefixes in units" )) end - return :($(result isa AbstractFloat ? UnitConversionFactor(result) : result)) + return result isa AbstractFloat ? UnitConversionFactor(result) : result end """ From dc04b9bef07e0b4ffdafe5e91832641530eed408 Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Wed, 11 Dec 2024 13:19:41 -0300 Subject: [PATCH 08/11] Update UnitConversionFactor definition --- src/conversion.jl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/conversion.jl b/src/conversion.jl index fb428d51..72f195cb 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -1,12 +1,14 @@ """ - UnitConversionFactor(x::Float64) + UnitConversionFactor(x::T) where {T<:AbstractFloat} Conversion factor with value `x`. Used by the [`convfact`](@ref) function in floating point conversion factors to preserve the precision of quantities. """ -struct UnitConversionFactor <: AbstractIrrational - x::Float64 +struct UnitConversionFactor{T<:AbstractFloat} <: AbstractIrrational + x::T + # the inner constructor necessary for ambiguity resolution + UnitConversionFactor(x::T) where {T<:AbstractFloat} = new{T}(x) end Base.:(==)(a::UnitConversionFactor, b::UnitConversionFactor) = a.x == b.x From bd81b5772b60a09db66600f1caeff6b9181f021e Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Wed, 11 Dec 2024 13:33:26 -0300 Subject: [PATCH 09/11] Update comments --- test/runtests.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index ddd9d3f2..704af71d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -229,7 +229,7 @@ end @test uconvert(u"kb^1000", 1u"kb^1001 * b^-1") === 1000u"kb^1000" @test uconvert(u"kOe^1000", 1u"kOe^1001 * Oe^-1") === 1000u"kOe^1000" # Issue 753: - # avoid converting the float type of quantities + # preserve the floating point precision of quantities @test Unitful.numtype(uconvert(m, BigFloat(100)cm)) === BigFloat @test Unitful.numtype(uconvert(cm, (BigFloat(1)π + im) * m)) === Complex{BigFloat} @test Unitful.numtype(uconvert(rad, BigFloat(360)°)) === BigFloat From 8becfd6bf5cb5819f7daae590cee03ee7348df10 Mon Sep 17 00:00:00 2001 From: Elias Carvalho Date: Wed, 11 Dec 2024 13:36:01 -0300 Subject: [PATCH 10/11] Update docstring --- src/conversion.jl | 4 ++-- test/runtests.jl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conversion.jl b/src/conversion.jl index 72f195cb..26534653 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -2,8 +2,8 @@ UnitConversionFactor(x::T) where {T<:AbstractFloat} Conversion factor with value `x`. -Used by the [`convfact`](@ref) function in floating point conversion factors -to preserve the precision of quantities. +Used by the [`convfact`](@ref) function to preserve +the floating-point precision of quantities. """ struct UnitConversionFactor{T<:AbstractFloat} <: AbstractIrrational x::T diff --git a/test/runtests.jl b/test/runtests.jl index 704af71d..dd32a39d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -229,7 +229,7 @@ end @test uconvert(u"kb^1000", 1u"kb^1001 * b^-1") === 1000u"kb^1000" @test uconvert(u"kOe^1000", 1u"kOe^1001 * Oe^-1") === 1000u"kOe^1000" # Issue 753: - # preserve the floating point precision of quantities + # preserve the floating-point precision of quantities @test Unitful.numtype(uconvert(m, BigFloat(100)cm)) === BigFloat @test Unitful.numtype(uconvert(cm, (BigFloat(1)π + im) * m)) === Complex{BigFloat} @test Unitful.numtype(uconvert(rad, BigFloat(360)°)) === BigFloat From 89da4a78d1af50c9401f3ed7022bab2e931d9d31 Mon Sep 17 00:00:00 2001 From: Elias Carvalho <73039601+eliascarv@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:00:30 -0300 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com> --- src/conversion.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conversion.jl b/src/conversion.jl index 26534653..e244911a 100644 --- a/src/conversion.jl +++ b/src/conversion.jl @@ -1,5 +1,5 @@ """ - UnitConversionFactor(x::T) where {T<:AbstractFloat} + UnitConversionFactor(x::AbstractFloat) Conversion factor with value `x`. Used by the [`convfact`](@ref) function to preserve @@ -11,6 +11,8 @@ struct UnitConversionFactor{T<:AbstractFloat} <: AbstractIrrational UnitConversionFactor(x::T) where {T<:AbstractFloat} = new{T}(x) end +Base.:*(a::UnitConversionFactor, b::BigFloat) = a.x * b +Base.:*(a::BigFloat, b::UnitConversionFactor) = a * b.x Base.:(==)(a::UnitConversionFactor, b::UnitConversionFactor) = a.x == b.x Base.hash(x::UnitConversionFactor, h::UInt) = hash(x.x, h) Base.BigFloat(x::UnitConversionFactor) = BigFloat(x.x)