Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#20710 closed enhancement (fixed)

upgrade glpk to 4.60

Reported by: fbissey Owned by:
Priority: major Milestone: sage-7.5
Component: packages: standard Keywords:
Cc: jpflori, embray, gouezel, fbissey Merged in:
Authors: François Bissey Reviewers: Matthias Koeppe, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: fd7afa8 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

I suggest to upgrade glpk to the latest available version (4.60 at the time of writing). The current version has been superseded in Gentoo and the formatting of the output has changed. Here are the tasks for this upgrade:

  1. use spkg-src instead of patching configure directly
  1. some part of have_error.patch is now upstream, but not every piece of that patch was accepted
  1. clean up SPKG.txt and other files so they are reflecting the current reality more closely without obsolete historical comments

Tarball: http://www.lmona.de/files/sage/glpk-4.60.tar.bz2 (generated with spkg-src)

Change History (72)

comment:1 Changed 5 years ago by fbissey

  • Authors set to François Bissey
  • Branch set to u/fbissey/glpk_4.60
  • Cc jpflori added
  • Commit set to 79883a7750ec829abc0c41a6747c919e7e54db09

Still doctests to fix.

@ jpflori could you have a look at the cygwin part? Upstream has added some stuff to add --no-undefined if host is *-*-cygwin* but you are also checking how gmp/mpir is built if host_os is cygwin* at the moment I have adapted the patch but may be it is safe to drop it completely. From configure.ac

AC_MSG_CHECKING(
   [if libtool needs -no-undefined flag to build shared libraries])
case "${host}" in
   *-*-cygwin* | *-*-mingw* | *-*-aix*)
      ## Add in the -no-undefined flag to LDFLAGS for libtool.
      AC_MSG_RESULT([yes])
      NOUNDEFINED=" -no-undefined"
      ;;
   *)
      ## Don't add in anything.
      AC_MSG_RESULT([no])
      ;;
esac
AC_SUBST([NOUNDEFINED])

with a matching bit for NOUNDEFINED in src/Makefile.am.


New commits:

3e82dbefirst shot at spkg-src for glpk
e3f7691updating glpk to 4.60 - first pass
79883a7Merge branch 'develop' into glpk_4.60

comment:2 Changed 5 years ago by jdemeyer

Replying to fbissey:

2) drop the have_error.patch has the functionality is now upstream without having to change sage's code.

Are you sure about this? As I mentioned in #19525, GLPK upstream accepted the Sage patch with significant modifications.

comment:3 follow-ups: Changed 5 years ago by fbissey

I had been using glpk-4.57 for a while (before moving to 4.60 in my private overlay before starting this ticket) and I had only one doctest failure (version reported is different and cannot just be ellipsed away because that doctest already uses tolerance) for a long time - until some recent MIP work which introduced new doctests. The failures are in verbosity and nothing about error reporting.

But to be sure do we already have a doctest for the functionality and if not, do you have a test case?

comment:4 in reply to: ↑ 3 Changed 5 years ago by jdemeyer

Replying to fbissey:

But to be sure do we already have a doctest for the functionality and if not, do you have a test case?

It has to do with error handling, which should be doctested. But I will check...

comment:5 in reply to: ↑ 3 ; follow-up: Changed 5 years ago by jdemeyer

Replying to fbissey:

version reported is different and cannot just be ellipsed away because that doctest already uses tolerance

Well, 4.57 matches 4.55 with abs tol 0.02 :-)

comment:6 in reply to: ↑ 5 Changed 5 years ago by fbissey

Replying to jdemeyer:

Replying to fbissey:

version reported is different and cannot just be ellipsed away because that doctest already uses tolerance

Well, 4.57 matches 4.55 with abs tol 0.02 :-)

But increasing the tolerance from 1e-5 just for that feels a bit silly. And currently I'd need 0.05.

comment:7 Changed 5 years ago by git

  • Commit changed from 79883a7750ec829abc0c41a6747c919e7e54db09 to b3efcbc8d4f8648d69563763a1f5b67db206457f

Branch pushed to git repo; I updated commit sha1. New commits:

87731aeSmall improvements in spkg-src
b3efcbcfix doctests

