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

MixedIntegerLinearProgram.add_constraint: Allow False, True #34882

Closed
mkoeppe opened this issue Dec 28, 2022 · 10 comments
Closed

MixedIntegerLinearProgram.add_constraint: Allow False, True #34882

mkoeppe opened this issue Dec 28, 2022 · 10 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 28, 2022

follow up from #34878

Also p.linear_constraints_parent()(False)

CC: @mantepse

Component: linear programming

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

@mkoeppe mkoeppe added this to the sage-9.8 milestone Dec 28, 2022
@mkoeppe

This comment has been minimized.

@mantepse
Copy link
Collaborator

comment:2

Meanwhile I understand that it woul be hard to do something sensible with p.linear_constraints_parent()(False), because it should probably be the same as p.linear_constraints_parent()(ZZ(0)), and this is used as the building block for chained (in)equalities. More precisely, it does not make sense (to me) to attach a truth value to p.linear_constraints_parent()(False).

@mantepse
Copy link
Collaborator

comment:3

There is actually a doctest introduced in #13646 that checks that True/False input fail, but I think it is outdated.

@mantepse
Copy link
Collaborator

comment:4

Matthias, you wrote in #34878 comment:22:

It would be OK with me for

  • p.add_constraint(True) to add the trivial constraint "0 <= 0"
  • p.add_constraint(False) to add the infeasible constraint "0 <= -1"

Wouldn't it be better to ignore p.add_constraint(True) entirely?

I am surprised how tricky this little modification is. In particular, it also affects _is_redundant_constraint.

@mantepse
Copy link
Collaborator

comment:5

To give some background, in #33238 I have, repeatedly, things like

                c = AZ_matrix[z][w] - B_matrix[z][w]
                if c.is_zero():
                    continue
                if c in ZZ:
                    raise MIPSolverException
                self.milp.add_constraint(c == 0, name="statistics")

Since the code is already hard to read, just having

                self.milp.add_constraint(AZ_matrix[z][w] == B_matrix[z][w], name="statistics")

seems nicer. Moreover, the check c in ZZ seems brittle. I guess I could use

               if c.coefficient(-1):

instead, but this also looks quite obscure.

Do you have an estimate for when #34251 will be ready? How does CVXPY deal with trivially feasible / infeasible constraints?

@mantepse
Copy link
Collaborator

comment:6

Oops, it would have to be if c.coefficient(-1) and (c - c.coefficient(-1)).is_zero():.

@mantepse
Copy link
Collaborator

comment:7

I am now convinced that this is not a good idea. It is much better to make the user convert the input to a linear constraint themselves.

@mantepse mantepse removed this from the sage-9.8 milestone Dec 29, 2022
@mantepse
Copy link
Collaborator

comment:8

In my case, if I simply make sure that AZ_matrix[z][w] - B_matrix[z][w] is a LinearFunction, everything works just fine.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 29, 2022

comment:9

Yes, working within the linear function parent is, of course, the correct solution.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 29, 2022

comment:10

Replying to Martin Rubey:

How does CVXPY deal with trivially feasible / infeasible constraints?

Usually modeling frameworks have a specialized function for forming fast sums, which also guarantees that the empty sum has the correct type. (Also Sage has MixedIntegerLinearProgram.sum for the same purpose.)

@mkoeppe mkoeppe closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2023
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

2 participants