Opened 2 years ago

Closed 2 years ago

#23596 closed enhancement (fixed)

Update GLPK to 4.63

Reported by: fbissey Owned by:
Priority: blocker Milestone: sage-8.1
Component: packages: standard Keywords:
Cc: dcoudert Merged in:
Authors: François Bissey Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: a6e1d23 (Commits) Commit: a6e1d232142c8e7c6a2746ec64c04793b9e763ec
Dependencies: Stopgaps:

Description (last modified by fbissey)

Routine upgrade glpk 4.61 and over break a debugging doctest as a source file has changed location and name.

tarball: http://sagetrac.lipn.univ-paris13.fr/sage/glpk-4.63.tar.bz2

Change History (38)

comment:1 Changed 2 years ago by fbissey

  • Authors set to Francois Bissey
  • Branch set to u/fbissey/glpk4.63
  • Commit set to 23102fece7febe7a6afd068d497f3f462a95c251
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

23102feversion bump. Slight improvement on skg-src. Fix broken doctest.

comment:2 Changed 2 years ago by jdemeyer

  • Authors changed from Francois Bissey to François Bissey

comment:3 Changed 2 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:4 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to needs_info

There is something strange happening when re-installing GLPK 4.60 (the current version in Sage) after installing 4.63. The libraries look like this:

lrwxrwxrwx 1 jdemeyer jdemeyer      17 Aug 11 14:19 local/lib/libglpk.so -> libglpk.so.40.1.0
lrwxrwxrwx 1 jdemeyer jdemeyer      17 Aug 11 14:19 local/lib/libglpk.so.40 -> libglpk.so.40.2.2
-rwxr-xr-x 1 jdemeyer jdemeyer 3592384 Aug 11 14:19 local/lib/libglpk.so.40.1.0
-rwxr-xr-x 1 jdemeyer jdemeyer 3626448 Aug  8 20:45 local/lib/libglpk.so.40.2.2

Note how both versions are installed and how local/lib/libglpk.so.40 points to the new version instead of the old.

This leads to doctest failures because the new version is still used by Sage.

Since this might mask other problems, I am unsetting positive_review until this is clarified.

comment:5 Changed 2 years ago by vbraun

  • Branch changed from u/fbissey/glpk4.63 to 23102fece7febe7a6afd068d497f3f462a95c251
  • Resolution set to fixed
  • Status changed from needs_info to closed

comment:6 Changed 2 years ago by vbraun

  • Commit 23102fece7febe7a6afd068d497f3f462a95c251 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

comment:7 follow-up: Changed 2 years ago by fbissey

Yes, this is curious. Is there anything interesting in your log of glpk-4.60 when you downgrade? It may be a quirk from libtool, but we can deal with that with a removal of old libraries first.

comment:8 in reply to: ↑ 7 ; follow-ups: Changed 2 years ago by jdemeyer

Replying to fbissey:

we can deal with that with a removal of old libraries first.

Not quite, we cannot retroactively patch the current version of GLPK.

comment:9 Changed 2 years ago by vbraun

We don't really make any promises about downgrades, so I wouldn't be overly concerned...

comment:10 in reply to: ↑ 8 Changed 2 years ago by fbissey

Replying to jdemeyer:

Replying to fbissey:

we can deal with that with a removal of old libraries first.

Not quite, we cannot retroactively patch the current version of GLPK.

Right. I didn't think of that obviously. Anything interesting in the log saying the linking failed?

comment:11 Changed 2 years ago by fbissey

OK, I can reproduce the issue and it is weird.

[glpk-4.60.p0] libtool: install: (cd /home/fbissey/sandbox/git-fork/sage/local/lib && { ln -s -f libglpk.so.40.1.0 libglpk.so.40 || { rm -f libglpk.so.40 && ln -s libglpk.so.40.1.0 libglpk.so.40; }; })
[glpk-4.60.p0] libtool: install: (cd /home/fbissey/sandbox/git-fork/sage/local/lib && { ln -s -f libglpk.so.40.1.0 libglpk.so || { rm -f libglpk.so && ln -s libglpk.so.40.1.0 libglpk.so; }; })

So the first operation silently fails but the second one succeeds. In fact I would say the first one is not executed.

comment:12 in reply to: ↑ 8 Changed 2 years ago by fbissey

Replying to jdemeyer:

Replying to fbissey:

we can deal with that with a removal of old libraries first.

Not quite, we cannot retroactively patch the current version of GLPK.

Going back to this one. We cannot make glpk-4.60 behave retro-actively. But we can prevent the problem to happen again in the future.

I have suspicions other libtool packages may behave the same way. Apart from future proofing at the spkg level, Erik's work in #22509 is probably the only thing that will enable a working downgrade path for that case. Again all this is for future improvements.

comment:13 Changed 2 years ago by fbissey

  • Branch changed from 23102fece7febe7a6afd068d497f3f462a95c251 to u/fbissey/glpk4.63
  • Commit set to 30c496fb7ab510df7b0a88e0ccae3050799ebdc6
  • Status changed from new to needs_review

@Jeroen: As I said I don't think we can fix the downgrade to 4.60[1]. I added a commit to remove old libraries, so that problem won't happen after glpk 4.63.

[1] All right, we could push a 4.60.p1 and then upgrade. But I don't think that's worth the extra work.


New commits:

23102feversion bump. Slight improvement on skg-src. Fix broken doctest.
a72d1afMerge branch 'develop' into glpk4.63
30c496fRemove old libraries before installing. This potential enable downrade in the future.

comment:14 follow-up: Changed 2 years ago by jpflori

  • Status changed from needs_review to positive_review

I think we can live without going back to 4.60.

comment:15 follow-up: Changed 2 years ago by jpflori

By the way were our patches ever forwarded upstream? I think I tried but don't remember what happened.

comment:16 in reply to: ↑ 14 Changed 2 years ago by jdemeyer

Replying to jpflori:

I think we can live without going back to 4.60.

Of course. My concern wasn't particularly about 4.60, but there might be a general problem which would occur for every version.

comment:17 in reply to: ↑ 15 Changed 2 years ago by jdemeyer

Replying to jpflori:

By the way were our patches ever forwarded upstream?

error_recovery.patch was forwarded upstream but rejected.

comment:18 Changed 2 years ago by vbraun

  • Branch changed from u/fbissey/glpk4.63 to 30c496fb7ab510df7b0a88e0ccae3050799ebdc6
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:19 Changed 2 years ago by vbraun

  • Commit 30c496fb7ab510df7b0a88e0ccae3050799ebdc6 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

I'm getting crashes on 32-bit...

comment:20 follow-up: Changed 2 years ago by vbraun

Also, with SAGE_DEBUG=yes

**********************************************************************
File "src/sage/numerical/backends/glpk_backend.pyx", line 544, in sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint
Failed example:
    q.add_constraint(p.new_variable()[0] <= 1)
Expected:
    Traceback (most recent call last):
    ...
    GLPKError: glp_set_mat_row: i = 1; len = 1; invalid row length
    Error detected in file glpapi01.c at line ...
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint[10]>", line 1, in <module>
        q.add_constraint(p.new_variable()[Integer(0)] <= Integer(1))
      File "sage/numerical/mip.pyx", line 1816, in sage.numerical.mip.MixedIntegerLinearProgram.add_constraint (/mnt/disk/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/numerical/mip.c:13640)
        self.add_constraint(lhs-rhs, max=0, name=name)
      File "sage/numerical/mip.pyx", line 1804, in sage.numerical.mip.MixedIntegerLinearProgram.add_constraint (/mnt/disk/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/numerical/mip.c:13065)
        self._backend.add_linear_constraint(constraint.items(), min, max, name)
      File "sage/numerical/backends/glpk_backend.pyx", line 569, in sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint (/mnt/disk/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/numerical/backends/glpk_backend.c:6697)
        sig_on()
    GLPKError: glp_set_mat_row: i = 1; len = 1; invalid row length 
    Error detected in file api/prob1.c at line 763
