Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#24824 closed defect (fixed)

Update GLPK to 4.65

Reported by: thansen Owned by:
Priority: major Milestone: sage-9.0
Component: packages: standard Keywords:
Cc: fbissey, gh-timokau, isuruf, mkoeppe Merged in:
Authors: Dima Pasechnik Reviewers: François Bissey
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: 655aad0 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by dimpase)

GLPK 4.65 was released recently and it prints the warning "Long-step dual simplex will be used" all over the place, leading to many failing sagemath doctests in Debian (they fixed it by backporting an upstream patch)

We backport the corresponding patch to GLPK 4.65. Having this in will have the tack of #28459 easier.

tarball: http://users.ox.ac.uk/~coml0531/sage/glpk-4.65.tar.bz2

Change History (35)

comment:1 in reply to:  description Changed 5 years ago by jdemeyer

Replying to thansen:

or if glpk just shouldn't print these warnings.

Reminds me of #20876.

I have no idea what the message "Long-step dual simplex will be used" means. Is it supposed to be a warning?

comment:2 Changed 5 years ago by thansen

It's this change in src/draft/glpios03.c:

+ #if 0 /* 20/I-2018 */
           xprintf("WARNING: LONG-STEP DUAL SIMPLEX WILL BE USED\n");
+ #else
+          xprintf("Long-step dual simplex will be used\n");
+ #endif

So it seems that just the text of the message was changed. Does the content of the message influence whether it's printed?

comment:3 Changed 5 years ago by jdemeyer

No idea...

comment:4 Changed 5 years ago by fbissey

Cc: fbissey added

comment:5 Changed 5 years ago by thansen

Upstream (Andrew Makhorin) says this:

GLP_MSG_OFF is not a relevant option to disable terminal/stdout output. Please use glp_term_out(GLP_OFF) and glp_term_out(GLP_ON) instead--these calls disable/enable terminal output on a lower level independently on options passed to the solvers.

See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891465#21

comment:6 Changed 5 years ago by thansen

Report Upstream: N/AReported upstream. Developers deny it's a bug.

comment:7 in reply to:  5 Changed 5 years ago by jdemeyer

Replying to thansen:

GLP_MSG_OFF is not a relevant option to disable terminal/stdout output.

It's not our goal to completely disable terminal output. We still want to see error messages for example.

comment:8 Changed 5 years ago by jdemeyer

It would also be good to know what the message "Long-step dual simplex will be used" means. Is it supposed to be a warning to say that we are doing something wrong?

comment:9 Changed 5 years ago by jdemeyer

This suggestion from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891465#20 is obviously the right thing to do:

It would be very helpful if GLP_MSG_OFF suppressed this message as it does with other messages.

That would be essentially a one-line patch.

comment:10 Changed 5 years ago by thansen

Report Upstream: Reported upstream. Developers deny it's a bug.Fixed upstream, but not in a stable release.

comment:11 Changed 5 years ago by thansen

Should be fixed in the next release. The patch is here:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891465#24

comment:12 Changed 5 years ago by gh-timokau

Cc: gh-timokau added

comment:13 Changed 4 years ago by jdemeyer

Description: modified (diff)
Summary: Update GLPK to 4.65Update GLPK to 4.66

comment:14 Changed 3 years ago by dimpase

Authors: Dima Pasechnik
Branch: u/dimpase/packages/glpk465
Cc: isuruf added
Commit: 36e1ac5d91a7bb7f5cb581441f8516138f869be5
Description: modified (diff)
Milestone: sage-8.2sage-8.9
Status: newneeds_review
Summary: Update GLPK to 4.66Update GLPK to 4.65

New commits:

36e1ac5update glpk to 4.65

comment:15 Changed 3 years ago by git

Commit: 36e1ac5d91a7bb7f5cb581441f8516138f869be58f1bda09e42fa886973635eb64016f276b0a999a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8f1bda0update glpk to 4.65

comment:16 Changed 3 years ago by dimpase

made the version 4.65.p0, not merely 4.65

comment:17 Changed 3 years ago by git

Commit: 8f1bda09e42fa886973635eb64016f276b0a999a655aad05f8268ee1c58a23e98d2455d2f6965759

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

655aad0make tests less sensitive to output formatting

comment:18 Changed 3 years ago by dimpase