comment:8 follow-up: Changed 5 years ago by fbissey

Besides the error reporting there is one last doctest that require particular attention, possibly from a specialist

sage -t --long src/sage/numerical/backends/glpk_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/glpk_backend.pyx", line 1000, in sage.numerical.backends.glpk_backend.GLPKBackend.solve
Failed example:
    p.solve() # rel tol 1
Exception raised:
    Traceback (most recent call last):
      File "/home/fbissey/sandbox/git-fork/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/fbissey/sandbox/git-fork/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.backends.glpk_backend.GLPKBackend.solve[74]>", line 1, in <module>
        p.solve() # rel tol 1
      File "sage/numerical/mip.pyx", line 2197, in sage.numerical.mip.MixedIntegerLinearProgram.solve (/home/fbissey/sandbox/git-fork/sage/src/build/cythonized/sage/numerical/mip.c:15434)
        self._backend.solve()
      File "sage/numerical/backends/glpk_backend.pyx", line 1040, in sage.numerical.backends.glpk_backend.GLPKBackend.solve (/home/fbissey/sandbox/git-fork/sage/src/build/cythonized/sage/numerical/backends/glpk_backend.cpp:8609)
        
    MIPSolverException: GLPK: The time limit has been exceeded
**********************************************************************

Also does that tell us something about error reporting as you intended it Jeroen?

comment:9 Changed 5 years ago by mkoeppe

Where's the tarball?

comment:10 Changed 5 years ago by fbissey

Fair point, I haven't uploaded it yet. I'll do it ASAP.

comment:11 Changed 5 years ago by mkoeppe

(Are you using the upstream tarball or some kind of repackaged tarball?)

comment:12 Changed 5 years ago by fbissey

Repackaged.

comment:13 Changed 5 years ago by fbissey

  • Description modified (diff)

comment:14 Changed 5 years ago by jdemeyer

  • Dependencies set to #20832

comment:15 in reply to: ↑ 3 Changed 5 years ago by jdemeyer

Replying to fbissey:

But to be sure do we already have a doctest for the functionality and if not, do you have a test case?

I added one in #20832.

comment:16 Changed 5 years ago by jdemeyer

  • Description modified (diff)

You still need this piece of have_error.patch (upstream explicitly rejected this):

  • src/env/error.c

    diff -ru glpk-4.55/src/env/error.c glpk-4.55-patched//src/env/error.c
    old new  
    4747      va_end(arg);
    4848      xprintf("Error detected in file %s at line %d\n",
    4949         env->err_file, env->err_line);
     50      env->err_file = NULL;
    5051      if (env->err_hook != NULL)
    5152         env->err_hook(env->err_info);
    5253      abort();
Last edited 5 years ago by jdemeyer (previous) (diff)

comment:17 follow-up: Changed 5 years ago by fbissey

OK, can you enlighten me on why you think it is needed and why it was rejected?

comment:18 in reply to: ↑ 17 Changed 5 years ago by jdemeyer

The default behaviour for GLPK when encountering an error is to print an error message and then abort(). This printing of the error message is done the same way as the usual printing of messages, for example with verbose output. The printing can be customized with a hook function.

GLPK also has an error hook, which is called after printing the error message. It can be used to do something else besides abort(). In the case of Sage, this does a longjmp() through the sig_on() mechanism, raising a Python exception.

In Sage, we want the usual messages to appear normally, but we want to intercept the error messages and convert them to a Python exception. For this to work, we need a way to detect whether we are handling an error or not. I added a patch to GLPK to support this. It allows to checking whether env->err_file is NULL or not. This env->err_file is set to some non-NULL value when entering the error handler and is set back to NULL after printing the error message, before the error hook.

The part which upstream did not accept was the setting back of env->err_file to NULL because they don't want to officially support error recovery. In upstream's view, once GLPK has encountered an error, it is in an invalid state and there is no way to recover.

comment:19 Changed 5 years ago by fbissey

Grumble.... At least that's a very small patch. Will add it here in the morning.

comment:20 Changed 5 years ago by git

  • Commit changed from b3efcbc8d4f8648d69563763a1f5b67db206457f to dfa1ca62fc17ee47212255dfeb615267db959268