**********************************************************************
1 item had failures:
   1 of  12 in sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint
    [535 tests, 1 failure, 4.87 s]

comment:21 in reply to: ↑ 20 Changed 2 years ago by fbissey

Replying to vbraun:

Also, with SAGE_DEBUG=yes

**********************************************************************
File "src/sage/numerical/backends/glpk_backend.pyx", line 544, in sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint
Failed example:
    q.add_constraint(p.new_variable()[0] <= 1)
Expected:
    Traceback (most recent call last):
    ...
    GLPKError: glp_set_mat_row: i = 1; len = 1; invalid row length
    Error detected in file glpapi01.c at line ...
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint[10]>", line 1, in <module>
        q.add_constraint(p.new_variable()[Integer(0)] <= Integer(1))
      File "sage/numerical/mip.pyx", line 1816, in sage.numerical.mip.MixedIntegerLinearProgram.add_constraint (/mnt/disk/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/numerical/mip.c:13640)
        self.add_constraint(lhs-rhs, max=0, name=name)
      File "sage/numerical/mip.pyx", line 1804, in sage.numerical.mip.MixedIntegerLinearProgram.add_constraint (/mnt/disk/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/numerical/mip.c:13065)
        self._backend.add_linear_constraint(constraint.items(), min, max, name)
      File "sage/numerical/backends/glpk_backend.pyx", line 569, in sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint (/mnt/disk/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/numerical/backends/glpk_backend.c:6697)
        sig_on()
    GLPKError: glp_set_mat_row: i = 1; len = 1; invalid row length 
    Error detected in file api/prob1.c at line 763
**********************************************************************
1 item had failures:
   1 of  12 in sage.numerical.backends.glpk_backend.GLPKBackend.add_linear_constraint
    [535 tests, 1 failure, 4.87 s]

That's the error you would get if the patch to sage/numerical/backends/glpk_backend.pyx is not applied. So for that one at least, it looks like something didn't apply cleanly.

Could we have some access to errors on 32bits?

comment:22 Changed 2 years ago by jdemeyer

I'm doing a 32bit build now. But I agree that in 20 something went wrong in applying this branch. Or possibly, you weren't trying to test this branch at all and you are hitting 4.

comment:23 follow-up: Changed 2 years ago by vbraun

Yes, thats probably whats going on in :comment:20

But the 32-issue is real, I just don't have the logs any more as the buildbot ran out of disks space

comment:24 in reply to: ↑ 23 Changed 2 years ago by jdemeyer

  • Status changed from new to needs_review

Replying to vbraun:

But the 32-issue is real, I just don't have the logs any more as the buildbot ran out of disks space

I cannot reproduce this on arando (32-bit Linux). Could you please try again, if you are running out of disk space, maybe that could explain why stuff breaks too.

comment:25 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:26 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work
sage: g = graphs.CycleGraph(5) ## line 4750 ##
sage: g.fractional_chromatic_index() ## line 4751 ##
2.5
sage: g.fractional_chromatic_index(solver="PPL") ## line 4756 ##
5/2
sage: g = graphs.PetersenGraph() ## line 4763 ##
sage: g.fractional_chromatic_index(solver='GLPK') ## line 4764 ##
------------------------------------------------------------------------
/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/cysignals/signals.so(+0x44e8)[0xf6acc4e8]
/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/cysignals/signals.so(+0x455f)[0xf6acc55f]
/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/cysignals/signals.so(+0x69a7)[0xf6ace9a7]
[0xf772ecb0]
/home/buildbot/slave/sage_git/build/local/lib/libglpk.so.40(+0x20e89)[0xc593ce89]
/home/buildbot/slave/sage_git/build/local/lib/libglpk.so.40(+0x2914c)[0xc594514c]
/home/buildbot/slave/sage_git/build/local/lib/libglpk.so.40(+0x29473)[0xc5945473]
/home/buildbot/slave/sage_git/build/local/lib/libglpk.so.40(+0x295dc)[0xc59455dc]
/home/buildbot/slave/sage_git/build/local/lib/libglpk.so.40(glp_intopt+0x5f5)[0xc59258a5]
/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/numerical/backends/glpk_backend.so(+0x127d5)[0xc60c97d5]
/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/numerical/mip.so(+0xcbf7)[0xcb7e4bf7]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyCFunction_Call+0x109)[0xf75c7299]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x9089)[0xf76357d9]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x7c4)[0xf76360e4]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x8f05)[0xf7635655]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x7c4)[0xf76360e4]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalCode+0x63)[0xf7636253]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x78e3)[0xf7634033]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x7c4)[0xf76360e4]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x8f05)[0xf7635655]
/home/buildbot/slave/sage_git/build/local/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x7c4)[0xf76360e4]

