GDP: Fix bug transforming Blocks in gdp.mbigm transformation

[bammari]

Fixes # .

The multiple big-M transformation fails to transform blocks.

Summary/Motivation:

In many cases, we may want to apply the multiple big-M transformation on blocks rather than an entire model.

For example, prior to this PR the following model would result in a failed transformation:

import pyomo.environ as pyo
from pyomo.gdp import Disjunction, Disjunct

m = pyo.ConcreteModel()
m.b = pyo.Block()
m.b.x = pyo.Var(bounds=(0, 5))
m.b.y = pyo.Var()
m.b.dis1 = Disjunct()
m.b.dis2 = Disjunct()

m.b.dis1.linear = pyo.Constraint(expr = m.b.x*0.5 + 3 == m.b.y)
m.b.dis2.linear = pyo.Constraint(expr = m.b.x*0.2 + 1 == m.b.y)
m.b.d = Disjunction(expr = [m.b.dis1, m.b.dis2])

pyo.TransformationFactory('gdp.mbigm').apply_to(m.b, threads=1)

Changes proposed in this PR:

• Add argument relative_to=instance on constraint.getname() and other_disjunct.getname() when setting up jobs for calculating M parameters.
• Add a test that applies the multiple big-M transformation on a block to ensure no failures occur.

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

1. I agree my contributions are submitted under the BSD license.
2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

[emma58]

Thank you so much for fixing the bug, @bammari! Do you have time to add a few assertions about the transformed model to your test?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.23%. Comparing base (168e961) to head (e4986a0).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3811      +/-   ##
==========================================
- Coverage   89.41%   89.23%   -0.19%     
==========================================
  Files         909      909              
  Lines      105579   105579              
==========================================
- Hits        94408    94212     -196     
- Misses      11171    11367     +196     

Flag	Coverage Δ
builders	29.12% <ø> (+<0.01%)	⬆️
default	86.04% <ø> (?)
expensive	35.76% <ø> (?)
linux	84.91% <ø> (-4.28%)	⬇️
linux_other	84.91% <ø> (-1.82%)	⬇️
osx	82.88% <ø> (ø)
win	84.98% <ø> (ø)
win_other	84.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jsiirola]

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)?

[bammari]

@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?

[jsiirola]

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?

[jsiirola]

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

[emma58]

(@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...