Cannot copy() instances of GurobiBackend.

[sagetrac-e-vaughan]

GurobiBackend does not implement copy().

I've attached a patch to add this functionality.

The patch also corrects a few doctests in the same file, that have solver="Gurobi" instead of solver="GUROBI".

(The patch was created with Sage 4.8.)

APPLY:

• attachment: 16400.patch
• attachment: trac_12973-review.patch

Depends on #14566

Component: linear programming

Keywords: Gurobi, GurobiBackend

Author: Emil R. Vaughan, Nathann Cohen

Reviewer: Volker Braun

Merged: sage-5.10.beta3

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

[sagetrac-e-vaughan]

Changed keywords from none to Gurobi, GurobiBackend

[nathanncohen]

comment:3

Wow !! Very good patch for a first ! :-D

Well, it did not apply cleanly on version 5.1.beta0 but that was to be expected if you worked on 4.8 Actually, the replacement from GUROBI to Gurobi has been done since, and that was the whole reason why it did not apply. I just rebased your patch on 5.1.beta0 and updated it on the trac ticket.

There is a problem though, for your patch does not compile. When I apply it and type "sage -b" I get this :

Error compiling Cython file:
------------------------------------------------------------
...
            sage: p.set_objective([2, 5])                                           # optional - Gurobi
            sage: p.write_lp(SAGE_TMP+"/lp_problem.lp")                             # optional - Gurobi
        """
        check(self.env, GRBwrite(self.model[0], filename))

    cpdef GurobiBackend copy(self):
         ^
------------------------------------------------------------

sage/numerical/backends/gurobi_backend.pyx:1079:10: C method 'copy' not previously declared in definition part of extension type
Error running command, failed with status 256.
Error installing modified sage library code.

This is because cdef methods should be added to gurobi_backend.pxd as it is added for glpk in glpk_backend.pxd. I will post another patch to do exactly that.

Nathann

[nathanncohen]

comment:4

Nice !

So I agree with your changes, but as I added something myself you also need to check that everything is fine on your side, and then we will be able to set this ticket to "positive review", which means that the release manager can add it to the next release of Sage!

Thank you for your patch !!

Nathann

[sagetrac-e-vaughan]

comment:6

Thanks Nathann! I am a bit reluctant to upgrade to Sage 5.x to test the patch due to the issues with Core2 Duo Macs....

Also, in my original patch, I did have a

cpdef GurobiBackend copy(self)

line.... Maybe that bit of the patch didn't apply properly in 5.1?

One question: why have you changed copy to be a cdef instead of a cpdef? (I used cpdef because that's what GLPKBackend uses (or did in 4.8).)

[nathanncohen]

comment:7

Helloooooo !!!

Thanks Nathann! I am a bit reluctant to upgrade to Sage 5.x to test the patch due to the issues with Core2 Duo Macs....

Ahaha. That's understandable :-D
So the bug happens even when you compile from source ? Scary mac..

Also, in my original patch, I did have a

cpdef GurobiBackend copy(self)

line.... Maybe that bit of the patch didn't apply properly in 5.1?

OOpps. Yes, probably. I looked at the list of rejected lines and saw so many GUROBI -> Gurobi, I probably missed that one.

One question: why have you changed copy to be a cdef instead of a cpdef? (I used cpdef because that's what GLPKBackend uses (or did in 4.8).)

Oops, you are right. Actually, cpef functions are functions that can be called at either C-level or Python level. Well, it more or less changes nothing in this situation, as the code onl calls the copy method from the backends at C level. It's actually totally up to you :-)

Nathann

[jdemeyer]

comment:9

Please fill in your real name as Author.

[nathanncohen]

Author: Nathann Cohen

[nathanncohen]

Reviewer: Nathann Cohen

[nathanncohen]

Changed author from Nathann Cohen to Emil R. Vaughan

[vbraun]

comment:12

patchbot:
apply 16400.patch, trac_12973-review.patch

[vbraun]

Dependencies: #14566

[nathanncohen]

Attachment: 16400.patch.gz

[nathanncohen]

Attachment: trac_12973-review.patch.gz

[nathanncohen]

comment:14

Rebased !

Nathann

[vbraun]

comment:15

lgtm

[vbraun]

Changed reviewer from Nathann Cohen to Volker Braun

[vbraun]

Changed author from Emil R. Vaughan to Emil R. Vaughan, Nathann Cohen

[nathanncohen]

comment:16

Just learned a new acronym. Thank you very much Volker ! :-)

Nathann

[jdemeyer]

Merged: sage-5.10.beta3