Branch pushed to git repo; I updated commit sha1. New commits:

e5145acMerge branch 'develop' into glpk_4.60
dfa1ca6Add patch for error recovery see #20710 comment 18 for detailled explanation.

comment:21 Changed 5 years ago by fbissey

OK delivered early. Now if we could have some comments from @jpflori about the cygwin bits we could put that to review.

comment:22 Changed 5 years ago by tscrim

  • Cc embray gouezel added

cc-ing @embray and @gouezel as they might also have some thoughts about the cygwin portion.

comment:23 Changed 5 years ago by fbissey

And I forgot there is a failing doctest that needs to be looked at by a specialist at comment:8 but I believe we already have someone copied. So this is to bring it back in focus.

comment:24 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:25 in reply to: ↑ 8 Changed 5 years ago by mkoeppe

Replying to fbissey:

Besides the error reporting there is one last doctest that require particular attention, possibly from a specialist

sage -t --long src/sage/numerical/backends/glpk_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/glpk_backend.pyx", line 1000, in sage.numerical.backends.glpk_backend.GLPKBackend.solve
Failed example:
    p.solve() # rel tol 1
Exception raised:
    Traceback (most recent call last):
      File "/home/fbissey/sandbox/git-fork/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/fbissey/sandbox/git-fork/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.backends.glpk_backend.GLPKBackend.solve[74]>", line 1, in <module>
        p.solve() # rel tol 1
      File "sage/numerical/mip.pyx", line 2197, in sage.numerical.mip.MixedIntegerLinearProgram.solve (/home/fbissey/sandbox/git-fork/sage/src/build/cythonized/sage/numerical/mip.c:15434)
        self._backend.solve()
      File "sage/numerical/backends/glpk_backend.pyx", line 1040, in sage.numerical.backends.glpk_backend.GLPKBackend.solve (/home/fbissey/sandbox/git-fork/sage/src/build/cythonized/sage/numerical/backends/glpk_backend.cpp:8609)
        
    MIPSolverException: GLPK: The time limit has been exceeded
**********************************************************************

This test tries to verify that no exception is raised when the optimization process is terminated early because of a set time limit. However, this is not actually true; no exception is raised only when GLPK has a feasible solution available when the time limit expires. Here (in version 4.60) it does not have a feasible solution available. I have not checked the details. On my machine, increasing the time limit to 0.1 fixes this test. Needless to say, this is of course a brittle test.

comment:26 Changed 5 years ago by mkoeppe

I should add that if one increases the time limit, one should probably increase "rel tol 1" to "rel tol 100", because it might find a better solution than 1 on a fast machine (or in a newer GLPK version).

comment:27 follow-up: Changed 5 years ago by fbissey

  • Cc fbissey added

Funky the last email I received from this ticket is when Jeroen changed the description :( - Adding myself as cc to avoid problems.

Unless someone wants to write a new test I will make the changes suggested by Matthias to the failing doctest.

comment:28 in reply to: ↑ 27 Changed 5 years ago by mkoeppe

Replying to fbissey:

Unless someone wants to write a new test I will make the changes suggested by Matthias to the failing doctest.

Go ahead. There's no fundamentally better way to write a new test for this "feature" of the interface.

comment:29 Changed 5 years ago by fbissey

I had to raise the time limit to 1.0, 0.1 was not enough. My machine is not especially slow so I think it may fail on other machines even with 1.0. I am thinking of raising it higher.

comment:30 Changed 5 years ago by git

  • Commit changed from dfa1ca62fc17ee47212255dfeb615267db959268 to c027aba847298d0adc61cfc8d85a421a1df1f5b6

Branch pushed to git repo; I updated commit sha1. New commits:

595c31fMerge branch 'develop' into glpk_4.60
c027abaFix last doctest for 4.60

comment:31 Changed 5 years ago by mkoeppe

I get this (on Mac OS X):

File "src/sage/numerical/mip.pyx", line 2477, in sage.numerical.mip.MixedIntegerLinearProgram.number_of_variables.get_backend
Failed example:
    p.solve()  # rel tol 1e-5
Expected:
    GLPK Simplex Optimizer, v4.60
    2 rows, 2 columns, 4 non-zeros
    *     0: obj =   7.000000000e+00  infeas =  0.000e+00 (0)
    *     2: obj =   9.400000000e+00  infeas =  0.000e+00 (0)
    OPTIMAL LP SOLUTION FOUND
    9.4
Got:
    GLPK Simplex Optimizer, v4.60
    2 rows, 2 columns, 4 non-zeros
    *     0: obj =   7.000000000e+00 inf =   0.000e+00 (2)
    *     2: obj =   9.400000000e+00 inf =   0.000e+00 (0)
    OPTIMAL LP SOLUTION FOUND
    9.399999999999999
Tolerance exceeded in 1 of 13:
    0 vs 2, tolerance inf > 1e-05

Should this ticket be in needs_review?)