Cc: mkoeppe added

w.r.t. the error_recovery patch, http://trac.sagemath.org/ticket/20710#comment:18 - one can ask upstream, instead of uncondionally applying our patch, make it conditional, on some new parameter that may be manipulated at runtime, how about this?

comment:19 Changed 3 years ago by jdemeyer

It's not that applying the patch upstream will break anything, they refused the patch on principle: they simply don't want to officially support the way how Sage is absusing their error handler. I highly doubt that making the patch conditional will convince upstream. We could ask again to apply the same patch, just in case that they changed their mind.

comment:20 Changed 3 years ago by dimpase

It's one thing to ask to unconditionlly change the default mode of operation, and quite another to ask to accept a patch for a possibility to manipulate the default mode.

Asking to apply the same patch again would be not welcome, I think, and might lessen our chances to get what I propose in.

Could you ask a question whether they'd welcome such a patch, before we spend time working on it?

comment:21 Changed 3 years ago by fbissey

Reviewers: François Bissey

Your strategy to remove blank lines doesn't work. At least with python2.7.

comment:22 Changed 3 years ago by dimpase

I only tested with python3. Surely it's a quirk that can be fixed.

comment:23 in reply to:  22 Changed 3 years ago by dimpase

Replying to dimpase:

I only tested with python3. Surely it's a quirk that can be fixed.

I just tested this branch with python 2.7, all good, too. I have no idea what's not working for you.

comment:24 Changed 3 years ago by fbissey

Status: needs_reviewpositive_review

Indeed I had made a mistake! It is all good.

comment:25 Changed 3 years ago by chapoton

Milestone: sage-8.9sage-9.0

moving milestone to 9.0 (after release of 8.9)

comment:26 Changed 3 years ago by vbraun

Branch: u/dimpase/packages/glpk465655aad05f8268ee1c58a23e98d2455d2f6965759
Resolution: fixed
Status: positive_reviewclosed

comment:27 Changed 3 years ago by slelievre

Commit: 655aad05f8268ee1c58a23e98d2455d2f6965759

comment:28 Changed 3 years ago by jhpalmieri

I'm seeing the message "Long-step dual simplex will be used" using a system installation of glpk, thereby causing lots of failed tests. Is there anything to be done about this? Refuse to use the system installation if it's version 4.65? Block the message somehow?

comment:29 Changed 3 years ago by slelievre

For GLPK 4.65, some package distributions ship a patched version that removes these "Long-step dual simplex will be used" messages, but some don't. Ideally when deciding whether to install GLPK or use the system one, our install scripts should take that into account.

comment:30 Changed 3 years ago by jhpalmieri

Is there a good way to detect that? Is there a standard patch-level or some other indication in the version number that this change has been made?

comment:31 Changed 3 years ago by thansen

comment:32 in reply to:  30 Changed 3 years ago by gh-timokau

Replying to jhpalmieri:

Is there a good way to detect that? Is there a standard patch-level or some other indication in the version number that this change has been made?

Couldn't we just run something that would print the warnings and check if the warnings are printed?

comment:33 in reply to:  31 ; Changed 3 years ago by jhpalmieri

Replying to thansen:

In Debian we patched the doctest parser to ignore these warnings:

https://salsa.debian.org/science-team/sagemath/-/blob/master/debian/patches/dt-version-glpk-4.65-ignore-warnings.patch

This looks good. Why don't we add this to Sage?

comment:34 in reply to:  33 ; Changed 3 years ago by dimpase

Replying to jhpalmieri:

Replying to thansen:

In Debian we patched the doctest parser to ignore these warnings:

https://salsa.debian.org/science-team/sagemath/-/blob/master/debian/patches/dt-version-glpk-4.65-ignore-warnings.patch

This looks good. Why don't we add this to Sage?

+1

comment:35 in reply to:  34 Changed 3 years ago by jhpalmieri

Replying to dimpase:

Replying to jhpalmieri:

Replying to thansen:

In Debian we patched the doctest parser to ignore these warnings:

https://salsa.debian.org/science-team/sagemath/-/blob/master/debian/patches/dt-version-glpk-4.65-ignore-warnings.patch

This looks good. Why don't we add this to Sage?

+1

See #29317.

Note: See TracTickets for help on using tickets.