Skip to content

Dev/ldalgs dynamic - #10

Open
Toflamus wants to merge 4 commits into
AlbertLee125:LDBD_Devfrom
Toflamus:dev/ldalgs-dynamic
Open

Dev/ldalgs dynamic#10
Toflamus wants to merge 4 commits into
AlbertLee125:LDBD_Devfrom
Toflamus:dev/ldalgs-dynamic

Conversation

@Toflamus

@Toflamus Toflamus commented Mar 6, 2026

Copy link
Copy Markdown
Collaborator

Fixes # .

Summary/Motivation:

This is the implementation of the dynamic cases.

Changes proposed in this PR:

  • Add code that is compatible with the DAE package
  • Refactor the previous tests to the new implementation

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR moves the DAE (Differential-Algebraic Equations) compatibility workaround for Pyomo issue #3101 from user-space test code into the GDPopt discrete solvers (LD-SDA and LD-BD). Previously, users had to manually reconstruct constraints inside Disjunct components after applying dae.collocation, due to a bug where the DAE discretization transformation didn't properly expand constraints inside disjuncts. This PR makes the solvers handle that step automatically.

Changes:

  • algorithm_base_class.py: Adds _has_dae_components (detects ContinuousSet/DerivativeVar) and _reconstruct_disjunct_constraints_if_dae (performs the constraint reconstruction), plus adds Disjunct and Constraint imports.
  • discrete_algorithm_base_class.py: Adds _ensure_dae_compatibility, a thin wrapper calling the base class reconstruction method.
  • ldsda.py / ldbd.py: Calls _ensure_dae_compatibility on the cloned working model during solver setup.
  • test_ldsda.py / test_ldbd.py: Removes the now-redundant manual workaround from the integration tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pyomo/contrib/gdpopt/algorithm_base_class.py Adds DAE detection and constraint-reconstruction helpers; adds Disjunct/Constraint imports
pyomo/contrib/gdpopt/discrete_algorithm_base_class.py Adds _ensure_dae_compatibility wrapper delegating to the base class method
pyomo/contrib/gdpopt/ldsda.py Calls _ensure_dae_compatibility on working model after cloning
pyomo/contrib/gdpopt/ldbd.py Calls _ensure_dae_compatibility on working model after cloning
pyomo/contrib/gdpopt/tests/test_ldsda.py Removes manual DAE workaround from integration test
pyomo/contrib/gdpopt/tests/test_ldbd.py Removes manual DAE workaround from integration test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pyomo/contrib/gdpopt/algorithm_base_class.py Outdated
Comment thread pyomo/contrib/gdpopt/discrete_algorithm_base_class.py
Comment on lines +194 to +231
def _has_dae_components(self, model):
try:
from pyomo.dae import ContinuousSet, DerivativeVar
except Exception:
return False

if any(
model.component_data_objects(ctype=ContinuousSet, descend_into=True)
):
return True
if any(
model.component_data_objects(ctype=DerivativeVar, descend_into=True)
):
return True
return False

def _reconstruct_disjunct_constraints_if_dae(self, model, logger=None):
if not self._has_dae_components(model):
return False

disjuncts = list(
model.component_data_objects(ctype=Disjunct, descend_into=True)
)
if not disjuncts:
return False

for disjunct in disjuncts:
for constraint in disjunct.component_objects(
ctype=Constraint, descend_into=True
):
constraint._constructed = False
constraint.construct()

if logger is not None:
logger.debug(
"Reconstructed disjunct constraints after DAE discretization."
)
return True

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

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

The new methods _has_dae_components and _reconstruct_disjunct_constraints_if_dae manipulate Pyomo's private attribute _constructed and are central to the new DAE compatibility feature. There are no unit tests for these methods in isolation. The only coverage comes from integration tests that require external solvers (appsi_highs and ipopt). Given that similar methods in the same file (e.g., _get_external_information, neighbor_search) have dedicated unit tests, these two new methods should also have unit tests that verify:

  1. _has_dae_components returns True for a model with ContinuousSet, and False for a plain model.
  2. _reconstruct_disjunct_constraints_if_dae correctly reconstructs disjunct constraints after discretization without requiring an external solver.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants