add Test.GenericDimensionful number type #39852

stevengj · 2021-02-27T21:38:52Z

As commented in #27899 (comment), we had actually defined ~~three~~ four different unitful types for tests (Furlong, PhysQuantity, MeterUnits, and MockUnitful). This unifies them all into a single type GenericDimensionful that is exported from Test.

(This is analogous to GenericString, GenericDict, etcetera — the goal is to make it easy for package developers to test that their code is generic to new number types without having to think about which additional dependency to pull in.)

Along the way, I discovered a few bugs in the support of dimensionful types by Base and LinearAlgebra functions and I fixed them. (The basic issue is that you should not assume you can convert between types with different dimensions, and should only rely on constructors.)

For now, I left in a const Furlong = Test.GenericDimensionful stub definition because it is needed by the Statistics.jl tests. Once Statistics.jl is updated we can change this to a deprecation. (Or delete it — according to juliahub, no other packages use it?)

stevengj · 2021-02-28T13:36:30Z

The remaining CI failures seem unrelated.

On package_linux32:

/buildworker/worker/package_linux32/build/usr/tools/llvm-size -A /buildworker/worker/package_linux32/build/usr/lib/julia/sys.so /buildworker/worker/package_linux32/build/usr/lib/libjulia.so /buildworker/worker/package_linux32/build/usr/bin/julia
Makefile:563: recipe for target 'build-stats' failed
make: *** [build-stats] Segmentation fault (core dumped)

on tester_linux64:

