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

Overhaul PyROS Preprocessor Subroutine and Subproblem Objects #3341

Open
wants to merge 232 commits into
base: main
Choose a base branch
from

Conversation

shermanjasonaf
Copy link
Contributor

@shermanjasonaf shermanjasonaf commented Aug 12, 2024

Fixes #2964.

Summary/Motivation:

Driven by recent algorithmic developments of the PyROS preprocessor, this PR implements sweeping changes to the PyROS preprocessor subroutine and subproblem formulations/object structures.

Changes proposed in this PR:

Update PyROS as follows:

  • Overhaul the preprocessor
    • Leverage decision rules and equality constraints to identify second-stage variables and state variables that are mathematically nonadjustable
    • More structured methods for reformulating variable domains/bounds to constraints
    • More structured methods for rearranging inequality constraints
    • Rewrite and extend coefficient matching: carefully account for state variable-independent constraints that cannot be reformulated by casting to inequality constraint pairs
  • Update the subproblem formulations and modeling objects
    • Modify block structures of the working model, master-like models, and separation model: partition the constraints by stage
    • Change the objective of the decision rule polishing problem to the 1-norm of the nonstatic terms of the decision rule expressions
    • Perform more careful initialization of the auxiliary uncertain parameter variables of the separation problem
  • Update the UncertaintySet class and pre-implemented subclasses to facilitate changes to the subproblems
    • Modify return protocol of UncertaintySet.set_as_constraint to make auxiliary uncertain parameters easier to track
    • Add new efficient methods for calculation of auxiliary uncertain parameter values for the CardinalitySet and FactorModelSet classes
    • Restrict range of valid FactorModelSet instances to those for which psi_mat is full column rank
    • Bug fix(es) for the IntersectionSet
  • Logging updates
    • Make log of the model component statistics slightly more informative
    • Add DEBUG-level messages for logging of the individual preprocessor steps
  • Documentation updates
    • Update "Methodology Overview" section
    • Add section on PyROS installation instructions
    • Adapt PyROS documentation to reorganized Pyomo documentation (post Reorganize online documentation #3382)
  • Make tests more rigorous and extensive
    • Address Restore PyROS failed separation problem test #2964
    • Use expression comparison methods (core.expr.compare.assertExpressionsEqual) for more precise model testing
    • Separate testing modules for the:
      • preprocessor
      • subproblem formulations
      • uncertainty set classes
        • more rigorous testing for point_in_set, set_as_constraint
      • PyROS solver as a whole

TODO

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.

@shermanjasonaf
Copy link
Contributor Author

@jsiirola On my machine, the test test_grcs::RegressionTest::test_discrete_separation_invalid_value_error fails; this appears to be a consequence of #3386.

@shermanjasonaf
Copy link
Contributor Author

That's it for updates post #3382.

Copy link
Contributor

@mrmundt mrmundt left a comment

Choose a reason for hiding this comment

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

I don't have anything that actually needs to be changed, just some stylistic suggestions. Overall it looks good.

Comment on lines 266 to 267
m.con1 = Constraint(expr=m.x1 * m.u1 ** (0.5) - m.x2 * m.u1 <= 2)
m.con2 = Constraint(expr=m.x1**2 - m.x2**2 * m.u1 == m.x3)
Copy link
Contributor

Choose a reason for hiding this comment

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

A suggestion that won't hold up this PR - it looks like you have a similar (if not exactly the same) model across multiple test suites. It is more maintainable to define a "model-creation" function that you can call so you don't have duplicate code.

Comment on lines 335 to 337
m.con1 = Constraint(expr=m.x1 * m.u1 ** (0.5) - m.x2 * m.u1 <= 2)
m.con2 = Constraint(expr=m.x1**2 - m.x2**2 * m.u1 == m.x3)

Copy link
Contributor

Choose a reason for hiding this comment

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

(An example of the repeat I mentioned above)



import logging
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general note across all the tests - you probably want to do:

import pyomo.common.unittest as unittest

We have a lot of extra capabilities in addition to the standard unittest offerings that are Pyomo-specific and generally helpful.


import logging
import textwrap
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general note across all the tests - you probably want to do:

import pyomo.common.unittest as unittest

We have a lot of extra capabilities in addition to the standard unittest offerings that are Pyomo-specific and generally helpful.


import logging
import time
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general note across all the tests - you probably want to do:

import pyomo.common.unittest as unittest

We have a lot of extra capabilities in addition to the standard unittest offerings that are Pyomo-specific and generally helpful.

param_var_data_list : list of VarData
Variable data objects representing the main uncertain
parameters.
conlist : ConstraintList
Copy link
Contributor

Choose a reason for hiding this comment

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

The nit-pickiest of nit-picks: con_list (because you use underscores in param_var_data_list and auxiliary_var_list)

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.

Restore PyROS failed separation problem test
5 participants