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

Tests in ToricVarieties/toric_blowups.jl depend on order of rays(normal_fan(simplex(2))) #4348

Closed
lgoettgens opened this issue Nov 25, 2024 · 5 comments · Fixed by #4353
Closed
Labels
bug Something isn't working topic: toric varieties

Comments

@lgoettgens
Copy link
Member

I have seen some test failures there today on CI, but only some of the jobs failed there, e.g. see

Do the tests here or the functions they test employ any kind of randomness? If yes, maybe the tests need to be adapted a bit like e.g. in #4153 by @benlorenz .

failure log:

Basic tests for simple toric blowup: Test Failed at /home/runner/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:44
  Expression: matrix(morphism_on_torusinvariant_weil_divisor_group(bl)) == matrix(ZZ, [1 0 0; 0 1 0; 0 0 1; 1 1 0])
   Evaluated: [1 0 0; 0 1 0; 0 0 1; 1 0 1] == [1 0 0; 0 1 0; 0 0 1; 1 1 0]

Stacktrace:
 [1] macro expansion
   @ /opt/hostedtoolcache/julia/1.11-nightly/x64/share/julia/stdlib/v1.11/Test/src/Test.jl:679 [inlined]
 [2] macro expansion
   @ ~/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:44 [inlined]
 [3] macro expansion
   @ /opt/hostedtoolcache/julia/1.11-nightly/x64/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined]
 [4] macro expansion
   @ ~/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:42 [inlined]
 [5] macro expansion
   @ /opt/hostedtoolcache/julia/1.11-nightly/x64/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined]
 [6] top-level scope
   @ ~/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:3

GC: pause 284.08ms. collected 555.574379MB. incr 
Heap stats: bytes_mapped 1280.31 MB, bytes_resident 998.86 MB,
heap_size 1346.21 MB, heap_target 1429.95 MB, Fragmentation 0.563
Properties of toric blowup defined by ideal: Test Failed at /home/runner/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:76
  Expression: II == center_unnormalized(bl)
   Evaluated: Sheaf of ideals on normal, smooth toric variety without torusfactor == Sheaf of ideals on normal, smooth toric variety without torusfactor

Stacktrace:
 [1] macro expansion
   @ /opt/hostedtoolcache/julia/1.11-nightly/x64/share/julia/stdlib/v1.11/Test/src/Test.jl:679 [inlined]
 [2] macro expansion
   @ ~/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:76 [inlined]
 [3] macro expansion
   @ /opt/hostedtoolcache/julia/1.11-nightly/x64/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined]
 [4] macro expansion
   @ ~/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:76 [inlined]
 [5] macro expansion
   @ /opt/hostedtoolcache/julia/1.11-nightly/x64/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined]
 [6] top-level scope
   @ ~/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:3
Test Summary:                                                             | Pass  Fail  Total   Time
Toric blowups                                                             |   31     2     33  24.2s
  Basic properties of BP2                                                 |    9            9   0.3s
  Basic attributes of BP2                                                 |    7            7   0.3s
  Trigger issue of PR3006                                                 |    2            2   0.6s
  Basic tests for simple toric blowup                                     |    3     1      4   2.4s
  Arithmetics, comparison and composition                                 |    4            4   0.3s
  Strict, total transform and exceptional divisor for simple toric blowup |    4            4   6.0s
  Properties of toric blowup defined by ideal                             |    2     1      3   2.2s
ERROR: LoadError: LoadError: Some tests did not pass: 31 passed, 2 failed, 0 errored, 0 broken.
in expression starting at /home/runner/work/Oscar.jl/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl:1

pinging @HereAround since this is about toric varieties

@lgoettgens lgoettgens added bug Something isn't working topic: toric varieties labels Nov 25, 2024
@HereAround
Copy link
Member

Do the tests here or the functions they test employ any kind of randomness?

To my knowledge, there should not be any randomness involved.

I hope to investigate more shortly (a.k.a. in the next couple of days - latest by beginning of the coming week). If you feel this is more urgent, please let me know and I will prioritize this.

@benlorenz
Copy link
Member

It does not seem to be randomness but due to some side-effects from Polymake.prefer blocks. This should not really happen but they should also not be necessary. I created #4351 to remove those blocks, merge target is the branch for the weight lattice.
(For this problem to appear the linear_program.jl test file must be executed as one of the first tests.)

But it would also be better if the blow-up testcase would not rely on a specific order of the rays of some normal fan:
In a session where this toric blowup test fails I get:

julia> rays(normal_fan(Oscar.simplex(2)))
3-element SubObjectIterator{RayVector{QQFieldElem}}:
 [1, 0]
 [-1, -1]
 [0, 1]

Without the side-effect from the prefer block I get

julia> rays(normal_fan(simplex(2)))
3-element SubObjectIterator{RayVector{QQFieldElem}}:
 [1, 0]
 [0, 1]
 [-1, -1]

and then the testcase succeeds.

@HereAround
Copy link
Member

@benlorenz I will be happy to update the blowup tests so that they do not rely on the order of the rays used.

@benlorenz
Copy link
Member

benlorenz commented Nov 25, 2024

For now #4351 seems to fix the errors, and I will rebase it to master to get it merged. But adjusting the test would still be good.

To reproduce the changed order you can do the following:

In a fresh julia session directly after loading Oscar run

julia> pt = convex_hull([0 1 0]);

julia> Polymake.prefer("beneath_beyond") do
         affine_hull(pt)
       end;

then the rays should print like this

julia> rays(normal_fan(simplex(2)))
3-element SubObjectIterator{RayVector{QQFieldElem}}:
 [1, 0]
 [-1, -1]
 [0, 1]

and then

julia> Oscar.test_module("test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl"; new=false,tempproject=false)

should fail.

If you run the rays(...) statement once before running the prefer block everything works fine, even after the prefer block.
(It should only change the preferred convex hull code inside the block but if it runs very early in the session it somehow seems to persist longer than intended.)

@benlorenz benlorenz changed the title Random test failures in ToricVarieties/toric_blowups.jl Tests in ToricVarieties/toric_blowups.jl depend on order of rays(normal_fan(simplex(2))) Nov 25, 2024
@lgoettgens
Copy link
Member Author

Thanks for looking into this @benlorenz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working topic: toric varieties
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants