Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LinearFunctionOrConstraint.__richcmp__ should replace before converting #24423

Closed
jdemeyer opened this issue Dec 22, 2017 · 10 comments
Closed

LinearFunctionOrConstraint.__richcmp__ should replace before converting #24423

jdemeyer opened this issue Dec 22, 2017 · 10 comments

Comments

@jdemeyer
Copy link

The logic in the hack to allow linear functions like x_0 <= x_1 <= x_2 is wrong: currently, when doing x <= y, it first converts x and y to the correct parent and then it checks for a chained comparison. This is wrong: it should check for a chained comparison on the input of the __richcmp__ function before converting.

One consequence is that the following does not work as expected:

sage: p.<x> = MixedIntegerLinearProgram()
sage: from sage.numerical.linear_functions import LinearFunctionsParent
sage: LF = LinearFunctionsParent(QQ)
sage: 3 <= x[0] <= LF(4)
x_0 <= 4

CC: @vbraun

Component: linear programming

Author: Jeroen Demeyer

Branch/Commit: 4606309

Reviewer: Marc Mezzarobba

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

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Jan 5, 2018

@jdemeyer
Copy link
Author

jdemeyer commented Jan 5, 2018

Commit: aba680c

@jdemeyer
Copy link
Author

jdemeyer commented Jan 5, 2018

New commits:

aba680cLinearFunctionOrConstraint.__richcmp__ should replace before converting

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2018

Changed commit from aba680c to 4606309

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2018

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

4606309LinearFunctionOrConstraint.__richcmp__ should replace before converting

@mezzarobba
Copy link
Member

comment:6

Can't really say I understand all implications, but this looks at least as reasonable as the previous version :-).

@mezzarobba
Copy link
Member

Reviewer: Marc Mezzarobba

@jdemeyer
Copy link
Author

jdemeyer commented Feb 7, 2018

comment:7

Thanks!

@vbraun
Copy link
Member

vbraun commented Feb 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants