-
Notifications
You must be signed in to change notification settings - Fork 9
Feature/300 piecewiseeffects for flows #310
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
base: main
Are you sure you want to change the base?
Changes from 23 commits
c402d57
e707d0a
3863102
ca6424f
69791d7
10635f8
a0a5785
3a88a59
c4db3b7
12fdae7
bcca8a2
1914b70
ed8a8f2
cc25934
6d73883
5ef56a5
d5e8e24
14505bb
dd6ea15
fb13157
f7658dd
380907e
ca1a9f0
109c6ab
454fe19
bd40414
fc71d85
ffd6b92
5ae6b05
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 |
|---|---|---|
|
|
@@ -12,8 +12,8 @@ | |
| from .config import CONFIG | ||
| from .core import NumericData, NumericDataTS, PlausibilityError, Scalar, TimeSeriesCollection | ||
| from .effects import EffectValuesUser | ||
| from .features import InvestmentModel, OnOffModel, PreventSimultaneousUsageModel | ||
| from .interface import InvestParameters, OnOffParameters | ||
| from .features import InvestmentModel, OnOffModel, PiecewiseEffectsPerFlowHourModel, PreventSimultaneousUsageModel | ||
| from .interface import InvestParameters, OnOffParameters, PiecewiseEffectsPerFlowHour | ||
| from .structure import Element, ElementModel, SystemModel, register_class_for_io | ||
|
|
||
| if TYPE_CHECKING: | ||
|
|
@@ -159,6 +159,7 @@ def __init__( | |
| relative_minimum: NumericDataTS = 0, | ||
| relative_maximum: NumericDataTS = 1, | ||
| effects_per_flow_hour: Optional[EffectValuesUser] = None, | ||
| piecewise_effects_per_flow_hour: Optional[PiecewiseEffectsPerFlowHour] = None, | ||
| on_off_parameters: Optional[OnOffParameters] = None, | ||
| flow_hours_total_max: Optional[Scalar] = None, | ||
| flow_hours_total_min: Optional[Scalar] = None, | ||
|
|
@@ -180,6 +181,7 @@ def __init__( | |
| def: :math:`load\_factor:= sumFlowHours/ (nominal\_val \cdot \Delta t_{tot})` | ||
| load_factor_max: maximal load factor (see minimal load factor) | ||
| effects_per_flow_hour: operational costs, costs per flow-"work" | ||
| piecewise_effects_per_flow_hour: piecewise relation between flow hours and effects | ||
| on_off_parameters: If present, flow can be "off", i.e. be zero (only relevant if relative_minimum > 0) | ||
| Therefore a binary var "on" is used. Further, several other restrictions and effects can be modeled | ||
| through this On/Off State (See OnOffParameters) | ||
|
|
@@ -207,6 +209,7 @@ def __init__( | |
| self.load_factor_max = load_factor_max | ||
| # self.positive_gradient = TimeSeries('positive_gradient', positive_gradient, self) | ||
| self.effects_per_flow_hour = effects_per_flow_hour if effects_per_flow_hour is not None else {} | ||
| self.piecewise_effects_per_flow_hour = piecewise_effects_per_flow_hour | ||
| self.flow_hours_total_max = flow_hours_total_max | ||
| self.flow_hours_total_min = flow_hours_total_min | ||
| self.on_off_parameters = on_off_parameters | ||
|
|
@@ -248,6 +251,8 @@ def transform_data(self, flow_system: 'FlowSystem'): | |
| self.effects_per_flow_hour = flow_system.create_effect_time_series( | ||
| self.label_full, self.effects_per_flow_hour, 'per_flow_hour' | ||
| ) | ||
| if self.piecewise_effects_per_flow_hour is not None: | ||
| self.piecewise_effects_per_flow_hour.transform_data(flow_system, self.label_full) | ||
| if self.on_off_parameters is not None: | ||
| self.on_off_parameters.transform_data(flow_system, self.label_full) | ||
| if isinstance(self.size, InvestParameters): | ||
|
|
@@ -398,6 +403,21 @@ def _create_shares(self): | |
| target='operation', | ||
| ) | ||
|
|
||
| if self.element.piecewise_effects_per_flow_hour is not None: | ||
| self.piecewise_effects = self.add( | ||
| PiecewiseEffectsPerFlowHourModel( | ||
| model=self._model, | ||
| label_of_element=self.label_of_element, | ||
| piecewise_origin=( | ||
| self.flow_rate.name, | ||
| self.element.piecewise_effects_per_flow_hour.piecewise_flow_rate, | ||
| ), | ||
| piecewise_shares=self.element.piecewise_effects_per_flow_hour.piecewise_shares, | ||
| zero_point=self.on_off.on if self.on_off is not None else False, | ||
| ), | ||
| ) | ||
| self.piecewise_effects.do_modeling() | ||
|
|
||
|
Comment on lines
+406
to
+420
Contributor
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. 💡 Verification agent 🧩 Analysis chainZero-point handling risks infeasibility when On/Off is absent. Passing False forces “exactly one segment active,” disallowing zero unless 0 is inside a piece. That can make otherwise valid flows (min=0) infeasible. Recommend allowing zero by default when no On/Off is present. - zero_point=self.on_off.on if self.on_off is not None else False,
+ zero_point=self.on_off.on if self.on_off is not None else True,Would you prefer a Flow-level flag (e.g., piecewise_zero_allowed_when_off: bool = True) to make this explicit? 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Listing flixopt module files (top-level):"
ls -la flixopt || true
echo
echo "Search for Piecewise* classes / identifiers..."
rg -n --hidden --no-ignore-vcs "PiecewiseEffectsPerFlowHourModel" -S || true
rg -n --hidden --no-ignore-vcs "PiecewiseEffects" -S || true
rg -n --hidden --no-ignore-vcs "piecewise_effects_per_flow_hour" -S || true
rg -n --hidden --no-ignore-vcs "piecewise" -S || true
echo
echo "Search for 'zero_point' usages:"
rg -n --hidden --no-ignore-vcs "zero_point" -S || true
echo
echo "Show flixopt/elements.py around the snippet (lines ~390-430):"
if [ -f flixopt/elements.py ]; then
nl -ba flixopt/elements.py | sed -n '390,430p' || true
else
echo "flixopt/elements.py not found"
fi
echo
echo "Search for classes with 'Piecewise' in their name (to find model implementation):"
rg -n --hidden --no-ignore-vcs "class .*Piecewise" -S || true
echo
echo "Search for constructor parameters that mention zero_point (def __init__ ... zero_point):"
rg -n --hidden --no-ignore-vcs "def __init__\([^\)]*zero_point" -S || true
echo
echo "Search for 'on_off' near piecewise-related code:"
rg -n --hidden --no-ignore-vcs "on_off" -S || trueLength of output: 53460 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== features.py 820-920 ==="
sed -n '820,920p' flixopt/features.py || true
echo
echo "=== features.py 1040-1140 ==="
sed -n '1040,1140p' flixopt/features.py || true
echo
echo "=== elements.py 392-422 ==="
sed -n '392,422p' flixopt/elements.py || true
echo
echo "=== components.py 528-540 ==="
sed -n '528,540p' flixopt/components.py || true
echo
echo "=== tests/test_linear_converter.py 420-440 ==="
sed -n '420,440p' tests/test_linear_converter.py || trueLength of output: 11083 Allow zero when On/Off is absent — fix in PiecewiseModel, not by passing True at call sites Confirmed: call sites (e.g. flixopt/elements.py) pass False when no On/Off; PiecewiseModel.do_modeling treats non-True/non-variable as sum(insides) == 1, forcing a segment and making flows with min=0 infeasible. Switching call-sites to True would create an unconstrained binary zero_point (undesirable). Recommended minimal fix: in flixopt/features.py -> PiecewiseModel.do_modeling change the else branch from Would you prefer an explicit Flow-level flag (e.g. piecewise_zero_allowed_when_off: bool) instead of changing the default? |
||
| def _create_bounds_for_load_factor(self): | ||
| # TODO: Add Variable load_factor for better evaluation? | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ | |
| from . import utils | ||
| from .config import CONFIG | ||
| from .core import NumericData, Scalar, TimeSeries | ||
| from .interface import InvestParameters, OnOffParameters, Piecewise | ||
| from .interface import InvestParameters, OnOffParameters, Piece, Piecewise | ||
| from .structure import Model, SystemModel | ||
|
|
||
| logger = logging.getLogger('flixopt') | ||
|
|
@@ -841,7 +841,7 @@ def __init__( | |
| label: str = '', | ||
| ): | ||
| """ | ||
| Modeling a Piecewise relation between miultiple variables. | ||
| Modeling a Piecewise relation between multiple variables. | ||
| The relation is defined by a list of Pieces, which are assigned to the variables. | ||
| Each Piece is a tuple of (start, end). | ||
|
|
||
|
|
@@ -850,7 +850,9 @@ def __init__( | |
| label_of_element: The label of the parent (Element). Used to construct the full label of the model. | ||
| label: The label of the model. Used to construct the full label of the model. | ||
| piecewise_variables: The variables to which the Pieces are assigned. | ||
| zero_point: A variable that can be used to define a zero point for the Piecewise relation. If None or False, no zero point is defined. | ||
| zero_point: A variable that can be used to define a zero point for the Piecewise relation. | ||
| If None or False, no zero point is defined. THis leads to 0 not being possible, | ||
| unless its its contained in a Piece. | ||
| as_time_series: Whether the Piecewise relation is defined for a TimeSeries or a single variable. | ||
| """ | ||
| super().__init__(model, label_of_element, label) | ||
|
|
@@ -896,21 +898,23 @@ def do_modeling(self): | |
| # b) eq: -On(t) + Segment1.onSeg(t) + Segment2.onSeg(t) + ... = 0 zusätzlich kann alles auch Null sein | ||
| if isinstance(self._zero_point, linopy.Variable): | ||
| self.zero_point = self._zero_point | ||
| rhs = self.zero_point | ||
| sign, rhs = '<=', self.zero_point | ||
| elif self._zero_point is True: | ||
| self.zero_point = self.add( | ||
| self._model.add_variables( | ||
| coords=self._model.coords, binary=True, name=f'{self.label_full}|zero_point' | ||
| ), | ||
| 'zero_point', | ||
| ) | ||
| rhs = self.zero_point | ||
| sign, rhs = '<=', self.zero_point | ||
| else: | ||
| rhs = 1 | ||
| sign, rhs = '=', 1 | ||
|
FBumann marked this conversation as resolved.
|
||
|
|
||
|
Comment on lines
900
to
912
Contributor
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. 💡 Verification agent 🧩 Analysis chainBug: single-segment constraint should be equality, not ≤. With “<=”, on=1 still allows selecting no segment, decoupling the variable from the piecewise definition (variable can collapse to 0). Use equality to enforce “exactly one segment when allowed.” - self.zero_point = self._zero_point
- sign, rhs = '<=', self.zero_point
+ self.zero_point = self._zero_point
+ sign, rhs = '==', self.zero_point
@@
- sign, rhs = '<=', self.zero_point
+ sign, rhs = '==', self.zero_point
@@
- sign, rhs = '=', 1
+ sign, rhs = '=', 1If you intended “≤” to allow both 0 and 1 segments when on=1, please confirm—this weakens the piecewise linkage. 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Searching repo for 'zero_point' occurrences ==="
rg -n --hidden -S 'zero_point' || true
if [ -f flixopt/features.py ]; then
echo "=== Showing flixopt/features.py (lines 820-940) ==="
nl -ba flixopt/features.py | sed -n '820,940p'
else
echo "=== flixopt/features.py not found; listing files with 'features.py' in name ==="
rg -n --hidden --hidden 'features.py' || true
fiLength of output: 2206 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Showing flixopt/features.py (lines 760-1140) ==="
sed -n '760,1140p' flixopt/features.py || true
echo
echo "=== Occurrences of sign/rhs/zero_point/on_off in flixopt/features.py (with context) ==="
rg -n -C3 -e 'sign, rhs' -e 'zero_point' -e 'on_off' flixopt/features.py || true
echo
echo "=== All occurrences of the token 'sign' with context ==="
rg -n -C5 '\bsign\b' flixopt/features.py || trueLength of output: 21979 Enforce equality for single-segment when zero_point exists. Change the constraint sign from '<=' to '=' when self._zero_point is a Variable or True so the constraint is sum(pieces.inside_piece) = zero_point (otherwise on/zero_point = 1 can still permit zero segments). File: flixopt/features.py (around lines 899–912) — update both branches that set sign = '<=' to '='; keep the else branch as '=' 1. 🤖 Prompt for AI Agents |
||
| self.add( | ||
| self._model.add_constraints( | ||
| sum([piece.inside_piece for piece in self.pieces]) <= rhs, | ||
| sum([piece.inside_piece for piece in self.pieces]), | ||
| sign, | ||
| rhs, | ||
| name=f'{self.label_full}|{variable.name}|single_segment', | ||
| ), | ||
| f'{var_name}|single_segment', | ||
|
|
@@ -1079,6 +1083,65 @@ def do_modeling(self): | |
| ) | ||
|
|
||
|
|
||
| class PiecewiseEffectsPerFlowHourModel(Model): | ||
| def __init__( | ||
| self, | ||
| model: SystemModel, | ||
| label_of_element: str, | ||
| piecewise_origin: Tuple[str, Piecewise], | ||
| piecewise_shares: Dict[str, Piecewise], | ||
| zero_point: Optional[Union[bool, linopy.Variable]], | ||
| label: str = 'PiecewiseEffectsPerFlowHour', | ||
| ): | ||
| super().__init__(model, label_of_element, label) | ||
| assert len(piecewise_origin[1]) == len(list(piecewise_shares.values())[0]), ( | ||
| 'Piece length of variable_segments and share_segments must be equal' | ||
| ) | ||
| self._zero_point = zero_point | ||
| self._piecewise_origin = piecewise_origin | ||
| self._piecewise_shares = piecewise_shares | ||
|
|
||
| self.shares: Dict[str, linopy.Variable] = {} | ||
|
|
||
| self.piecewise_model: Optional[PiecewiseModel] = None | ||
|
|
||
| def do_modeling(self): | ||
| self.shares = { | ||
| effect: self.add( | ||
| self._model.add_variables(coords=self._model.coords, name=f'{self.label_full}|{effect}'), f'{effect}' | ||
| ) | ||
| for effect in self._piecewise_shares | ||
| } | ||
|
|
||
| piecewise_variables = { | ||
| self._piecewise_origin[0]: self._piecewise_origin[1], | ||
| **{ | ||
| self.shares[effect_label].name: self._piecewise_shares[effect_label] | ||
| for effect_label in self._piecewise_shares | ||
| }, | ||
| } | ||
|
|
||
| self.piecewise_model = self.add( | ||
| PiecewiseModel( | ||
| model=self._model, | ||
| label_of_element=self.label_of_element, | ||
| piecewise_variables=piecewise_variables, | ||
| zero_point=self._zero_point, | ||
| as_time_series=True, | ||
| label='PiecewiseEffectsPerFlowHour', | ||
| ) | ||
| ) | ||
|
|
||
| self.piecewise_model.do_modeling() | ||
|
|
||
| # Shares | ||
| self._model.effects.add_share_to_effects( | ||
| name=self.label_of_element, | ||
| expressions={effect: variable * self._model.hours_per_step for effect, variable in self.shares.items()}, | ||
| target='operation', | ||
| ) | ||
|
|
||
|
|
||
| class PreventSimultaneousUsageModel(Model): | ||
| """ | ||
| Prevents multiple Multiple Binary variables from being 1 at the same time | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.