Jaeyeon/newdispatch

[nullplay]

Hope this fixes the continuous tests on CI.

[codecov]

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (2c17510) 76.04% compared to head (6239cad) 76.06%.

Files	Patch %	Lines
src/util/limits.jl	66.66%	7 Missing ⚠️
src/dimensions.jl	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #362      +/-   ##
==========================================
+ Coverage   76.04%   76.06%   +0.02%     
==========================================
  Files          80       80              
  Lines        6830     6832       +2     
==========================================
+ Hits         5194     5197       +3     
+ Misses       1636     1635       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[willow-ahrens]

Something weird I have found:

julia> hash(1.0, UInt(123))
0x5bca7c69b794f75d

julia> hash(1.0f0, UInt(123))
0x5bca7c69b794f75d

julia> hash(limit(1.0f0), UInt(123))
0xab8298daf3de8022

julia> d = Dict{Any, String}(1.0=>"hello")
Dict{Any, String} with 1 entry:
  1.0 => "hello"

julia> d[limit(1.0f0)]
"hello"

julia> for i = 1:10000
       d[i] = "bar"
       end

julia> d[limit(1.0f0)]
ERROR: KeyError: key limit(1.0) not found
Stacktrace:
 [1] getindex(h::Dict{Any, String}, key::Limit{Float32})
   @ Base ./dict.jl:482
 [2] top-level scope
   @ REPL[37]:1

Our problem is that isequal(1.0, limit(1.0f0)) == true, which isn't really helpful for us when implementing shash, as we'd really like different hashes for things with different types. I'm trying this out on #363.

[willow-ahrens]

I used to do it this way, but all the extra calls to simplify slowed the compiler down, so we avoid this now.

[nullplay]

The CI tests on the main branch used to fail because the result of simplify was occasionally different on x64 Ubuntu Julia 1.6.7 when the Finch expression included a Limit.

Therefore, I attempted to explicitly launch the simplify pass using this code snippet to verify if it works. In any case, it appears that your main branch now passes the continuous tests after your hash fix, so I assume the underlying issue was related to the static hash handling of the Limit type. Consequently, we can remove this change.

[willow-ahrens]

This for loop avoids method ambiguities on types that define e.g. max(a::MyType, b::Real). If we define max(a::Real, b::Limit) where Limit <: Real, then we get an ambiguity error. If we want to take your approach, we need to make Limit not be a subtype of Number. I understand that this is suboptimal.

[willow-ahrens]

Oh, I see that currently Limit <: Number, so Limit is not currently subtyping Real. I think this might work then. But should we have Limit <: Real?

[nullplay]

This was also a random attempt to address the CI issues in continuous testing. While it makes the code cleaner, it introduces ambiguity. Since the CI now passes, I believe we no longer need this pull request. Let's close it.