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

Sporadic test failure comparing extrema and minimum, maximum #54166

Closed
Zentrik opened this issue Apr 21, 2024 · 4 comments · Fixed by #54222
Closed

Sporadic test failure comparing extrema and minimum, maximum #54166

Zentrik opened this issue Apr 21, 2024 · 4 comments · Fixed by #54222
Assignees
Labels
compiler:codegen Generation of LLVM IR and native code regression Regression in behavior compared to a previous version
Milestone

Comments

@Zentrik
Copy link
Member

Zentrik commented Apr 21, 2024

The test at

@test all(x -> isequal(x[1], x[2:3]), zip(vext,vmin,vmax))
which is called by
@testset "NaN/missing test for extrema with dims #43599" begin
fails for me depending on what random matrices get generated.

E.g.

julia> a = Union{Missing, Int}[missing -3.0 3.0; -1.0 missing 0.0; missing 2.0 -3.0]
3×3 Matrix{Union{Missing, Int64}}:
   missing  -3          3
 -1           missing   0
   missing   2         -3

julia> dims = (1, 2)
(1, 2)

julia> vext = extrema(a; dims)
1×1 Matrix{Tuple{Union{Missing, Int64}, Union{Missing, Int64}}}:
 (missing, missing)

julia> vmin, vmax = minimum(a; dims), maximum(a; dims)
(Union{Missing, Int64}[-3;;], Union{Missing, Int64}[3;;])

julia> all(x -> isequal(x[1], x[2:3]), zip(vext,vmin,vmax))
false

julia> versioninfo()
Julia Version 1.12.0-DEV.379
Commit aad7245853 (2024-04-20 23:40 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: 16 × AMD Ryzen 7 5700U with Radeon Graphics
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver2)
Threads: 1 default, 0 interactive, 1 GC (on 16 virtual cores)

I don't see this on 1.10.2 but I do on 1.11.0-alpha1, 1.11.0-beta1 and master.

@Zentrik Zentrik added the regression Regression in behavior compared to a previous version label Apr 21, 2024
@Zentrik Zentrik added this to the 1.11 milestone Apr 21, 2024
@Seelengrab
Copy link
Contributor

Seems to be due to a change to minimum/maximum, which seem to ignore missing.

v1.10.1:

julia> ex = Union{Missing, Int64}[missing -2; missing -2];

julia> dims = (1,2)
(1, 2)

julia> extrema(ex; dims)
1×1 Matrix{Tuple{Union{Missing, Int64}, Union{Missing, Int64}}}:
 (missing, missing)

julia> minimum(ex; dims)
1×1 Matrix{Union{Missing, Int64}}:
 missing

julia> maximum(ex; dims)
1×1 Matrix{Union{Missing, Int64}}:
 missing

julia> versioninfo()
Julia Version 1.10.1
Commit 7790d6f0641 (2024-02-13 20:41 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 24 × AMD Ryzen 9 7900X 12-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, znver3)
Threads: 1 default, 0 interactive, 1 GC (on 24 virtual cores)
Environment:
  JULIA_PKG_USE_CLI_GIT = true

v1.12:

julia> ex = Union{Missing, Int64}[missing -2; missing -2];

julia> dims = (1,2)
(1, 2)

julia> extrema(ex; dims)
1×1 Matrix{Tuple{Union{Missing, Int64}, Union{Missing, Int64}}}:
 (missing, missing)

julia> minimum(ex; dims)
1×1 Matrix{Union{Missing, Int64}}:
 -2

julia> maximum(ex; dims)
1×1 Matrix{Union{Missing, Int64}}:
 -2

julia> versioninfo()
Julia Version 1.12.0-DEV.306
Commit 0221ebc9b1 (2024-04-06 08:13 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: 24 × AMD Ryzen 9 7900X 12-Core Processor
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver4)
Threads: 23 default, 1 interactive, 23 GC (on 24 virtual cores)
Environment:
  JULIA_PKG_USE_CLI_GIT = true

@KristofferC
Copy link
Member

KristofferC commented Apr 22, 2024

I bisected this to 8db1294

~/julia$ manyjulias 8db129472ea88a9c5c4a6 test.jl 
# [ Info: Translated requested revision 8db129472ea88a9c5c4a6 to commit 8db129472ea88a9c5c4a6e94dff7b330723aed28
minimum(ex; dims) = Union{Missing, Int64}[-3;;]

~/julia$ manyjulias 8db129472ea88a9c5c4a6~1 test.jl 
# [ Info: Translated requested revision 8db129472ea88a9c5c4a6~1 to commit 3d8b10770a7a7317dde13a
minimum(ex; dims) = Union{Missing, Int64}[missing;;]

cc @vtjnash

@KristofferC
Copy link
Member

I think this is a codegen / inference bug because JuliaInterpreter gets it right:

julia> using JuliaInterpreter

julia> @interpret minimum(ex; dims)
1×1 Matrix{Union{Missing, Int64}}:
 missing

julia> minimum(ex; dims)
1×1 Matrix{Union{Missing, Int64}}:
 -2

@KristofferC KristofferC added the compiler:codegen Generation of LLVM IR and native code label Apr 22, 2024
@vtjnash
Copy link
Member

vtjnash commented Apr 22, 2024

That seems possible. 8db1294 rewrote some of the memory load/store code to try to make it share more code with the other paths, but the way it stores the type selector bit and computes alignments is much different, so I might have missed a place that needed to be updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants