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

Added method to find constraints value for MILP #38055

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

verreld7
Copy link
Contributor

@verreld7 verreld7 commented May 22, 2024

Impelement new method to show the values of constraints of MILP according to the current variable's values. Based on #7300.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have updated the documentation and checked the documentation preview.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, please add a test for a single integer input.

What happens when an index is outside of the valid input range?

Comment on lines 1571 to 1575
sum = 0
cons = self.constraints(i)[1]
for index, coeff in zip(cons[0], cons[1]):
sum += var_val[index] * coeff
return sum
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sum = 0
cons = self.constraints(i)[1]
for index, coeff in zip(cons[0], cons[1]):
sum += var_val[index] * coeff
return sum
s = 0
cons = self.constraints(i)[1]
for index, coeff in zip(cons[0], cons[1]):
s += var_val[index] * coeff
return s

Please avoid using builtin names like sum.

Comment on lines 1571 to 1575
sum = 0
cons = self.constraints(i)[1]
for index, coeff in zip(cons[0], cons[1]):
sum += var_val[index] * coeff
return sum
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sum = 0
cons = self.constraints(i)[1]
for index, coeff in zip(cons[0], cons[1]):
sum += var_val[index] * coeff
return sum
s = 0
cons = self.constraints(i)[1]
for index, coeff in zip(cons[0], cons[1]):
s += var_val[index] * coeff
return s

Please avoid using builtin names like sum.

var_val = self.get_values([n for n in var])

if indices is None:
indices = [n for n in range(0, self.number_of_constraints())]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
indices = [n for n in range(0, self.number_of_constraints())]
indices = [n for n in range(self.number_of_constraints())]

The 0 is unneeded in Python.

Comment on lines 1550 to 1552
EXAMPLES:

sage: p = MixedIntegerLinearProgram(maximization=True, solver='GLPK')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXAMPLES:
sage: p = MixedIntegerLinearProgram(maximization=True, solver='GLPK')
EXAMPLES::
sage: p = MixedIntegerLinearProgram(maximization=True, solver='GLPK')

and indent the rest accordingly.

Comment on lines 1564 to 1566
[24.0, 6.0, -1.5, 1.5]

"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[24.0, 6.0, -1.5, 1.5]
"""
[24.0, 6.0, -1.5, 1.5]
"""

Comment on lines 1583 to 1592

if isinstance(indices, (int, Integer)):
result.append(calculate_value(self, indices))
return result
elif isinstance(indices, list):
for i in indices:
result.append(calculate_value(self, i))
return result
else:
raise ValueError("constraints_values() requires a list of integers, though it will accommodate None or an integer.")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isinstance(indices, (int, Integer)):
result.append(calculate_value(self, indices))
return result
elif isinstance(indices, list):
for i in indices:
result.append(calculate_value(self, i))
return result
else:
raise ValueError("constraints_values() requires a list of integers, though it will accommodate None or an integer.")
elif indices in ZZ:
indices = [ZZ(indices)]
return [calculate_value(self, i) for i in indices]

[24.0, 6.0, -1.5, 1.5]

"""
from sage.rings.integer import Integer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from sage.rings.integer import Integer
from sage.rings.integer_ring import ZZ

@tscrim tscrim removed the v: small label Nov 7, 2024
Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you were too greedy with a find-replace sum -> result. Please revert almost all of those.

sage: p.constraints_values()
[24.0, 6.0, -1.5, 1.5]

TESTS::
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TESTS::
TESTS:

Comment on lines 1575 to 1578
ValueError: invalid row index 2

"""
from sage.rings.integer_ring import ZZ
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ValueError: invalid row index 2
"""
from sage.rings.integer_ring import ZZ
ValueError: invalid row index 2
"""
from sage.rings.integer_ring import ZZ

Comment on lines 1528 to 1538
- ``indices`` -- select which constraint(s) values to return

- If ``indices = None``, the method returns the list of all the
constraints values.

- If ``indices`` is an integer `i`, the method returns value of constraint
`i`.

- If ``indices`` is a list of integers, the method returns the list
of the corresponding value of constraints.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ``indices`` -- select which constraint(s) values to return
- If ``indices = None``, the method returns the list of all the
constraints values.
- If ``indices`` is an integer `i`, the method returns value of constraint
`i`.
- If ``indices`` is a list of integers, the method returns the list
of the corresponding value of constraints.
- ``indices`` -- select which constraint(s) values to return:
* ``None`` - return the list of all the constraints values
* an integer `i` - return the value of constraint `i`
* a list of integers - return the list of the
corresponding value of constraints

Copy link

github-actions bot commented Nov 11, 2024

Documentation preview for this PR (built with commit b114d7a; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

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

Successfully merging this pull request may close these issues.

3 participants