comment:27 Changed 2 years ago by fbissey

Ok, so this it is timing out. PetersenGraph that rings a bell, there was something about that recently I think.

comment:28 Changed 2 years ago by fbissey

  • Cc dcoudert added

It is "caused" by #23658 that just got merged in 8.1.beta4. It seem the test was added to check a similar issue was fixed. Copying David Coudert.

comment:29 follow-up: Changed 2 years ago by dcoudert

Can someone tell me the procedure to test this ticket? I don't know how to do it. Thanks.

comment:30 in reply to: ↑ 29 Changed 2 years ago by fbissey

Replying to dcoudert:

Can someone tell me the procedure to test this ticket? I don't know how to do it. Thanks.

Hi David, the problem only seem to be happen on 32bits platforms. You should be able to test it on top of 8.1.beta4 which includes #23658 in which you introduced the test that is now failing. Follow the usual procedure from http://doc.sagemath.org/html/en/developer/manual_git.html#chapter-manual-git . The problem is only with src/sage/graphs/graph.py on line 1372.

comment:31 Changed 2 years ago by dcoudert

I follwed the manual_git.html page and got the following (git is not my friend).

confetti:sage dcoudert$  git fetch trac 30c496f
fatal: Couldn't find remote ref 30c496f
confetti:sage dcoudert$ git fetch trac 30c496fb7ab510df7b0a88e0ccae3050799ebdc6
confetti:sage dcoudert$ git checkout -b my_branch FETCH_HEAD
fatal: Cannot update paths and switch to branch 'my_branch' at the same time.
Did you intend to checkout 'FETCH_HEAD' which can not be resolved as commit?

Anyway, I don't have access to a 32bit machine. So I don't know how to test and help solving the issue.

comment:32 Changed 2 years ago by jdemeyer

  • Branch changed from 30c496fb7ab510df7b0a88e0ccae3050799ebdc6 to u/jdemeyer/30c496fb7ab510df7b0a88e0ccae3050799ebdc6

comment:33 follow-up: Changed 2 years ago by jdemeyer

  • Commit set to a6e1d232142c8e7c6a2746ec64c04793b9e763ec
  • Status changed from needs_work to needs_review

I marked the problematic test as # known bug. This should allow use to move forward with this ticket.


New commits:

fecaf01version bump. Slight improvement on skg-src. Fix broken doctest.
baacd6cRemove old libraries before installing
a6e1d23Mark failing test as "known bug"

comment:34 Changed 2 years ago by jdemeyer

  • Priority changed from major to blocker

Changing priority to blocker because the patchbots are all posting doctest failures due to the downgrade bug 4.

comment:35 Changed 2 years ago by dcoudert

I succeed to install and test the patch (thank you Jeroen for the change in the branch name. It ease my life). For me it's working well, no infinite loop.

comment:36 in reply to: ↑ 33 Changed 2 years ago by jdemeyer

This minor change still needs review:

Replying to jdemeyer:

a6e1d23Mark failing test as "known bug"

comment:37 Changed 2 years ago by fbissey

  • Status changed from needs_review to positive_review

Sure, let's get this show back on the road.

comment:38 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/30c496fb7ab510df7b0a88e0ccae3050799ebdc6 to a6e1d232142c8e7c6a2746ec64c04793b9e763ec
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.