Error in testset SparseArrays/sparse:
Error During Test at none:1
  Got exception outside of a @test
  ProcessExitedException(7)
  Stacktrace:
    [1] try_yieldto(undo::typeof(Base.ensure_rescheduled))
      @ Base ./task.jl:705
    [2] wait
      @ ./task.jl:764 [inlined]
    [3] wait(c::Base.GenericCondition{ReentrantLock})
      @ Base ./condition.jl:113
    [4] take_buffered(c::Channel{Any})
      @ Base ./channels.jl:389
    [5] take!(c::Channel{Any})
      @ Base ./channels.jl:383
    [6] take!(::Distributed.RemoteValue)
      @ Distributed /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:599
    [7] remotecall_fetch(::Function, ::Distributed.Worker, ::String, ::Vararg{String}; kwargs::Base.Iterators.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
      @ Distributed /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:390
    [8] remotecall_fetch(::Function, ::Int64, ::String, ::Vararg{String}; kwargs::Base.Iterators.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
      @ Distributed /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:421
    [9] macro expansion
      @ /buildworker/worker/tester_linux64/build/share/julia/test/runtests.jl:231 [inlined]
   [10] (::var"#29#39"{Vector{Task}, var"#print_testworker_errored#35"{ReentrantLock, Int64, Int64}, var"#print_testworker_stats#33"{ReentrantLock, Int64, Int64, Int64, Int64, Int64, Int64}, Vector{Any}, Dict{String, DateTime}})()
      @ Main ./task.jl:406

and on tester_freebsd64:

Error in testset Sockets:
Error During Test at none:1
  Got exception outside of a @test
  ProcessExitedException(4)
  Stacktrace:
    [1] try_yieldto(undo::typeof(Base.ensure_rescheduled))
      @ Base ./task.jl:705
    [2] wait
      @ ./task.jl:764 [inlined]
    [3] wait(c::Base.GenericCondition{ReentrantLock})
      @ Base ./condition.jl:113
    [4] take_buffered(c::Channel{Any})
      @ Base ./channels.jl:389
    [5] take!(c::Channel{Any})
      @ Base ./channels.jl:383
    [6] take!(::Distributed.RemoteValue)
      @ Distributed /usr~/worker/package_freebsd64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:599
    [7] remotecall_fetch(::Function, ::Distributed.Worker, ::String, ::Vararg{String}; kwargs::Base.Iterators.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
      @ Distributed /usr~/worker/package_freebsd64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:390
    [8] remotecall_fetch(::Function, ::Int64, ::String, ::Vararg{String}; kwargs::Base.Iterators.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
      @ Distributed /usr~/worker/package_freebsd64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:421
    [9] macro expansion
      @ /usr~/worker-tabularasa/tester_freebsd64/build/share/julia/test/runtests.jl:231 [inlined]
   [10] (::var"#29#39"{Vector{Task}, var"#print_testworker_errored#35"{ReentrantLock, Int64, Int64}, var"#print_testworker_stats#33"{ReentrantLock, Int64, Int64, Int64, Int64, Int64, Int64}, Vector{Any}, Dict{String, DateTime}})()
      @ Main ./task.jl:406

sostock · 2021-02-28T13:53:17Z

Do we know that there are packages that use Furlong and none that use PhysQuantity?

base/floatfuncs.jl

stdlib/Test/src/GenericDimensionfuls.jl

stevengj · 2021-02-28T16:14:36Z

Do we know that there are packages that use Furlong and none that use PhysQuantity?

According to juliahub.com, the only package that uses either one is Statistics.jl (which uses Furlong).

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

sostock · 2021-03-02T09:44:16Z

Should we get a second opinion on the DimensionMismatch pun?

sostock · 2021-03-02T11:11:10Z

Should we have

GenericDimensionful{2}(1) == GenericDimensionful{2.0}(1) == GenericDimensionful{2//1}(1)

or should we expect that people only use the canonical form (Int for integer exponents, Rational{Int} for fractional exponents)? We could also enforce the canonical form in the constructor.

Edit: I see that the above equality should now hold via promotion.

… benefit

sostock · 2021-03-02T10:26:25Z

stdlib/Test/test/dimensionful.jl

+    @test_throws DimensionMismatch x == GD{2}(4)
+    @test_throws ErrorException x == 3


IMO, these shouldn’t error, they should return false (I don’t think == ever errors). Unitful and Dates also return false in this case.

Adding equivalent tests for isequal would also be good.

stdlib/Test/src/GenericDimensionfuls.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

sostock · 2021-03-02T20:13:18Z

I think a lot of things would be easier if we only allowed the canonical type for any p.

sostock · 2021-03-02T20:13:34Z

Sorry, I closed it on accident.

sostock · 2021-03-02T20:16:50Z

Since we have a custom isequal method, we also need to add compatible hash methods. In particular, since GenericDimensionful{0}(x) is equal to x, they must have the same hash.

sostock · 2021-03-02T20:19:07Z

If we want to allow non-canonical exponents, we need to fix this method ambiguity:

julia> promote(GenericDimensionful{0}(2), GenericDimensionful{0.0}(2))
ERROR: MethodError: promote_rule(::Type{GenericDimensionful{0, Int64}}, ::Type{GenericDimensionful{0.0, Int64}}) is ambiguous. Candidates:
  promote_rule(::Type{GenericDimensionful{p, T}}, ::Type{GenericDimensionful{q, S}}) where {p, q, T, S} in Test.GenericDimensionfuls at /home/sebastian/Development/julia/usr/share/julia/stdlib/v1.7/Test/src/GenericDimensionfuls.jl:43
  promote_rule(::Type{GenericDimensionful{0, T}}, ::Type{S}) where {T, S<:Number} in Test.GenericDimensionfuls at /home/sebastian/Development/julia/usr/share/julia/stdlib/v1.7/Test/src/GenericDimensionfuls.jl:46
Possible fix, define
  promote_rule(::Type{GenericDimensionful{0, T}}, ::Type{GenericDimensionful{q, S}}) where {T, q, S}

sostock · 2021-07-12T14:55:19Z

stdlib/Test/src/GenericDimensionfuls.jl

+end
+GenericDimensionful(x::T) where {T<:Number} = GenericDimensionful{1,T}(x)
+GenericDimensionful(x::GenericDimensionful) = x
+(::Type{T})(x::GenericDimensionful) where {T<:Number} = T(x.val)::T


Suggested change

(::Type{T})(x::GenericDimensionful) where {T<:Number} = T(x.val)::T

(::Type{T})(x::GenericDimensionful{0}) where {T<:Number} = T(x.val)::T

So that we don’t convert dimensionful numbers to dimensionless ones.

vtjnash · 2022-03-28T20:59:10Z

Are we still interested in these fixes, or what stalled it? Furlong has picked up a few bugfixes, so those will need to be moved into the new type here

add Test.GenericDimensionful number type

da44308

stevengj added the testsystem The unit testing framework and Test stdlib label Feb 27, 2021

stevengj added 2 commits February 27, 2021 16:40

correct PR number in NEWS

5d06fcb

whoops

32779b5

stevengj requested a review from sostock February 27, 2021 22:10

stevengj added 5 commits February 27, 2021 18:42

disallowing unit ranges for dimensionful types was intentional

2cb8cc3

retain old Furlongs module for backwards compatibility, but deprecate it

4b4096c

no need for export, since deprecation does it

3ccda49

^ fixes

9834e21

more test fixes

92c8da1

stevengj mentioned this pull request Feb 28, 2021

Make minimum/maximum generic for Frequencies JuliaMath/AbstractFFTs.jl#54

Merged

sostock suggested changes Feb 28, 2021

View reviewed changes

stevengj and others added 5 commits February 28, 2021 11:18

Update stdlib/Test/src/GenericDimensionfuls.jl

48fff24

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

Update stdlib/Test/src/GenericDimensionfuls.jl

4e6826a

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

Update stdlib/Test/src/GenericDimensionfuls.jl

cc3d36a

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

various updates, added tests

3fb095f

support Number (dimensionful) atol arg in isapprox

c129422

stevengj mentioned this pull request Mar 1, 2021

LinearAlgebra.givens with unitful arguments #36430

Merged

stevengj added 4 commits March 1, 2021 12:55

rm MockUnitful

fe68453

more fixes

a424c52

note

9e65ea1

more fixes

e93f555

stevengj added 3 commits March 2, 2021 10:46

fix hypot test

2b2ad2b

rm x^GD{0} methods, which introduce too many ambiguities for not much…

6ae16b5

… benefit

test fix

518198a

sostock suggested changes Mar 2, 2021

View reviewed changes

stevengj and others added 3 commits March 2, 2021 13:33

don't throw in promote_type

0341b5c

better promote_rule

d4672bd

Update stdlib/Test/src/GenericDimensionfuls.jl

586451d

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

sostock closed this Mar 2, 2021

sostock reopened this Mar 2, 2021

sostock reviewed Jul 12, 2021

View reviewed changes

sostock mentioned this pull request Feb 11, 2022

RFC: Replace PhysQuantities by Furlongs #44126

Closed

sostock mentioned this pull request Dec 14, 2022

Units interface package PainterQubits/Unitful.jl#595

Open

sostock mentioned this pull request Jan 18, 2023

handle unitful infinite arguments JuliaMath/QuadGK.jl#65

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Test.GenericDimensionful number type #39852

add Test.GenericDimensionful number type #39852

stevengj commented Feb 27, 2021 •

edited

Loading

stevengj commented Feb 28, 2021

sostock commented Feb 28, 2021

stevengj commented Feb 28, 2021

sostock commented Mar 2, 2021

sostock commented Mar 2, 2021 •

edited

Loading

sostock Mar 2, 2021

sostock commented Mar 2, 2021

sostock commented Mar 2, 2021

sostock commented Mar 2, 2021 •

edited

Loading

sostock commented Mar 2, 2021

sostock Jul 12, 2021

vtjnash commented Mar 28, 2022

		@test_throws DimensionMismatch x == GD{2}(4)
		@test_throws ErrorException x == 3

	(::Type{T})(x::GenericDimensionful) where {T<:Number} = T(x.val)::T
	(::Type{T})(x::GenericDimensionful{0}) where {T<:Number} = T(x.val)::T

add Test.GenericDimensionful number type #39852

Are you sure you want to change the base?

add Test.GenericDimensionful number type #39852

Conversation

stevengj commented Feb 27, 2021 • edited Loading

stevengj commented Feb 28, 2021

sostock commented Feb 28, 2021

stevengj commented Feb 28, 2021

sostock commented Mar 2, 2021

sostock commented Mar 2, 2021 • edited Loading

sostock Mar 2, 2021

Choose a reason for hiding this comment

sostock commented Mar 2, 2021

sostock commented Mar 2, 2021

sostock commented Mar 2, 2021 • edited Loading

sostock commented Mar 2, 2021

sostock Jul 12, 2021

Choose a reason for hiding this comment

vtjnash commented Mar 28, 2022

stevengj commented Feb 27, 2021 •

edited

Loading

sostock commented Mar 2, 2021 •

edited

Loading

sostock commented Mar 2, 2021 •

edited

Loading