alarm test failure in CombinatorialPolyhedron.f_vector

[embray]

Ever since the merging of #26887 I get this one test failure:

sage -t --long --warn-long 157.3 src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx
**********************************************************************
File "src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx", line 1132, in sage.geometry.polyhedron.combinatorial_polyhedron.base.CombinatorialPolyhedron.f_vector
Failed example:
    try:
        alarm(0.5)
        C.f_vector()
    except:
        print("alarm!")
Expected:
    alarm!
Got:
    (1,
     25,
     300,
     2300,
     12650,
     53130,
     177100,
     480700,
     1081575,
     2042975,
     3268760,
     4457400,
     5200300,
     5200300,
     4457400,
     3268760,
     2042975,
     1081575,
     480700,
     177100,
     53130,
     12650,
     2300,
     300,
     25,
     1)
**********************************************************************
1 item had failures:
   1 of  12 in sage.geometry.polyhedron.combinatorial_polyhedron.base.CombinatorialPolyhedron.f_vector
    [324 tests, 1 failure, 6.49 s]

It's a minor issue, but I mark the ticket priority as "critical" as this is likely to affect others as well, and constitutes a regression. There's no way to guarantee that 0.5 seconds, or any amount of time really, is short enough to trigger the alarm as soon as the test requires (though if nothing else it should be a shorter time). I did check, to be certain, that lowering the alarm time in this test does cause it to pass for me. But that's no guarantee it won't still fail on an even faster machine.

Although, if as this comment states, there is no explicit reason for these tests, I would just remove them.

CC: @kliem

Component: geometry

Keywords: f_vector, polytopes

Author: Jonathan Kliem

Branch/Commit: e62f499

Reviewer: Erik Bray

Issue created by migration from https://trac.sagemath.org/ticket/28287

[embray]

comment:2

Also I'll note that, in the process of trying to test this manually, I got bitten by this issue again, which I see now that we never made a ticket for. I'll go do that now.

[embray]

comment:3

In fact, although I can get this test to pass at the command prompt by itself, I can't seem to get the copy in the source code to pass when I modify it to lower the alarm() time. It seems there are results being cached from earlier tests that cause that call to C.f_vector() to return almost immediately.

[kliem]

comment:4

Replying to @embray:

In fact, although I can get this test to pass at the command prompt by itself, I can't seem to get the copy in the source code to pass when I modify it to lower the alarm() time. It seems there are results being cached from earlier tests that cause that call to C.f_vector() to return almost immediately.

There is nothing being cashed at that point. Also if there was something being cashed or C.f_vector would be faster than alarm, alarm would escape the try ... except environment and would cause sage to crash. This didn't seem to be the issue. (Also the specific calculation takes 1.92 s at my machine. As f_vector is not parallelized (yet), I doubt that your machine can do it under a second.)

What I think is happening, is that the alarm is triggered, but has no effect. For some reason the alarm just vanishes and the procedure keeps on going. This would also explain many patchbot reports, where there is a timeout with this module.

I don't seem to be the only one with this problem: https://patchbot.sagemath.org/log/27973/Ubuntu/14.04/i686/3.13.0-167-generic/arando/2019-07-25%2015:09:25?short

As a resolution one could either figure out,

• why alarm doesn't trigger anything in some cases (I don't think I will be successful with this) or
• we can just delete all the tests including alarm interrupts in CombinatorialPolyhedron (as stated in the description of this ticket).

I would prefer the second option, as the first depends on somebody with time and knowledge. We won't have a certificate anymore that the methods in CombinatorialPolyhedron can be interrupted, so we have to trust anyone making changes to not break the ability to interrupt.

[kliem]

comment:5

One more example for alarm being messed up with sage -t:

https://groups.google.com/d/msg/sage-devel/wqnilUCLsT4/4riK4FnhDQAJ

In sage/rings/integer.pyx the alarm interrupts takes much longer than they should take, when using sage -t.

[kliem]

Commit: e62f499

[kliem]

Author: Jonathan Kliem

[kliem]

New commits:

e62f499

removed tests involving alarm interrupts

[kliem]

Branch: public/28287

[embray]

Reviewer: Erik Bray

[embray]

comment:7

Thanks, I think maybe that's the best for now. Writing a test like this to work reliably is hard, and unless there's a specific regression to test related to interruptability I don't know that such tests add much value.

If you do want to have one, it has to be a case where it for sure won't return unless interrupted.

[vbraun]

Changed branch from public/28287 to e62f499