Opened 4 years ago

Closed 4 years ago

#24423 closed defect (fixed)

LinearFunctionOrConstraint.__richcmp__ should replace before converting

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.2
Component: linear programming Keywords:
Cc: vbraun Merged in:
Authors: Jeroen Demeyer Reviewers: Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: 4606309 (Commits, GitHub, GitLab) Commit: 4606309005099356eda39142a9abc425d341337e
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

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

Change History (8)

comment:1 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/linearfunctionorconstraint___richcmp___should_replace_before_converting

comment:3 Changed 4 years ago by jdemeyer

  • Commit set to aba680c1b78354977eb4745de79a48f9391d9d4d
  • Status changed from new to needs_review

New commits:

aba680cLinearFunctionOrConstraint.__richcmp__ should replace before converting

comment:4 Changed 4 years ago by git

  • Commit changed from aba680c1b78354977eb4745de79a48f9391d9d4d to 4606309005099356eda39142a9abc425d341337e

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

4606309LinearFunctionOrConstraint.__richcmp__ should replace before converting

comment:5 Changed 4 years ago by jdemeyer

  • Cc vbraun added

comment:6 Changed 4 years ago by mmezzarobba

  • Reviewers set to Marc Mezzarobba
  • Status changed from needs_review to positive_review

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

comment:7 Changed 4 years ago by jdemeyer

Thanks!

comment:8 Changed 4 years ago by vbraun

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