Infinities.jl icon indicating copy to clipboard operation
Infinities.jl copied to clipboard

hashing is part unimplemented, part incorrect

Open nsajko opened this issue 1 year ago • 5 comments

Test:

using Test, Infinities

@testset "hash versus floating-point infinity" begin
    @testset "one argument" begin
        @test hash(Inf) === hash(∞)
    end
    @testset "two arguments" begin
        for h ∈ rand(UInt, 256)
            @test hash(Inf, h) === hash(∞, h)
        end
    end
end

Result:

Test Summary:                       | Fail  Error  Total   Time
hash versus floating-point infinity |    1    256    257  22.4s
  one argument                      |    1             1   1.6s
  two arguments                     |         256    256  20.9s
ERROR: Some tests did not pass: 0 passed, 1 failed, 256 errored, 0 broken.

nsajko avatar Sep 23 '24 10:09 nsajko

The proper fix is to implement the two-argument hash method by simply delegating to hash(::Float32, ::UInt) (Float64 would work, too, but I guess hashing Float32 is faster than hashing Float64?).

NB: I plan on fixing this indirectly by:

  1. creating a package TypeDomainRealInfinities, implementing negative and positive infinity as singleton types correctly
  2. getting the new package under JuliaMath
  3. getting Infinities to depend on the package and redefining Infinity

nsajko avatar Sep 23 '24 10:09 nsajko

I don't see why creating new packages is needed for fixing hash.

Note Infinity represents a broader concept then a positive real infinity. For example if the real line or complex plane are compactified then Infinity would be fine to represent the infinity. But that said, originally Infinity was the same as InfiniteCardinality{0}, that is, it was a subtype of Integer. And so there was a lot of discussion that ended up with the current setup.

dlfivefifty avatar Sep 23 '24 14:09 dlfivefifty

why creating new packages is needed for fixing hash

It's not. I just want a package with a clear and more narrow scope to depend on, so I thought I'd fix existing bugs while at it.

Note Infinity represents a broader concept then a positive real infinity.

But it's equal to floating-point Inf, so the hashes must be equal, too.

julia> using Infinities

julia> isequal(Inf, ∞)
true

julia> hash(Inf) == hash(∞)
false

nsajko avatar Sep 23 '24 15:09 nsajko

Are the infinities not supposed to equal each other?

nsajko avatar Sep 23 '24 15:09 nsajko

Yes they are meant to be equal to each other. You'd have to go back to old discussion to understand why we wanted Infinity <: Number.... not sure I actually remember.

dlfivefifty avatar Sep 23 '24 15:09 dlfivefifty