comment:32 Changed 5 years ago by fbissey

What does that (2) instead of (0) means? I am guessing we should go to review but we may want to fix this first. cygwin people will have to catch later, although part of me is convinced that I shouldn't add anything at all for cygwin.

comment:33 Changed 5 years ago by fbissey

Oh I see there are other differences. I need to check...

comment:34 Changed 5 years ago by fbissey

I definitely didn't correct that one properly even on linux. Fix coming.

comment:35 Changed 5 years ago by git

  • Commit changed from c027aba847298d0adc61cfc8d85a421a1df1f5b6 to b8aba3a6100241f7e63c9cd5b49579119f97ed2a

Branch pushed to git repo; I updated commit sha1. New commits:

b8aba3aproperly fix doctest mip.pyx

comment:36 Changed 5 years ago by fbissey

  • Status changed from new to needs_review

And now putting to review.

comment:37 Changed 5 years ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:38 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long src/sage/libs/glpk/error.pyx
**********************************************************************
File "src/sage/libs/glpk/error.pyx", line 98, in sage.libs.glpk.error.setup_glpk_error_handler
Failed example:
    res = p.solve()
Expected:
          0: obj = ...
Got:
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of  11 in sage.libs.glpk.error.setup_glpk_error_handler
    [12 tests, 1 failure, 1.34 s]

comment:39 Changed 5 years ago by fbissey

That's the test for #20832. I had forgotten about that one. It is very curious, when run at the sage prompt it gives the expected result but in the doctest we have that blank line. And of course it didn't happen with the previous version of glpk. Any idea Jeroen?

comment:40 follow-up: Changed 5 years ago by mkoeppe

Could you merge the current master?

comment:41 Changed 5 years ago by git

  • Commit changed from b8aba3a6100241f7e63c9cd5b49579119f97ed2a to 5654e49c2a02f9b3407b9249cdc4596bef95d776

Branch pushed to git repo; I updated commit sha1. New commits:

5654e49Merge branch 'develop' into glpk_4.60

comment:42 in reply to: ↑ 40 Changed 5 years ago by fbissey

Replying to mkoeppe:

Could you merge the current master?

Done.

comment:43 Changed 5 years ago by jdemeyer

  • Dependencies #20832 deleted

comment:44 Changed 5 years ago by jdemeyer

  • Branch changed from u/fbissey/glpk_4.60 to u/jdemeyer/glpk_4.60

comment:45 Changed 5 years ago by jdemeyer

  • Commit changed from 5654e49c2a02f9b3407b9249cdc4596bef95d776 to 28ae533fbf946cf85bcd98bb3f2fc7e3ffcf3b35
  • Description modified (diff)

Better use an official upstream tarball instead of a repackaged one.


New commits:

adec9f8first shot at spkg-src for glpk
1ee3a1fupdating glpk to 4.60 - first pass
3d2538eSmall improvements in spkg-src
0b36c8bfix doctests
9cf4d4fAdd patch for error recovery see #20710 comment 18 for detailled explanation.
f7ba9d0Fix last doctest for 4.60
28ae533properly fix doctest mip.pyx

comment:46 follow-up: Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-7.3 to sage-7.5

I see. We need patches to the autoconf sources. So yes, we need a custom tarball.

