-
Notifications
You must be signed in to change notification settings - Fork 234
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
Add support for Greybox model DOF counitng in degrees_of_freedom function #1512
base: main
Are you sure you want to change the base?
Changes from 2 commits
fb0f8fd
a719e47
9a09cc0
4a25d72
2324cbb
01c3a8a
e67e56b
f6a36e6
880a1be
75cc0c1
03291fa
4b6de4c
d9ff557
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ | |
from pyomo.core.expr import identify_variables | ||
from pyomo.common.collections import ComponentSet | ||
from pyomo.common.deprecation import deprecation_warning | ||
from pyomo.contrib.pynumero.interfaces.external_grey_box import ExternalGreyBoxBlock | ||
|
||
import idaes.logger as idaeslog | ||
|
||
|
@@ -377,7 +378,31 @@ def number_activated_equalities(block): | |
Returns: | ||
Number of activated equality Constraint components in block | ||
""" | ||
return sum(1 for _ in activated_equalities_generator(block)) | ||
return sum( | ||
1 for _ in activated_equalities_generator(block) | ||
) + number_grey_box_equalities(block) | ||
|
||
|
||
def number_grey_box_equalities(block): | ||
""" | ||
Method to return the number of equality constraints in GreyBox | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say, "Compute total number of equality constraints for all GreyBox objects in this block." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
A GreyBox model is always assumed to be 0DOFs where each output[i]==f(inputs) | ||
where f is GreyBox model, this should be true regardless if | ||
GreyBox model is doing internal optimization or not, as every output | ||
is calculated through a the GreyBox internal model using provided inputs. | ||
Args: | ||
block : model to be studied | ||
Returns: | ||
Number of activated equality Constraint components in block | ||
""" | ||
equalities = 0 | ||
for grey_box in _iter_indexed_block_data_objects( | ||
block, ctype=ExternalGreyBoxBlock, active=True, descend_into=True | ||
): | ||
equalities += len(grey_box.outputs) | ||
return equalities | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As written, this will be incorrect for grey box models that have a nonzero number of "explicitly defined equality constraints" (as opposed to the equality constraints that are created to define the outputs). I think all that's needed to correct this is: equalities += grey_box.get_external_model().n_equality_constraints() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, did not think about this scenario! Updated to test graybox with suggestion, this now includes an example constraint that binds 2 inputs together.
|
||
|
||
|
||
def deactivated_equalities_generator(block): | ||
|
@@ -529,7 +554,7 @@ def deactivated_inequalities_generator(block): | |
block : model to be studied | ||
Returns: | ||
A generator which returns all indeactivated equality Constraint | ||
A generator which returns all in deactivated equality Constraint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "in" should be removed here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
components block | ||
""" | ||
for c in total_inequalities_generator(block): | ||
|
@@ -1034,6 +1059,19 @@ def unfixed_variables_in_activated_equalities_set(block): | |
for v in variables_in_activated_equalities_set(block): | ||
if not v.fixed: | ||
var_set.add(v) | ||
|
||
# Checks for greyboxes, and if they exist will add | ||
# input and output vars to var_set if they are free | ||
# inputs and outputs are defined names for greybox class and should always exist | ||
for grey_box in _iter_indexed_block_data_objects( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be good to have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added one for variables, and unfixed variables. |
||
block, ctype=ExternalGreyBoxBlock, active=True, descend_into=True | ||
): | ||
for in_var in grey_box.inputs: | ||
if not grey_box.inputs[in_var].fixed: | ||
var_set.add(grey_box.inputs[in_var]) | ||
for out_var in grey_box.outputs: | ||
if not grey_box.outputs[out_var].fixed: | ||
var_set.add(grey_box.outputs[out_var]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, I think you need to make sure that these variables don't already appear in |
||
return var_set | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,10 @@ | |
|
||
from idaes.core.util.model_statistics import * | ||
from idaes.core.util.model_statistics import _iter_indexed_block_data_objects | ||
from pyomo.contrib.pynumero.interfaces.external_grey_box import ( | ||
ExternalGreyBoxBlock, | ||
ExternalGreyBoxModel, | ||
) | ||
|
||
|
||
@pytest.mark.unit | ||
|
@@ -685,6 +689,42 @@ def test_degrees_of_freedom(m): | |
assert degrees_of_freedom(m.b2) == -1 | ||
|
||
|
||
@pytest.mark.unit | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also have test for the functions to collect the number of greybox variables and constraints. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added tests |
||
def test_degrees_of_freedom_with_graybox(): | ||
"""non functional graybox model added to m fixture, to test DOFs | ||
GreyBoxModel has 3 inputs and 2 outputs calculated an unknown function""" | ||
|
||
class BasicGrayBox(ExternalGreyBoxModel): | ||
def input_names(self): | ||
return ["a1", "a2", "a3"] | ||
|
||
def output_names(self): | ||
return ["o1", "o2"] | ||
|
||
m = ConcreteModel() | ||
|
||
m.gb = ExternalGreyBoxBlock(external_model=BasicGrayBox()) | ||
# verify DOFS works on stand alone greybox | ||
assert degrees_of_freedom(m) == 3 | ||
m.gb.inputs.fix() | ||
assert degrees_of_freedom(m) == 0 | ||
m.gb.outputs.fix() | ||
assert degrees_of_freedom(m) == -2 | ||
m.gb.outputs.unfix() | ||
|
||
# verify DOFs works on greybox connected to other vars on a model via constraints | ||
m.a1 = Var(initialize=1) | ||
m.a1.fix() | ||
m.gb.inputs["a1"].unfix() | ||
m.a1_eq = Constraint(expr=m.a1 == m.gb.inputs["a1"]) | ||
assert degrees_of_freedom(m) == 0 | ||
m.o1 = Var(initialize=1) | ||
m.o1_eq = Constraint(expr=m.o1 == m.gb.outputs["o1"]) | ||
m.o1.fix() | ||
assert degrees_of_freedom(m) == -1 | ||
|
||
|
||
@pytest.mark.unit | ||
def test_large_residuals_set(m): | ||
# Initialize derivative var values so no errors occur | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
number_of_X
is less ambiguous in English.Can add
-> int
to indicate return type and, if it's possible, the expected type ofblock
.In docs: "Method to" is superfluous and at any rate this is a function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done