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

Intermittent test failure in math-effect test #45781

Closed
Keno opened this issue Jun 22, 2022 · 3 comments · Fixed by #45993
Closed

Intermittent test failure in math-effect test #45781

Keno opened this issue Jun 22, 2022 · 3 comments · Fixed by #45993
Assignees

Comments

@Keno
Copy link
Member

Keno commented Jun 22, 2022

Failure is test order dependent and reproducible with

JULIA_CPU_THREADS=1 ../julia runtests.jl LinearAlgebra/svd LinearAlgebra/pinv LinearAlgebra/givens LinearAlgebra/structuredbroadcast numbers copy math
Running parallel tests with:
  nworkers() = 1
  nthreads() = 1
  Sys.CPU_THREADS = 1
  Sys.total_memory() = 1007.822 GiB
  Sys.free_memory() = 992.748 GiB

Test                          (Worker) | Time (s) | GC (s) | GC % | Alloc (MB) | RSS (MB)
LinearAlgebra/svd                  (1) |        started at 2022-06-22T03:20:39.381
LinearAlgebra/svd                  (1) |    50.03 |   2.66 |  5.3 |    7520.24 |   665.83
LinearAlgebra/pinv                 (1) |        started at 2022-06-22T03:21:29.526
LinearAlgebra/pinv                 (1) |     9.04 |   0.66 |  7.3 |    1456.71 |   839.06
LinearAlgebra/givens               (1) |        started at 2022-06-22T03:21:38.564
LinearAlgebra/givens               (1) |     4.25 |   0.21 |  5.0 |     441.16 |   848.18
LinearAlgebra/structuredbroadcast  (1) |        started at 2022-06-22T03:21:42.814
LinearAlgebra/structuredbroadcast  (1) |    33.33 |   2.27 |  6.8 |    4345.93 |   909.84
numbers                            (1) |        started at 2022-06-22T03:22:16.142
numbers                            (1) |    48.12 |   1.14 |  2.4 |    4384.62 |  1356.91
copy                               (1) |        started at 2022-06-22T03:23:04.259
copy                               (1) |     2.52 |   0.09 |  3.4 |     277.91 |  1356.91
math                               (1) |        started at 2022-06-22T03:23:06.782
┌ Warning: bad effects found for sin(::Float32)
│   eff = (+c,+e,?n,?t,+s)
└ @ Main.Test6Main_math ~/julia/test/math.jl:1467
┌ Warning: bad effects found for cos(::Float32)
│   eff = (+c,+e,?n,?t,+s)
└ @ Main.Test6Main_math ~/julia/test/math.jl:1467
┌ Warning: bad effects found for tan(::Float32)
│   eff = (+c,+e,?n,?t,+s)
└ @ Main.Test6Main_math ~/julia/test/math.jl:1467
math                               (1) |    36.14 |   1.31 |  3.6 |    4391.28 |  1625.89

Test Summary: |    Pass  Broken    Total     Time
  Overall     | 3493226       3  3493229  3m04.5s
    SUCCESS

Originally posted by @Keno in #45670 (comment)

@staticfloat
Copy link
Member

@aviatesk and I managed to squeeze this down to reproducible with just:

Base.runtests(["numbers", "math"]; ncores=1)

@martinholters
Copy link
Member

Looks like sign(pi) (contained in numbers test) is the offender.
Without:

julia> eff = Base.infer_effects(sin, (Float32,))
(+c,+e,?n,+t,+s)

julia> Core.Compiler.is_foldable(eff)
true

Fresh session:

julia> sign(pi)
1.0

julia> eff = Base.infer_effects(sin, (Float32,))
(+c,+e,?n,?t,+s)

julia> Core.Compiler.is_foldable(eff)
false

@martinholters
Copy link
Member

Drilling a bit into sign(pi), I've arrived at

julia> promote_type(Irrational{}, Bool)
Float64

julia> Base.infer_effects(sin, (Float32,))
(+c,+e,?n,?t,+s)

OTOH (in a fresh session again):

julia> Base.promote_result(Irrational{}, Bool, Float64, Irrational{})
Float64

julia> promote_type(Irrational{}, Bool)
Float64

julia> Base.infer_effects(sin, (Float32,))
(+c,+e,?n,+t,+s)

aviatesk added a commit that referenced this issue Jul 11, 2022
In a similar spirit to #40561, we can relax the recursion detection to
guarantee `:terminates` effect and allow the effects analysis to not
taint `:terminates` effect when there are no cycles in `MethodInstance`s
in a call graph.

fix #45781
aviatesk added a commit that referenced this issue Jul 12, 2022
In a similar spirit to #40561, we can relax the recursion detection to
guarantee `:terminates` effect and allow the effects analysis to not
taint `:terminates` effect when there are no cycles in `MethodInstance`s
in a call graph.

fix #45781
aviatesk added a commit that referenced this issue Jul 20, 2022
In a similar spirit to #40561, we can relax the recursion detection to
guarantee `:terminates` effect and allow the effects analysis to not
taint `:terminates` effect when there are no cycles in `MethodInstance`s
in a call graph.

fix #45781
ffucci pushed a commit to ffucci/julia that referenced this issue Aug 11, 2022
In a similar spirit to JuliaLang#40561, we can relax the recursion detection to
guarantee `:terminates` effect and allow the effects analysis to not
taint `:terminates` effect when there are no cycles in `MethodInstance`s
in a call graph.

fix JuliaLang#45781
pcjentsch pushed a commit to pcjentsch/julia that referenced this issue Aug 18, 2022
In a similar spirit to JuliaLang#40561, we can relax the recursion detection to
guarantee `:terminates` effect and allow the effects analysis to not
taint `:terminates` effect when there are no cycles in `MethodInstance`s
in a call graph.

fix JuliaLang#45781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants