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.

Sign up for GitHub

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

Enable derived costing package to define a flow cost #983

Merged
merged 3 commits into from
Oct 6, 2022
Merged

Enable derived costing package to define a flow cost #983

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f30dcb
enabling package to already define a flow cost
bknueven Oct 5, 2022
f61bb43
add RuntimeError and associated test
bknueven Oct 6, 2022
ff7ca31
extending docstring to describe new behavior
bknueven Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions idaes/core/base/costing_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,13 +337,19 @@ def register_flow_type(self, flow_type, cost):
"""
self.flow_types.add(flow_type)

# Create a Var to hold the cost
# Units will be different between flows, so have to use scalar Vars
fvar = pyo.Var(
units=pyo.units.get_units(cost), doc=f"Cost parameter for {flow_type} flow"
)
self.add_component(f"{flow_type}_cost", fvar)
fvar.fix(cost)
name = f"{flow_type}_cost"
current_component = self.component(name)
if current_component is not None and current_component is cost:
pass
else:
Copy link
Member

Choose a reason for hiding this comment

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

I think I see an edge case here that needs to be caught - what happens if a component name already exists but is not cost? I think this would implicitly replace the existing component, so I think we need to catch this and raise an Exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree -- I was just making a minimal change to existing behavior. Happy to add an Exception here though.

Copy link
Member

Choose a reason for hiding this comment

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

I think an Exception is the best approach - it is easy to add in here, and I generally never try to second guess the user. I.e. if they do something wrong it is best to tell them to fix it rather than make assumptions.

# Create a Var to hold the cost
# Units will be different between flows, so have to use scalar Vars
fvar = pyo.Var(
units=pyo.units.get_units(cost),
doc=f"Cost parameter for {flow_type} flow",
)
self.add_component(name, fvar)
fvar.fix(cost)

self._registered_flows[flow_type] = []

Expand Down
24 changes: 19 additions & 5 deletions idaes/core/base/tests/test_costing_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"""
import pytest

from pyomo.environ import ConcreteModel, Constraint, Set, units as pyunits, Var
from pyomo.environ import ConcreteModel, Constraint, Set, units as pyunits, Var, Param
from pyomo.util.check_units import assert_units_consistent, assert_units_equivalent

from idaes.core import declare_process_block_class, UnitModelBlockData
Expand Down Expand Up @@ -165,7 +165,12 @@ def build_global_params(self):
self.base_currency = pyunits.USD_test
self.base_period = pyunits.year

self.defined_flows = {"test_flow_1": 0.2 * pyunits.J}
self.test_flow_2_cost = Param(initialize=0.07, units=pyunits.kW)

self.defined_flows = {
"test_flow_1": 0.2 * pyunits.J,
"test_flow_2": self.test_flow_2_cost,
}

self._bgp = True

Expand Down Expand Up @@ -219,9 +224,12 @@ def costing(self):
def test_basic_attributes(self, costing):
assert costing.costing._registered_unit_costing == []
assert isinstance(costing.costing.flow_types, Set)
assert len(costing.costing.flow_types) == 1
assert len(costing.costing.flow_types) == 2
assert "test_flow_1" in costing.costing.flow_types
assert costing.costing._registered_flows == {"test_flow_1": []}
assert costing.costing._registered_flows == {
"test_flow_1": [],
"test_flow_2": [],
}

assert costing.costing._costing_methods_map == {
TypeAData: TestCostingPackageData.method_1,
Expand All @@ -234,6 +242,8 @@ def test_basic_attributes(self, costing):
assert costing.costing.test_flow_1_cost.value == 0.2
assert_units_equivalent(costing.costing.test_flow_1_cost.get_units(), pyunits.J)

assert isinstance(costing.costing.test_flow_2_cost, Param)

# Test that build_global_parameters was called successfully
assert costing.costing._bgp

Expand All @@ -250,7 +260,11 @@ def test_register_flow_type(self, costing):
)
assert "test_flow" in costing.costing.flow_types

assert costing.costing._registered_flows == {"test_flow_1": [], "test_flow": []}
assert costing.costing._registered_flows == {
"test_flow_1": [],
"test_flow_2": [],
"test_flow": [],
}

@pytest.mark.unit
def test_cost_flow_invalid_type(self, costing):
Expand Down