Skip to content

GDP: Fix bug transforming Blocks in gdp.mbigm transformation #3811

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.

Sign up for GitHub

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

Open
bammari wants to merge 10 commits into
base: main
Choose a base branch
from
+35 −2
Open

GDP: Fix bug transforming Blocks in gdp.mbigm transformation #3811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a78f5a5
Change getname argument relative_to to ensure constraint and disjunct…
bammari Jan 8, 2026
85d5fa6
Add test to ensure transformation works on block
bammari Jan 9, 2026
c67ec2c
assertEqual instead of assert
bammari Jan 9, 2026
c79c58b
black and assert names equal
bammari Jan 9, 2026
4298328
assert names equal
bammari Jan 9, 2026
fd5dc25
Merge branch 'Pyomo:main' into mbigm-resolve-names
bammari Jan 9, 2026
7fd5d1b
skip transform on block test if Gurobi not available
bammari Jan 9, 2026
4054409
Merge branch 'mbigm-resolve-names' of https://github.com/bammari/pyom…
bammari Jan 9, 2026
e4986a0
Add assertions that test whether linear constraint in disjunct 1 is t…
bammari Jan 9, 2026
1a00db6
Merge branch 'main' into mbigm-resolve-names
blnicho Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pyomo/gdp/plugins/multiple_bigm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,8 @@ def _transform_disjunctionDatas(
if jobs:
jobs_by_name = [
(
constraint.getname(fully_qualified=True),
other_disjunct.getname(fully_qualified=True),
constraint.getname(fully_qualified=True, relative_to=instance),
other_disjunct.getname(fully_qualified=True, relative_to=instance),
unsuccessful_solve_msg,
is_upper,
)
Expand Down
33 changes: 33 additions & 0 deletions pyomo/gdp/tests/test_mbigm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
TransformationFactory,
value,
Var,
Block,
)
from pyomo.gdp import Disjunct, Disjunction, GDP_Error
from pyomo.gdp.tests.common_tests import (
Expand Down Expand Up @@ -1190,3 +1191,35 @@ def test_complain_for_unrecognized_Suffix(self):
TransformationFactory('gdp.mbigm').apply_to(
m, reduce_bound_constraints=False
)

@unittest.skipUnless(gurobi_available, "Gurobi is not available")
def test_transform_on_block(self):
# In multiple_bigm.py, if constraint.getname and disjunct.getname do not
# set relative_to=instance, the transformation fails because of mismatched
# names in _calc_M.
# This test ensures that the transformation works on blocks as well as
# ConcreteModels.
m = ConcreteModel()
m.b = Block()
m.b.x = Var(bounds=(0, 5))
m.b.y = Var()
m.b.dis1 = Disjunct()
m.b.dis2 = Disjunct()

m.b.dis1.linear = Constraint(expr=m.b.x * 0.5 + 3 == m.b.y)
m.b.dis2.linear = Constraint(expr=m.b.x * 0.2 + 1 == m.b.y)
m.b.d = Disjunction(expr=[m.b.dis1, m.b.dis2])
Comment on lines +1209 to +1211
Copy link
Member

@jsiirola jsiirola Jan 12, 2026

Choose a reason for hiding this comment

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

Quick question: this tests the case where all the variables are defined by the subtree being transformed. What happens if one of the Vars is defined outside the subtree (e.g., on m instead of under m.b)?

Copy link
Author

@bammari bammari Jan 12, 2026

Choose a reason for hiding this comment

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

@jsiirola In this example, if we defined variable x as m.x instead of m.b.x, the transformed expression would be:

0.5 * m.x + 3 - m.b.y <= 3.5 * m.b.dis2.binary_indicator_var instead of

0.5 * m.b.x + 3 - m.b.y <= 3.5 * m.b.dis2.binary_indicator_var

Do you see any potential issues with this?

Copy link
Member

@jsiirola jsiirola Jan 12, 2026

Choose a reason for hiding this comment

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

Nope - that should be fine. I don't think it will be a huge issue, as the change this PR is making doesn't actually affect Vars (I wasn't thinking clearly last night -- you are only getting relative names for active components [Constraints & Disjuncts] - and not Vars).

BUT, this does raise the question: what do we expect the following to do??

m.d1 = Disjunct()
m.b = Block()
m.b.d2 = Disjunct()
m.b.disj = Disjunction(expr=[m.d1, m.b.d2])
TransformationFactory('gdp.mbigm').apply_to(m.b, threads=2)

Thoughts? @emma58?

Copy link
Member

@jsiirola jsiirola Jan 12, 2026

Choose a reason for hiding this comment

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

(Spot checking... this update to mbigm leads to an exception. bigm will transform the model (including deactivating m.d1)

Copy link
Contributor

@emma58 emma58 Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

(@emma58 crawls under a table...)

(From under the table,) we expect the model above to put the transformed constraints on m.b, and to deactivate m.d1 and m.b.d2. Scoping should be determined by the Disjunction and not care about where the Disjuncts are living at all. So basically, the bigm behavior is the desired behavior. I can look at this...


mbm = TransformationFactory('gdp.mbigm')
mbm.apply_to(m.b, threads=1)
dis1_cons = mbm.get_transformed_constraints(m.b.dis1.linear)
assertExpressionsEqual(
self,
dis1_cons[0].expr,
2.0 * m.b.dis2.binary_indicator_var <= 0.5 * m.b.x + 3 - m.b.y,
)
assertExpressionsEqual(
self,
dis1_cons[1].expr,
0.5 * m.b.x + 3 - m.b.y <= 3.5 * m.b.dis2.binary_indicator_var,
)