comment:47 in reply to: ↑ 46 ; follow-up: Changed 5 years ago by fbissey

Replying to jdemeyer:

I see. We need patches to the autoconf sources. So yes, we need a custom tarball.

Thank you, I prefer repacking rather than "custom" patches to configure, Makefile.in and al. Of course, I would prefer running autoreconf on the spot from a pristine tarball but that trade off has already been sold in sagemath.

Any ideas what is going on with the error reporting code?

comment:48 in reply to: ↑ 47 Changed 5 years ago by jdemeyer

Replying to fbissey:

Any ideas what is going on with the error reporting code?

I'm just building this branch now...

comment:49 Changed 5 years ago by git

  • Commit changed from 28ae533fbf946cf85bcd98bb3f2fc7e3ffcf3b35 to 3e8450b13d5c5353cd45d5881acf59994cf28b6b

Branch pushed to git repo; I updated commit sha1. New commits:

3e8450bFix error recovery patch

comment:50 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to positive_review

Trivial fix: upstream apparently uses err_st instead of err_file.

comment:51 Changed 5 years ago by jdemeyer

  • Status changed from positive_review to needs_review

comment:52 Changed 5 years ago by fbissey

I'll test that ASAP.

comment:53 Changed 5 years ago by fbissey

  • Status changed from needs_review to positive_review

Solves the problem here

sage -t --long /usr/lib64/python2.7/site-packages/sage/libs/glpk/error.pyx
    [12 tests, 1.68 s]
----------------------------------------------------------------------
All tests passed!

comment:54 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work
5058sage -t --long src/sage/numerical/backends/glpk_backend.pyx
5059**********************************************************************
5060File "src/sage/numerical/backends/glpk_backend.pyx", line 912, in sage.numerical.backends.glpk_backend.GLPKBackend.solve
5061Failed example:
5062    lp.solve()
5063Expected:
5064    2.0
5065Got:
5066    glp_exact: 3 rows, 2 columns, 6 non-zeros
5067    GNU MP bignum library is being used
5068    *     2:   objval =                      2   (0)
5069    *     2:   objval =                      2   (0)
5070    OPTIMAL SOLUTION FOUND
5071    2.0
5072**********************************************************************
5073File "src/sage/numerical/backends/glpk_backend.pyx", line 944, in sage.numerical.backends.glpk_backend.GLPKBackend.solve
5074Failed example:
5075    lp.solve() == test # yes, we want an exact comparison here
5076Expected:
5077    True
5078Got:
5079    glp_exact: 1 rows, 1 columns, 1 non-zeros
5080    GNU MP bignum library is being used
5081    *     0:   objval =                      0   (0)
5082    *     1:   objval =   9.00719925474095e+15   (0)
5083    OPTIMAL SOLUTION FOUND
5084    True
5085**********************************************************************
5086File "src/sage/numerical/backends/glpk_backend.pyx", line 1578, in sage.numerical.backends.glpk_backend.GLPKBackend.write_lp
5087Failed example:
5088    p.write_lp(os.path.join(SAGE_TMP, "lp_problem.lp"))
5089Expected nothing
5090Got:
5091    Writing problem data to '/Users/buildslave-sage/slave/sage_git/dot_sage/temp/osx/98275/lp_problem.lp'...
5092    9 lines were written
5093**********************************************************************
5094File "src/sage/numerical/backends/glpk_backend.pyx", line 1598, in sage.numerical.backends.glpk_backend.GLPKBackend.write_mps
5095Failed example:
5096    p.write_mps(os.path.join(SAGE_TMP, "lp_problem.mps"), 2)
5097Expected nothing
5098Got:
5099    Writing problem data to '/Users/buildslave-sage/slave/sage_git/dot_sage/temp/osx/98275/lp_problem.mps'...
5100    17 records were written
5101**********************************************************************
5102File "src/sage/numerical/backends/glpk_backend.pyx", line 2228, in sage.numerical.backends.glpk_backend.GLPKBackend.print_ranges
5103Failed example:
5104    p.print_ranges()
5105Expected:
5106    1
5107Got:
5108    glp_print_ranges: optimal basic solution required
5109    1
5110**********************************************************************
5111File "src/sage/numerical/backends/glpk_backend.pyx", line 2232, in sage.numerical.backends.glpk_backend.GLPKBackend.print_ranges
5112Failed example:
5113    p.print_ranges()
5114Expected:
5115    GLPK ... - SENSITIVITY ANALYSIS REPORT                                                                         Page   1
5116    <BLANKLINE>
5117    Problem:
5118    Objective:  7.5 (MAXimum)
5119    <BLANKLINE>
5120       No. Row name     St      Activity         Slack   Lower bound       Activity      Obj coef  Obj value at Limiting
5121                                              Marginal   Upper bound          range         range   break point variable
5122    ------ ------------ -- ------------- ------------- -------------  ------------- ------------- ------------- ------------
5123         1              NU       3.00000        .               -Inf         .           -2.50000        .
5124                                               2.50000       3.00000           +Inf          +Inf          +Inf
5125    <BLANKLINE>
5126    GLPK ... - SENSITIVITY ANALYSIS REPORT                                                                         Page   2
5127    <BLANKLINE>
5128    Problem:
5129    Objective:  7.5 (MAXimum)
5130    <BLANKLINE>
5131       No. Column name  St      Activity      Obj coef   Lower bound       Activity      Obj coef  Obj value at Limiting
5132                                              Marginal   Upper bound          range         range   break point variable
5133    ------ ------------ -- ------------- ------------- -------------  ------------- ------------- ------------- ------------
5134         1              NL        .            2.00000        .                -Inf          -Inf          +Inf
5135                                               -.50000          +Inf        3.00000       2.50000       6.00000
5136    <BLANKLINE>
5137         2              BS       1.50000       5.00000        .                -Inf       4.00000       6.00000
5138                                                .               +Inf        1.50000          +Inf          +Inf
5139    <BLANKLINE>
5140    End of report
5141    <BLANKLINE>
5142    0
5143Got:
5144    Write sensitivity analysis report to '/Users/buildslave-sage/slave/sage_git/dot_sage/temp/osx/98275/ranges.tmp'...
5145    GLPK 4.60 - SENSITIVITY ANALYSIS REPORT                                                                         Page   1
5146    <BLANKLINE>
5147     Problem:    
5148     Objective:  7.5 (MAXimum)
5149    <BLANKLINE>
5150        No. Row name     St      Activity         Slack   Lower bound       Activity      Obj coef  Obj value at Limiting
5151                                               Marginal   Upper bound          range         range   break point variable
5152     ------ ------------ -- ------------- ------------- -------------  ------------- ------------- ------------- ------------
5153          1              NU       3.00000        .               -Inf         .           -2.50000        .     
5154                                                2.50000       3.00000           +Inf          +Inf          +Inf
5155    <BLANKLINE>
5156     GLPK 4.60 - SENSITIVITY ANALYSIS REPORT                                                                         Page   2
5157    <BLANKLINE>
5158     Problem:    
5159     Objective:  7.5 (MAXimum)
5160    <BLANKLINE>
5161        No. Column name  St      Activity      Obj coef   Lower bound       Activity      Obj coef  Obj value at Limiting
5162                                               Marginal   Upper bound          range         range   break point variable
5163     ------ ------------ -- ------------- ------------- -------------  ------------- ------------- ------------- ------------
5164          1              NL        .            2.00000        .                -Inf          -Inf          +Inf
5165                                                -.50000          +Inf        3.00000       2.50000       6.00000
5166    <BLANKLINE>
5167          2              BS       1.50000       5.00000        .                -Inf       4.00000       6.00000
5168                                                 .               +Inf        1.50000          +Inf          +Inf
5169    <BLANKLINE>
5170     End of report
5171    <BLANKLINE>
5172    <BLANKLINE>
5173    0
5174**********************************************************************
51754 items had failures:
5176   2 of  11 in sage.numerical.backends.glpk_backend.GLPKBackend.print_ranges
5177   2 of  76 in sage.numerical.backends.glpk_backend.GLPKBackend.solve
5178   1 of   7 in sage.numerical.backends.glpk_backend.GLPKBackend.write_lp
5179   1 of   7 in sage.numerical.backends.glpk_backend.GLPKBackend.write_mps
5180    [535 tests, 6 failures, 4.97 s]

