Skip to content

Bugfixes for core.lp_dual transformation#3672

Merged
blnicho merged 19 commits intoPyomo:mainfrom
emma58:linear-dual-fix
Aug 6, 2025
Merged

Bugfixes for core.lp_dual transformation#3672
blnicho merged 19 commits intoPyomo:mainfrom
emma58:linear-dual-fix

Conversation

@emma58
Copy link
Copy Markdown
Contributor

@emma58 emma58 commented Aug 1, 2025

Fixes # .

Summary/Motivation:

NOTE: This is built on top of #3651, so it will be impossible to review until that is merged.

This fixes a couple bugs in the core.lp_dual transformation. In particular:

  1. The Parameterized Standard Form walker now checks if a Var is being treated as a parameter before checking if it is fixed--this is very important because it mean that we were losing fixed Vars we were treating as data, leading to unexpected behavior in dualize-and-combine methods.
  2. We now raise an error for completely empty primal models because we don't actually do the right thing in the sense that strong duality won't hold (whatever that means), so I'd rather just not do anything
  3. We now raise an error when parameterization causes a primal Constraint to become trivial because, unlike with trivial constraints not due to parameterization, we cannot just ignore them, but the behavior from taking the dual with them in
    (whatever that means) is rather unexpected, I think.
  4. We now raise an error when the primal model has discrete variables (or other domains we can't handle).

Changes proposed in this PR:

  • Fixing the three bugs above, the first of which is actually a change to parameterized standard form
  • Adding tests

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.

@blnicho blnicho moved this from Todo to Review In Progress in August 2025 Release Aug 4, 2025
@blnicho blnicho requested review from blnicho and jsiirola August 5, 2025 18:46
Copy link
Copy Markdown
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

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

Looks good. Two comments that we should consider (but it is fine to leave them for a future PR).


return dense

def tocsr(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We ought to follow scipy's lead and only implement a single method that tocsr and tocsc both use (instead of virtually duplicate code)

@github-project-automation github-project-automation bot moved this from Review In Progress to Reviewer Approved in August 2025 Release Aug 6, 2025
Copy link
Copy Markdown
Member

@blnicho blnicho left a comment

Choose a reason for hiding this comment

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

I found a few typos but otherwise this looks fine

@blnicho blnicho merged commit f95d119 into Pyomo:main Aug 6, 2025
1 of 2 checks passed
@github-project-automation github-project-automation bot moved this from Reviewer Approved to Done in August 2025 Release Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

4 participants