comment:55 Changed 5 years ago by fbissey

What machine is that?

comment:56 Changed 5 years ago by fbissey

OK something changed with this doctest in between the first time it went to the bot and now. Not sure what yet, but apart from the time limit all the corrected doctest have returned to their previous behavior in that file.

comment:57 Changed 5 years ago by jdemeyer

For me, all tests still pass with in the file src/sage/numerical/backends/glpk_backend.pyx (both with and without --long).

comment:58 Changed 5 years ago by fbissey

Possible, I am seeing the change on a recent snapshot of Volker development branch on github (6th of October). So probably after the last beta was published.

comment:59 Changed 5 years ago by jdemeyer

I confirm the bug, Volker's output is correct: GLPK does print that output, so we should see it. So the doctests should be restored and I will look why it's failing for me.

comment:60 Changed 5 years ago by jdemeyer

False alarm... I was testing an older version of this branch. Good!

So we just need to restore the doctests, I'll do that right now.

comment:61 Changed 5 years ago by git

  • Commit changed from 3e8450b13d5c5353cd45d5881acf59994cf28b6b to fd7afa8008652519f1bee108fd6809b1d147089d

Branch pushed to git repo; I updated commit sha1. New commits:

fd7afa8Fix doctests for GLPK verbose output

comment:62 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:63 Changed 5 years ago by jdemeyer

ping

comment:64 Changed 5 years ago by fbissey

  • Reviewers changed from Matthias Koeppe to Matthias Koeppe, Jeroen Demeyer
  • Status changed from needs_review to positive_review

Putting it back to positive review.

comment:65 Changed 5 years ago by mkoeppe

Looks good to me too.

comment:66 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/glpk_4.60 to fd7afa8008652519f1bee108fd6809b1d147089d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:67 Changed 4 years ago by mderickx

  • Commit fd7afa8008652519f1bee108fd6809b1d147089d deleted

It seems like glpk needs an update again in order to keep the patchbot happy. See #23777 .

comment:68 follow-up: Changed 4 years ago by infinity0

We have to carry a very counterintuitive patch in Debian because the GLPK err_st = 0 patch was not accepted upstream. It takes me several minutes to understand what I myself wrote, every time I read it. :/

Is there any way to do the equivalent thing (err_st = 0) in Sage instead of GLPK? I thought Cython let you access C internals.

comment:69 follow-up: Changed 4 years ago by infinity0

Or as an alternative attempt at upstreaming this again: perhaps err_st = 0 could be conditioned so that most users of GLPK are unaffected, but Sage could pass in a special flag to GLPK to activate this behaviour just for Sage.

comment:70 in reply to: ↑ 68 Changed 4 years ago by jdemeyer

Replying to infinity0:

Is there any way to do the equivalent thing (err_st = 0) in Sage instead of GLPK?

No. Otherwise we would have done that.

I thought Cython let you access C internals.

That's true, but only C internals which are actually exposed by the library. This concerns some internal structure of GLPK which is not exposed by any API.

comment:71 in reply to: ↑ 69 Changed 4 years ago by jdemeyer

Replying to infinity0:

Or as an alternative attempt at upstreaming this again: perhaps err_st = 0 could be conditioned so that most users of GLPK are unaffected

The way I understood upstream's answer is that this patch only affects uses of GLPK which are explicitly unsupported. So, by definition this patch wouldn't break any supported use cases. They don't want to accept this patch since it might imply support for something that they don't support. In the view of GLPK, Sage is using GLPK in a wrong way.

comment:72 Changed 4 years ago by jdemeyer

The feature in question is recovery after an error. As far as I can tell, this was first used by Sage in #12309 and we never had problems with it. So, this is a feature which seemingly works but which is unsupported. And GLPK doesn't want to accept a patch to improve something unsupported.

Maybe you could try to convince GLPK again as a Debian maintainer. Alternatively, could you convince Debian to include the Sage patch for GLPK?

Note: See TracTickets for help on using tickets.