Feature/rename on off to status #477
Conversation
## Parameters Class (`OnOffParameters` → `StatusParameters`) | Current Name | Recommended Name | Rationale | |--------------|------------------|-----------| | `OnOffParameters` | **`StatusParameters`** | Aligns with PyPSA, clearer semantics | | `effects_per_switch_on` | **`effects_per_startup`** | Standard UC terminology | | `effects_per_running_hour` | **`effects_per_active_hour`** | Clear, concise, matches "active" state | | `on_hours_total_min` | **`active_hours_min`** | Total (not consecutive) active hours | | `on_hours_total_max` | **`active_hours_max`** | Total (not consecutive) active hours | | `consecutive_on_hours_min` | **`min_uptime`** | Standard UC term (consecutive) | | `consecutive_on_hours_max` | **`max_uptime`** | Standard UC term (consecutive) | | `consecutive_off_hours_min` | **`min_downtime`** | Standard UC term (consecutive) | | `consecutive_off_hours_max` | **`max_downtime`** | Standard UC term (consecutive) | | `switch_on_total_max` | **`startup_limit`** | Clearer intent, matches "startup" | | `force_switch_on` | **`force_startup_tracking`** | More explicit about what is forced | ## Model Class (`OnOffModel` → `StatusModel`) ### Class Name | Current Name | Recommended Name | |--------------|------------------| | `OnOffModel` | **`StatusModel`** | ### Constructor Parameters | Current Name | Recommended Name | Rationale | |--------------|------------------|-----------| | `on_variable` | **`status`** | Aligns with PyPSA and literature | | `previous_states` | **`previous_status`** | Consistency with status variable | ### Variables (short_name in add_variables/expression_tracking_variable) | Current Name | Recommended Name | Type | Notes | |--------------|------------------|------|-------| | `self.on` | **`self.status`** | Input variable | Main binary state variable | | `'off'` | **Remove variable** | Binary variable | Replace with expression `1 - status` | | `'switch\|on'` | **`'startup'`** | Binary variable | Startup event indicator | | `'switch\|off'` | **`'shutdown'`** | Binary variable | Shutdown event indicator | | `'switch\|count'` | **`'startup_count'`** | Integer variable | Number of startups | | `'on_hours_total'` | **`'active_hours'`** | Continuous variable | Total active duration | | `'consecutive_on_hours'` | **`'uptime'`** | Continuous variable | Consecutive active hours | | `'consecutive_off_hours'` | **`'downtime'`** | Continuous variable | Consecutive inactive hours | ### Properties | Current Name | Recommended Name | Returns | Meaning | |--------------|------------------|---------|---------| | `on_hours_total` | **`active_hours`** | `linopy.Variable` | Total active hours | | `off` | **Remove property** | — | Use `1 - status` expression | | `switch_on` | **`startup`** | `linopy.Variable \| None` | Startup events | | `switch_off` | **`shutdown`** | `linopy.Variable \| None` | Shutdown events | | `switch_on_nr` | **`startup_count`** | `linopy.Variable \| None` | Number of startups | | `consecutive_on_hours` | **`uptime`** | `linopy.Variable \| None` | Consecutive active hours | | `consecutive_off_hours` | **`downtime`** | `linopy.Variable \| None` | Consecutive inactive hours | ### Internal Methods | Current Name | Recommended Name | |--------------|------------------| | `_get_previous_on_duration()` | **`_get_previous_uptime()`** | | `_get_previous_off_duration()` | **`_get_previous_downtime()`** | ### Internal Properties/Flags (in parameters) | Current Name | Recommended Name | |--------------|------------------| | `use_off` | **Remove** (use expression instead) | | `use_switch_on` | **`use_startup_tracking`** | | `use_consecutive_on_hours` | **`use_uptime_tracking`** | | `use_consecutive_off_hours` | **`use_downtime_tracking`** | ## Constraint Names (short_name in add_constraints) | Current Name | Recommended Name | |--------------|------------------| | `'complementary'` | **Remove** (no off variable) | | `'on_hours_total'` | **`'active_hours'`** | | `'switch\|on'`, `'switch\|off'` | **`'startup'`, `'shutdown'`** | | `'switch\|count'` | **`'startup_count'`** | | `'consecutive_on_hours'` | **`'uptime'`** | | `'consecutive_off_hours'` | **`'downtime'`** | ## Complete Terminology Summary (Option A) **State:** - `status` (binary): 1 = active, 0 = inactive **Events:** - `startup` (binary): transition from inactive to active - `shutdown` (binary): transition from active to inactive **Durations:** - `active_hours` (continuous): **total** hours in active state across time horizon - `uptime` (continuous): **consecutive** hours currently active (UC standard) - `downtime` (continuous): **consecutive** hours currently inactive (UC standard) **Parameter Bounds:** - `active_hours_min/max`: limits on **total** active hours - `min_uptime/max_uptime`: limits on **consecutive** active hours (UC standard) - `min_downtime/max_downtime`: limits on **consecutive** inactive hours (UC standard) - `startup_limit`: maximum number of startup events **Effects:** - `effects_per_startup`: costs/impacts per startup event - `effects_per_active_hour`: costs/impacts per active hour This aligns perfectly with PyPSA and the unit commitment literature! 🎯
1. interface.py - Module docstring now references "Status decisions"
2. components.py - Updated all docstrings:
- status_parameters parameter descriptions
- Example code updated with new parameter names (effects_per_startup, min_uptime, startup_limit)
- Fixed incorrect "OnOff feature" docstring to "Investment feature"
- Updated TODO comment to reference StatusParameters
3. linear_converters.py - All docstrings updated:
- Import statement updated to StatusParameters
- All parameter descriptions updated
- All example code updated with new terminology
4. flow_system.py - Updated references from "consecutive_on_hours" to "uptime and downtime" and on_off_parameters to status_parameters
5. modeling.py - Updated docstring from "switch-on/off variables" to "state transition constraints for binary switching variables"
Documentation Markdown Files Updated:
1. Flow.md - All references updated:
- Links to StatusParameters
- "on/off state" → "active/inactive state"
- Parameter names updated
2. StatusParameters.md (renamed from OnOffParameters.md) - Comprehensive updates:
- Title changed to "StatusParameters"
- All terminology updated: on/off → active/inactive
- Mathematical notation updated: s^on/s^off → s^startup/s^shutdown
- Duration variables: d^on/d^off → d^uptime/d^downtime
- Parameter names updated in all examples
- All Python code examples updated with new API
3. Other modeling pattern docs - Updated all references to StatusParameters and active/inactive terminology
4. mkdocs.yml - Navigation updated to reference StatusParameters.md
All docstrings and documentation now consistently use the new Status terminology aligned with PyPSA and unit commitment standards!
|
@coderabbitai review |
|
Caution Review failedThe pull request is closed. WalkthroughComprehensive terminology migration renaming binary state equipment model from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (25)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/user-guide/mathematical-notation/index.md (1)
107-111: Fix incorrect Implementation reference for state transitionsThe “State transitions” row currently links to:
[BoundingPatterns.state_transition_bounds()][flixopt.modeling.BoundingPatterns.state_transition_bounds]but the actual implementation lives on
ModelingPrimitives.state_transition_bounds()(see flixopt/modeling.py). This will break the cross-reference and mislead readers.Consider updating the Implementation column to:
-| **State transitions** | [state-transitions](modeling-patterns/state-transitions.md) | [`BoundingPatterns.state_transition_bounds()`][flixopt.modeling.BoundingPatterns.state_transition_bounds] | +| **State transitions** | [state-transitions](modeling-patterns/state-transitions.md) | [`ModelingPrimitives.state_transition_bounds()`][flixopt.modeling.ModelingPrimitives.state_transition_bounds] |flixopt/components.py (1)
571-577: Fix Transmission type hint and guard absolute_losses==0 to avoid missing status submodelsThe Transmission changes are directionally good (status-based operation and automatic status creation when absolute losses are modeled), but there are two concrete issues:
Type hint for
status_parameters(Line 667)
The signature currently usesstatus_parameters: StatusParameters = None, which is inconsistent with other constructors and technically incorrect for static checkers. It should be optional:- status_parameters: StatusParameters = None, + status_parameters: StatusParameters | None = None,Potential AttributeError when
absolute_lossesis explicitly zero
InTransmissionModel.__init__, you only attach defaultStatusParameterswhenabsolute_lossesis non-None andnp.any(element.absolute_losses != 0). However, increate_transmission_equationthe additional term is added wheneverabsolute_losses is not None:if self.element.absolute_losses is not None: con_transmission.lhs += in_flow.submodel.status.status * self.element.absolute_lossesIf a user passes
absolute_losses=0(or an array of zeros), the__init__guard will skip creating status_parameters for flows, butcreate_transmission_equationwill still try to accessin_flow.submodel.status, likely raising an AttributeError.To align behavior with the initialization logic and make
absolute_losses=0behave like “no absolute losses”, you can reuse the non-zero check:def create_transmission_equation(self, name: str, in_flow: Flow, out_flow: Flow) -> linopy.Constraint: ...
if self.element.absolute_losses is not None:con_transmission.lhs += in_flow.submodel.status.status * self.element.absolute_losses
if (self.element.absolute_losses is not Noneand np.any(self.element.absolute_losses != 0)):con_transmission.lhs += in_flow.submodel.status.status * self.element.absolute_lossesThis keeps the model robust for the `absolute_losses=0` case and avoids depending on an implicit “never pass 0, use None” convention.Also applies to: 633-637, 667-681, 733-737, 768-770
🧹 Nitpick comments (12)
docs/user-guide/mathematical-notation/index.md (1)
59-60: StatusParameters references are consistent and clear
- Swapping
OnOffParameters→StatusParametersandon_off_parameters→status_parametersin the Features list, User API note, Quick Reference, and Class Lookup keeps this page aligned with the new public API.- The “On/off operation” label plus StatusParameters link clearly communicates the concept; if you ever revisit wording, “Binary/status operation” could be slightly clearer, but it’s not required.
Also applies to: 62-62, 100-100, 122-122
flixopt/interface.py (1)
1080-1332: StatusParameters API and transform_data behavior look coherentThe new
StatusParametersclass is internally consistent and matches the intended semantics:
- Constructor and attributes cleanly separate:
- startup-related effects (
effects_per_startup),- per-active-hour effects (
effects_per_active_hour),- global utilization (
active_hours_min/max),- local uptime/downtime (
min_uptime/max_uptime,min_downtime/max_downtime),- cycling limits (
startup_limit,force_startup_tracking).transform_dataapplies the same patterns asInvestParameters:
- effects are passed through
flow_system.fit_effects_to_model_coords(...),- scalar bounds are mapped with
fit_to_model_coords(...), using['period', 'scenario']only where PS semantics are intended (active_hours_*,startup_limit), which aligns with the type hints (Numeric_PSvsNumeric_TPS).- Predicate properties are straightforward:
use_uptime_tracking/use_downtime_trackingdepend only on the presence of min/max values,use_inactiveproxiesuse_downtime_tracking, which is a reasonable indicator of when an explicit inactive variable is needed,use_startup_trackingrespectsforce_startup_trackingand falls back to checkingeffects_per_startup/startup_limitvia_has_value.Two minor, non-blocking suggestions:
Document migration from OnOffParameters
Since this is a public API rename, consider briefly mentioning in the class docstring (or in higher-level docs) thatStatusParametersreplaces the formerOnOffParametersand how common fields map (startup vs on, uptime/downtime vs consecutive_on/off). This helps external users upgrade.Optional backward-compat alias (if desired)
If you want a softer migration path, you could add a deprecatedOnOffParameters = StatusParametersalias in the public interface module (or__init__) with a deprecation warning in the docs. Not required if you’re comfortable with a breaking change.docs/user-guide/mathematical-notation/features/StatusParameters.md (1)
119-121: Optional wording tweak in minimum uptime descriptionThe phrase “at least … prior to the switch” is fine, but if you want to match the style hints you could simplify it to “at least … before the switch” or similar. Purely stylistic; the math is clear.
tests/test_component.py (1)
446-449: Transmission tests: status access path updated and consistentThe assertions now read
transmission.in1.submodel.status.status.solution.valuesinstead of an on/off-specific path, matching the new status-based submodel interface and still checking full activation where expected.The remaining assertion messages (“On does not work properly”) still use the old wording but are understandable; you can optionally update them to say “status” for consistency, though it’s not required for correctness.
Also applies to: 507-510, 585-588
examples/02_Complex/complex_example.py (1)
54-56: Checkeffects_per_startupargument type; consider mapping by effect labelUsing
StatusParameterson the boiler, CHP, andbhkw_2is consistent with the new API. One detail to verify:
- Here
effects_per_startupis passed as a scalar (0.01), whereas other examples in this codebase (e.g. theTransmissiondocstring inflixopt/components.py) use a dict mapping effect labels to values, likeeffects_per_startup={'maintenance': 0.1}.- For consistency with other effect-related parameters (e.g.
effects_per_active_hour={Costs.label: ...}) and to avoid type mismatches, you may want to use something likeeffects_per_startup={Costs.label: 0.01}(and similarly for the CHP andbhkw_2), assumingStatusParametersexpects a mapping.Please double-check the StatusParameters signature and adjust accordingly if it indeed requires a dict.
Also applies to: 72-80, 91-92, 115-116
tests/test_functional.py (2)
14-18: Top-level docstring mentionsTestStatusclass that does not existThe updated docstring bullet “Status operational constraints (
TestStatus).” suggests a dedicated test class, but the file only defines free functions for these checks. Consider either renaming this bullet to describe the group of functions or adding the corresponding test class if that’s planned.
333-369: Status-related functional tests look correct; consider minor naming cleanups
- The migration from
on_off_parameterstostatus_parameters=fx.StatusParameters(...)and fromsubmodel.on_off.*tosubmodel.status.*is consistently applied across the status tests (test_on,test_off,test_startup_shutdown,test_on_total_max,test_on_total_bounds,test_consecutive_uptime_downtime,test_consecutive_off).- The expected status patterns, off-variables (
status.off), startup events (status.startup), and cumulative constraints (active_hours_min/max, min/max uptime/downtime) are all asserted in a way that matches the described behavior.Two minor consistency points:
- In
test_startup_shutdown, the test name and comment talk about “Startup/Shutdown”, but the code still accessesboiler.Q_th.submodel.status.switch_off. If the Status model exposes ashutdownvariable (or if you intend to move away from “switch_off” terminology), it might be clearer to align the attribute name with the new terminology; otherwise, consider updating the docstring to continue using “switch off”.- In
test_consecutive_off, the docstring still says “consecutive on hours” while the test actually targets downtime viamax_downtime/min_downtimeandstatus.off. Renaming that description to “consecutive off hours” or similar would better reflect the test’s purpose.Behavior-wise, the tests look solid; these are just naming/docstring tweaks for clarity.
Also applies to: 372-421, 423-478, 481-603, 605-663, 666-733
flixopt/components.py (1)
801-805: PiecewiseConversion zero_point now correctly tied to component statusUsing
zero_point=self.status.status if self.status is not None else Falseis a sensible way to gate piecewise variables by the component’s status whenStatusParametersare present, while falling back to a constant zero point when there is no status model. Together with the TODO about reducing binary usage, this is a reasonable intermediate design and matches how the updated tests expect the piecewise model to be wired.Also applies to: 807-815
flixopt/elements.py (2)
59-62: StatusParameters plumbing in Component and Flow looks consistent; consider minor API clarity.
- The introduction of
status_parameterson bothComponentandFlow, and the correspondingtransform_datacalls, are wired coherently and mirror howInvestParametersis handled.- The plausibility checks around
fixed_relative_profile+status_parametersandrelative_minimum > 0withoutstatus_parametersare good guardrails for user modeling mistakes.- One minor point: several checks use truthiness of parameter objects (e.g.,
if self.status_parametersvsis not None). IfStatusParametersever defines__len__/__bool__, that could behave unexpectedly. Using explicitis not Nonewould be more robust and consistent with other code paths.Also applies to: 87-100, 112-116, 309-337, 427-457, 480-505, 524-535
843-887: Component-level StatusModel composition is sound; consider a less brittle previous_status interface.
- The logic to auto-attach default
StatusParameters()to flows when the component itself hasstatus_parametersor usesprevent_simultaneous_flowsensures that all relevant flows gain astatusfeature before modeling; that’s important for the mutual-exclusion constraint.- The component-level
StatusModelis constructed consistently with the flow-level one and the linking constraints between componentstatusand flowstatusvariables (single-flow equality vs. multi-flow upper/lower bounds) look correct.previous_statusaggregation builds a combined history by readingflow.submodel.status._previous_status, padding to the max length, and OR‑ing across flows. This is logically reasonable but relies on the private_previous_statusattribute ofStatusModel, which is somewhat brittle.If feasible, exposing a public
previous_status(or similar) onStatusModeland using that here would make this coupling more robust against future internal refactors.Also applies to: 889-895, 905-923
tests/test_flow.py (1)
521-582: Status-related flow tests are comprehensive and align with the new API.
- The updated tests under
TestFlowOnModelandTestFlowOnInvestModelcorrectly exercise:
- basic status behavior (
status,active_hours) and its interaction withrelative_minimum/relative_maximum,- effects per active hour and per startup via
StatusParameters.effects_per_active_hour/effects_per_startup,- uptime/downtime constraints with and without
previous_flow_rate,- startup counting via
startup_limitandstartup_count,- combinations of status with both optional and mandatory investments.
- Variable and constraint names asserted in the tests (
...|status,...|active_hours,...|uptime,...|downtime,...|startup_count, etc.) match the modeling logic inFlowModel/StatusModel.You might consider one extra test for
effects_per_startupwithoutstartup_limit(if supported byStatusParameters) to ensureuse_startup_trackingis enabled solely by the presence of startup effects as well, but overall this test suite looks strong.Also applies to: 584-642, 648-671, 683-707, 711-724, 730-805, 811-887, 893-970, 976-1032, 1038-1068, 1074-1171, 1177-1263
flixopt/features.py (1)
186-195: Consider usingNoneinstead ofnp.inffor unbounded active_hours_max.The
active_hourstracking variable currently uses:bounds = ( self.parameters.active_hours_min if self.parameters.active_hours_min is not None else 0, self.parameters.active_hours_max if self.parameters.active_hours_max is not None else np.inf, )Elsewhere (e.g.,
FlowModel.total_flow_hours) unbounded uppers are expressed withNone. For consistency with the rest of the codebase and to avoid introducing extremely large numeric bounds into the solver, you may want to switch thenp.infsentinel toNonehere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
-
CHANGELOG.md(3 hunks) -
docs/user-guide/mathematical-notation/dimensions.md(1 hunks) -
docs/user-guide/mathematical-notation/elements/Flow.md(3 hunks) -
docs/user-guide/mathematical-notation/features/OnOffParameters.md(0 hunks) -
docs/user-guide/mathematical-notation/features/StatusParameters.md(1 hunks) -
docs/user-guide/mathematical-notation/index.md(3 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/bounds-and-states.md(4 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/duration-tracking.md(1 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/index.md(2 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md(4 hunks) -
docs/user-guide/recipes/index.md(1 hunks) -
examples/02_Complex/complex_example.py(4 hunks) -
examples/02_Complex/complex_example_results.py(1 hunks) -
examples/03_Calculation_types/example_calculation_types.py(2 hunks) -
examples/04_Scenarios/scenario_example.py(2 hunks) -
examples/05_Two-stage-optimization/two_stage_optimization.py(1 hunks) -
flixopt/__init__.py(2 hunks) -
flixopt/components.py(12 hunks) -
flixopt/elements.py(21 hunks) -
flixopt/features.py(2 hunks) -
flixopt/flow_system.py(2 hunks) -
flixopt/interface.py(4 hunks) -
flixopt/linear_converters.py(20 hunks) -
flixopt/modeling.py(1 hunks) -
mkdocs.yml(1 hunks) -
tests/conftest.py(8 hunks) -
tests/test_component.py(16 hunks) -
tests/test_flow.py(24 hunks) -
tests/test_functional.py(19 hunks) -
tests/test_linear_converter.py(7 hunks) -
tests/test_scenarios.py(3 hunks) -
tests/test_storage.py(2 hunks)
💤 Files with no reviewable changes (1)
- docs/user-guide/mathematical-notation/features/OnOffParameters.md
🧰 Additional context used
🪛 LanguageTool
docs/user-guide/mathematical-notation/features/StatusParameters.md
[style] ~120-~120: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...r at least
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.10)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
🔇 Additional comments (34)
CHANGELOG.md (1)
54-101: Verify changelog completeness against actual code changes.The Unreleased section comprehensively documents the OnOff→Status rename across parameters, variables, properties, and internal names. Ensure this changelog aligns with all actual code changes across the codebase (including tests, examples, and internal modules referenced in the PR objectives).
docs/user-guide/recipes/index.md (1)
31-31: Terminology aligns with new StatusParameters namingRenaming to “Status Constraints” matches the new status/active-inactive terminology and keeps the recipes index consistent with the rest of the docs.
docs/user-guide/mathematical-notation/dimensions.md (1)
105-105: Consistent binary-state terminologySwitching from “on/off” to “active/inactive” here is consistent with StatusParameters and improves conceptual alignment without changing meaning.
flixopt/modeling.py (1)
578-589: Docstring matches implementation semanticsThe updated description (“state transition constraints for binary switching variables”) correctly reflects the role of
switch_on/switch_offand the mathematical formulation below. No implementation issues.docs/user-guide/mathematical-notation/elements/Flow.md (1)
26-27: Flow docs correctly wired to StatusParametersThe updates:
- Referencing StatusParameters in the extension paragraph,
- Mentioning “Scaled Bounds with State” in combination with StatusParameters,
- Adding the
status_parameterskey parameter,- Including StatusParameters in “See Also”,
all accurately reflect the new status/active-inactive feature. No changes needed.
Also applies to: 37-38, 51-52, 59-59
flixopt/interface.py (1)
2-4: Module docstring accurately reflects new scopeUpdating the module description to cover “Investment and Status decisions” correctly reflects the addition of StatusParameters alongside InvestParameters.
flixopt/flow_system.py (1)
53-53: LGTM! Documentation updated to reflect StatusParameters terminology.The docstring updates correctly reflect the rename from
OnOffParameterstoStatusParametersand associated terminology changes (consecutive_on_hours → uptime/downtime).Also applies to: 74-74
mkdocs.yml (1)
27-27: LGTM! Navigation updated to point to renamed documentation.The navigation entry correctly points to the new
StatusParameters.mdfile.examples/02_Complex/complex_example_results.py (1)
32-33: LGTM! Plot variable references updated to use status indicator.The heatmap data references correctly use
|statusinstead of the previous|onindicator, aligning with the StatusParameters/StatusModel terminology.docs/user-guide/mathematical-notation/modeling-patterns/duration-tracking.md (1)
157-157: LGTM! Cross-reference updated to StatusParameters.The "Used In" section correctly references
StatusParameterswith updated terminology (active/inactive instead of on/off).docs/user-guide/mathematical-notation/modeling-patterns/index.md (1)
20-20: LGTM! Documentation terminology updated consistently.Both the description and cross-reference correctly use the new StatusParameters terminology (active/inactive instead of on/off).
Also applies to: 46-46
docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md (3)
49-49: LGTM! Terminology updated to active/inactive.The description correctly uses "active/inactive switching" instead of "on/off switching".
177-185: LGTM! Code examples updated to use startup/shutdown terminology.The example code correctly demonstrates the new API using
startupandshutdownvariables instead ofswitch_onandswitch_off, aligning with the StatusParameters terminology.Also applies to: 193-200
224-224: LGTM! Cross-reference updated to StatusParameters.The "Used In" section correctly references
StatusParameterswith updated terminology.examples/03_Calculation_types/example_calculation_types.py (1)
82-83: LGTM! Examples updated to use StatusParameters API.The example code correctly uses:
status_parametersinstead ofon_off_parametersfx.StatusParametersconstructoreffects_per_startupfield instead ofeffects_per_switch_onAll changes align with the new StatusParameters API.
Also applies to: 91-91
docs/user-guide/mathematical-notation/modeling-patterns/bounds-and-states.md (2)
28-28: LGTM! Bounds with State documentation updated consistently.The description correctly uses "active/inactive operation" terminology and references the updated
StatusParametersfeature.Also applies to: 48-48
81-81: LGTM! Scaled Bounds with State documentation updated consistently.The description correctly uses "active/inactive behavior" terminology and both cross-references properly link to
StatusParameters.Also applies to: 110-111
flixopt/__init__.py (1)
31-51: Public API now exposesStatusParametersinstead ofOnOffParametersImport and
__all__are correctly updated, but this removesOnOffParametersfrom the public surface and will break externalfrom flixopt import OnOffParametersimports. Please confirm this rename is documented (changelog, migration notes) and that the intended version bump reflects the API break.examples/04_Scenarios/scenario_example.py (1)
118-137: Scenario example correctly migrated tostatus_parametersUsing
status_parameters=fx.StatusParameters()onQ_thandP_elaligns this example with the new StatusParameters-based API and the tests; no issues spotted.tests/conftest.py (1)
129-145: Fixture StatusParameters configuration looks consistent; double attachment worth confirmingAll these fixtures use
StatusParameterswith the expected fields (effects_per_active_hour,effects_per_startup,active_hours_*,min/max_uptime,max_downtime,startup_limit), and the rename from OnOffParameters is applied cleanly.In
Converters.Boilers.complex,Converters.CHPs.base,Converters.LinearConverters.*, andflow_system_long, some components receivestatus_parametersboth on the component and on individual flows. If that was already the case with OnOffParameters and the intention is to have separate status tracks (e.g. component-level effects vs. flow-level duration constraints), this is fine; otherwise, consider centralizing status behavior to avoid maintaining multiple binary state variables for the same physical unit.Also applies to: 149-179, 185-195, 199-207, 213-228, 231-253, 596-618
tests/test_storage.py (1)
408-426: Simultaneous charge/discharge test correctly switched tostatusvariablesThe test now checks for
SimultaneousStorage(Q_th_in)|status/|statuson out and uses their sum ≤ 1 to enforce mutual exclusivity. This matches the new status-based formulation and looks correct.examples/05_Two-stage-optimization/two_stage_optimization.py (1)
47-77: Two-stage example StatusParameters mapping is coherentThe new
status_parametersonKessel(Q_fu)andBHKW2useeffects_per_startupandmin_uptime/min_downtimein a way that matches the old startup and consecutive on/off semantics. Example remains consistent with the new API.tests/test_component.py (4)
72-177: Component with multiple flows: status & bounds-with-state checks look correct
- Status variables for each flow and the component (
TestComponent(... )|status,TestComponent|status) plus corresponding|active_hoursvariables are asserted consistently.- Lower/upper flow bounds for
Out2correctly apply the bounds-with-state pattern using the status variable (>= status * 0.3 * 300,<= status * 300 * rel_max).- The aggregation constraints
TestComponent|on|lb/ubensure the component-level status is 1 iff at least one flow is active and 0 otherwise, despite the legacy “|on|” name in the constraint key.This all matches the intended StatusParameters semantics; the remaining “on” wording in constraint names seems purely internal/legacy and not functionally problematic.
178-237: Single-flow component status coupling is consistentFor the single-input component:
- Variables
TestComponent(In1)|statusandTestComponent|statusare created as binaries.- Flow-rate bounds use status to gate the minimum and maximum.
- Constraint
TestComponent|statusequates component and flow status.This is the expected special case of the multi-flow formulation and looks correct.
239-357: Previous-state tests with multiple flows align status and boundsThe previous-state multi-flow test mirrors the non-previous-state test:
- Status and
active_hoursvariables/constraints for each flow and the component are present.- Bounds-with-state constraints and aggregation (
|on|lb/ub) are the same as in the simpler test, now just withprevious_flow_ratepopulated.Given the shared structure, this provides good coverage that the status logic behaves consistently when previous flow rates are nonzero.
380-417: Uptime initialization withprevious_on_hoursmatches hourly timestep assumptionThe parameterized test checks
comp.submodel.constraints['TestComponent|uptime|initial'] == comp.submodel.variables['TestComponent|uptime'].isel(time=0) == comp.submodel.variables['TestComponent|status'].isel(time=0) * (previous_on_hours + 1)With hourly timesteps in these fixtures,
previous_on_hours + 1correctly represents the total consecutive on-time including the first modeled hour, and applying it only when status att=0is 1 is sensible.If you later support non-hourly or variable time steps for these status patterns, you may want to revisit this test (and the underlying implementation) to ensure
previous_on_hoursis clearly defined in consistent units (steps vs. hours).tests/test_scenarios.py (1)
140-170: Scenario fixtures correctly migrated toStatusParameters
- The complex boiler now uses
StatusParameters(effects_per_active_hour=...)at the component level and a richerStatusParameters(...)onQ_thwithactive_hours_min/max, uptime/downtime, startup effects, andstartup_limit.- The piecewise
LinearConverterscenario fixture usesStatusParameters(effects_per_startup=0.01).These follow the same patterns as the non-scenario fixtures and keep the tests aligned with the new status-based model.
Also applies to: 213-233
tests/test_linear_converter.py (2)
137-192: StatusParameters test for LinearConverter looks consistent and valuableThe
test_linear_converter_with_statustest correctly wiresStatusParametersintoLinearConverter, checks presence ofstatusandactive_hoursvariables, validates theactive_hoursconstraint formulation, and verifies the temporal costs constraint based oneffects_per_active_hour. The naming (Converter|status,Converter|active_hours,Converter->costs(temporal)) aligns with the new status semantics and the LinearConverterModel design.
374-379: Piecewise conversion tests correctly distinguish plain vs status-aware behavior
- The updated comment and assertion for the plain piecewise case (no status parameters) correctly ensure the
single_segmentconstraint bounds the sum ofinside_piecebinaries by 1.- In
test_piecewise_conversion_with_onoff, wiringstatus_parametersintoLinearConverterand then asserting:
- existence of
Converter|status,- use of a non-None
zero_pointin the piecewise model,single_segmentRHS bounded bymodel.variables['Converter|status'],- and the
active_hoursand temporal cost constraints based onstatus
matches the intended gating of segment activation and cost calculation by the component’s status.The pattern provides good coverage of piecewise+status integration and aligns with the new StatusParameters API.
Also applies to: 381-497
flixopt/components.py (2)
18-21: LinearConverter status_parameters integration looks correctImporting
StatusParameters, documenting thestatus_parametersargument, and forwarding it fromLinearConverter.__init__intoComponent.__init__keeps the public API coherent and matches how Transmission wires its own status parameters. UsingStatusParameters | None = Nonein the signature is also type-correct withfrom __future__ import annotations.Also applies to: 51-55, 165-176
967-971: StorageModel.investment property docstring clarifiedThe
investmentproperty now clearly documents that it exposes the investment feature and gracefully returnsNonewhen absent. This is a small but helpful clarification of the public API surface.flixopt/linear_converters.py (1)
18-19: StatusParameters adoption across converter classes is consistent and non-breakingAcross
Boiler,Power2Heat,HeatPump,CoolingTower,CHP, andHeatPumpWithSource:
status_parameters: StatusParameters | None = Noneis added to each constructor and forwarded toLinearConverter.__init__via thestatus_parameterskeyword.- TYPE_CHECKING imports and docstrings/examples are updated to use
StatusParameters, including illustrative configurations (min_uptime, active_hours_max, effects_per_startup/effects_per_active_hour).- The underlying conversion logic (conversion_factors, COP/eta bounds, etc.) remains unchanged.
This keeps the high-level converter APIs aligned with the new status model while preserving existing behavior for users who don’t pass status parameters.
Also applies to: 37-66, 75-92, 122-123, 141-149, 162-178, 210-211, 234-238, 249-265, 297-300, 321-325, 338-354, 390-391, 418-422, 437-457, 503-506, 529-533, 550-567
flixopt/features.py (1)
16-20: StatusModel implementation correctly generalizes on/off behavior to status semantics.
- The constructor cleanly separates configuration (
parameters), currentstatusvariable, andprevious_statushistory, matching howFlowModelandComponentModelpass data in.- Active-hours tracking via
expression_tracking_variableand the startup/startup_count machinery viaBoundingPatterns.state_transition_boundsand temporal sums are consistent with how the tests intests/test_flow.pyreason about constraints and effects.- Uptime/downtime tracking through
ModelingPrimitives.consecutive_duration_trackingwithstate_variable=self.statusvs.self.inactiveplus_get_previous_uptime/_get_previous_downtimematches the test expectations around consecutive on/off hours and previous history.- The effects logic (
effects_per_active_hour,effects_per_startup) correctly applies status- and startup-based multipliers and ties into the existing effects infrastructure.Overall, this is a solid refactor of the feature model from on/off to status, and it appears to integrate well with the rest of the modeling stack.
Also applies to: 145-174, 175-221, 251-269, 273-306, 307-322
flixopt/elements.py (1)
600-663: All verification checks passed - code changes are well-implemented and thoroughly tested.The review's concerns about regressions in flows with
status_parametersand scalarprevious_flow_rateare already addressed by the existing test suite:
- conftest.py fixtures include CHP units with
previous_flow_rate=10andstatus_parameters, and Boiler fixtures withprevious_flow_rate=0and status parameters- test_flow.py has 10 test cases exercising
status_parametersincluding cases withprevious_flow_ratearrays- Implementation correctly handles scalars via the
np.isscalar()check inprevious_statusproperty (line 786), wrapping scalars into lists before DataArray conversionThe code structure is sound:
absolute_flow_rate_boundscorrectly setslb=0when status is enabled_create_status_modelproperly passesprevious_statustoStatusModel_constraint_flow_ratecleanly implements all four combinations (with/without status/investment)No functional issues or test gaps identified.
CHANGELOG.md
Outdated
| - Parameter `status_parameters` → `status_parameters` in `Flow.__init__()` | ||
| - Parameter `status_parameters` → `status_parameters` in `Component.__init__()` |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify redundant parameter documentation.
These entries show the parameter name remaining unchanged (status_parameters → status_parameters), which contradicts the purpose of the breaking changes section. Either:
- These are not breaking changes and should be removed, or
- The old parameter names should differ (e.g.,
on_off_parameters→status_parameters)
Please clarify the actual parameter name change for Flow.__init__() and Component.__init__().
I'll verify the actual parameter names in the codebase to determine if the changelog entry is accurate.
🏁 Script executed:
#!/bin/bash
# Search for Flow class and its __init__ method
echo "=== Flow class __init__ ==="
rg -nA 10 'class Flow' --type py -A 15 | head -40
echo ""
echo "=== Component class __init__ ==="
# Search for Component class and its __init__ method
rg -nA 10 'class Component' --type py -A 15 | head -40Length of output: 5267
🌐 Web query:
OnOff Status rename breaking changes Python parameter names
💡 Result:
Do you mean a specific project or protocol (e.g., Home Assistant, Zigbee, Matter/CHIP, Bluetooth Mesh, a Python library)? Please specify which codebase so I can search and list the exact renames and breaking changes to parameter names.
🏁 Script executed:
#!/bin/bash
# Search for Flow.__init__ method signature
echo "=== Flow.__init__() method ==="
rg -nA 20 'class Flow\(Element\):' flixopt/elements.py | grep -A 30 'def __init__'
echo ""
echo "=== Component.__init__() method ==="
# Search for Component.__init__ method signature
rg -nA 20 'class Component\(Element\):' flixopt/elements.py | grep -A 30 'def __init__'
echo ""
echo "=== Searching for on_off_parameters or status_parameters in __init__ ==="
rg -n 'status_parameters|on_off_parameters' flixopt/elements.pyLength of output: 2239
🏁 Script executed:
#!/bin/bash
# Search entire codebase for on_off_parameters
echo "=== Searching for on_off_parameters everywhere ==="
rg -n 'on_off_parameters'
echo ""
echo "=== Searching git history for on_off_parameters ==="
git log --all -p --follow -S 'on_off_parameters' -- '*.py' | head -100
echo ""
echo "=== Checking git blame on CHANGELOG.md lines 89-90 ==="
git blame CHANGELOG.md | sed -n '89,90p'Length of output: 6215
Fix parameter rename documentation in CHANGELOG.
The changelog incorrectly documents lines 89-90 as status_parameters → status_parameters. Based on the git history, the actual rename was on_off_parameters → status_parameters.
Update both entries to reflect the correct parameter name change:
- Parameter
on_off_parameters→status_parametersinFlow.__init__() - Parameter
on_off_parameters→status_parametersinComponent.__init__()
🤖 Prompt for AI Agents
In CHANGELOG.md around lines 89 to 90, the entries incorrectly show
"status_parameters → status_parameters"; update both lines to reflect the actual
rename from on_off_parameters to status_parameters by changing each entry text
to "Parameter `on_off_parameters` → `status_parameters` in `Flow.__init__()`"
and "Parameter `on_off_parameters` → `status_parameters` in
`Component.__init__()` respectively, preserving surrounding formatting and
punctuation.
Changes Made:
1. Fixed error message in modeling.py
- Corrected ModelingPrimitives.state_transition_bounds() → BoundingPatterns.state_transition_bounds() in error message (flixopt/modeling.py:591)
2. Fixed Transmission type hint (flixopt/components.py:667)
- Changed status_parameters: StatusParameters = None → status_parameters: StatusParameters | None = None
3. Fixed absolute_losses=0 edge case (flixopt/components.py:768)
- Added np.any(self.element.absolute_losses != 0) check in create_transmission_equation to match the initialization logic
- This prevents AttributeError when absolute_losses is explicitly set to 0
4. Updated test assertion messages (tests/test_component.py)
- Changed "On does not work properly" → "Status does not work properly"
5. Fixed effects_per_startup type (examples/02_Complex/complex_example.py)
- Changed scalar effects_per_startup=0.01 → dict effects_per_startup={Costs.label: 0.01} in all 3 occurrences
- Now consistent with the StatusParameters API which expects a dict mapping effect names to values
6. Updated test_functional.py docstring
- Removed reference to non-existent TestStatus class
- Updated to accurately describe the status-related test functions
7. Consistent unbounded upper bounds (flixopt/features.py:191)
- Changed np.inf → None for unbounded active_hours_max
- Now consistent with FlowModel's total_flow_hours pattern
All changes maintain backward compatibility and align with the codebase's existing patterns. The documentation in index.md was already correct (BoundingPatterns is
the right class for state_transition_bounds).
1. CHANGELOG.md - Fixed parameter rename documentation (lines 89-90) - Changed incorrect status_parameters → status_parameters - To correct: on_off_parameters → status_parameters 2. CHANGELOG.md - Removed duplicate logger warning (line 803 in v2.1.0) - Removed duplicate entry that was already documented in v2.0.1 - Fixed v2.0.1 entry to say on_off_parameters (the name at that time) 3. StatusParameters.md - Aligned flow bounds formulation (line 229) - Updated summary to include max(ε, rel_lower) like the main text - Now consistent: s(t) · P · max(ε, rel_lower) ≤ p(t) ≤ s(t) · P · rel_upper 4. features.py - Narrowed previous_status type hint (line 155) - Changed from Numeric_TPS | None to xr.DataArray | None - Added import xarray as xr (line 12) - This accurately reflects that _get_previous_uptime() and _get_previous_downtime() use xarray APIs All changes are verified to compile correctly and maintain consistency with the codebase patterns!
1. Constraint naming in tests (tests/test_component.py:126-127, 158, 168,
338, 348):
- Updated test expectations from 'TestComponent|on|lb' and
'TestComponent|on|ub' to 'TestComponent|status|lb' and
'TestComponent|status|ub' to match the actual constraint names
2. Added 'off' property to StatusModel (flixopt/features.py:284-287):
- Added a new property that returns 1 - self.status for backward
compatibility with tests expecting an off attribute
3. Fixed deprecated parameter name (tests/test_functional.py:435):
- Changed force_switch_on=True to force_startup_tracking=True in
StatusParameters
4. Fixed property name (tests/test_functional.py:466):
- Changed switch_off to shutdown to match the actual property name in
StatusModel
switch_on -> activate switch_off -> deactivate
switch_on -> activate switch_off -> deactivate state_variable -> state
switch_on -> activate switch_off -> deactivate state_variable -> state
✅ All Parameters Now Documented
Each primitive now has complete parameter documentation with:
- Clear description of what each parameter does
- Type expectations
- Default values where applicable
✅ Focused on Math & Parameters
Removed:
- Excessive examples at low level
- Use case lists that belong at higher levels
Enhanced:
- Mathematical formulations (using proper · symbol for multiplication)
- Clear behavior descriptions
- Precise return value documentation
Updated Functions:
ModelingPrimitives:
1. expression_tracking_variable
- All 6 parameters documented
- Clear math formulation
2. consecutive_duration_tracking
- All 9 parameters documented
- Explained Big-M value
- Clear what constraints are returned
3. mutual_exclusivity_constraint
- All 4 parameters documented
- Simplified, focused on math
BoundingPatterns:
4. basic_bounds
- All 4 parameters documented
- Concise formulation
5. bounds_with_state
- All 5 parameters documented
- Explained epsilon (ε) usage
6. scaled_bounds
- All 5 parameters documented
- Clear scaling relationship
7. scaled_bounds_with_state
- All 7 parameters documented
- Explained Big-M formulation
8. state_transition_bounds
- All 7 parameters documented
- Removed verbose examples, kept math focus
9. continuous_transition_bounds
- All 8 parameters documented
- Clear Big-M constraint explanation
Result
✅ All parameters documented
✅ Math-focused docstrings
✅ Consistent format across all primitives
✅ Tests still passing
The modeling primitives now have professional, complete documentation!
# Conflicts: # docs/user-guide/mathematical-notation/features/OnOffParameters.md # examples/02_Complex/complex_example.py # examples/03_Calculation_types/example_calculation_types.py # examples/04_Scenarios/scenario_example.py # examples/05_Two-stage-optimization/two_stage_optimization.py # flixopt/components.py # flixopt/elements.py # flixopt/features.py # flixopt/flow_system.py # flixopt/interface.py # flixopt/linear_converters.py # tests/conftest.py # tests/test_flow.py # tests/test_functional.py # tests/test_linear_converter.py # tests/test_scenarios.py
# Conflicts: # flixopt/interface.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
tests/test_scenarios.py (1)
220-235: PiecewiseConversion fixture correctly wired to StatusParametersUsing
status_parameters=fx.StatusParameters(effects_per_startup=0.01)on theLinearConverterkeeps the fixture aligned with the new status-based API while leaving the scenario-weight tests focused on weights. If you want consistency with other places (e.g. complex example) you might consider passing a dict keyed by an effect label, but it’s not required by the tests here.tests/test_linear_converter.py (1)
381-497: Piecewise + StatusParameters test is well-structured; consider renaming functionThe updated
test_piecewise_conversion_with_onoff:
- Builds a
LinearConverterwith bothpiecewise_conversionandStatusParameters.- Verifies
Converter|statusexists and is used as a zero point for the piecewise model.- Checks that the single-segment constraint is now bounded by
Converter|status, so segments cannot be active when the converter is inactive.- Re-asserts the
active_hoursaggregation and temporal cost effect with status.Function name (
_with_onoff) is now slightly misleading given it tests StatusParameters; consider renaming to_with_statusfor clarity.tests/test_functional.py (1)
333-363: Basic Flow status test (test_on) is correctly migrated
thermal_flownow usesstatus_parameters=fx.StatusParameters(), and the test:
- Reads
boiler.thermal_flow.submodel.status.status.solution.valuesand expects[0, 1, 1, 0, 1].- Verifies the corresponding flow rates.
This keeps the original semantics but via the new status model. Only minor nit: the error message still mentions
"__on"; you may optionally update it to reference “status”.flixopt/components.py (1)
576-577: Transmission: status-based behavior for absolute losses is wired correctly, with one minor design caveat
- The new
status_parametersargument and doc updates correctly describe that absolute losses imply binary status behavior.- In
TransmissionModel.__init__, defaultingflow.status_parameters = StatusParameters()whenabsolute_lossesis nonzero guaranteesin_flow.submodel.status.statusexists before it is used increate_transmission_equation, so the extra+ status * absolute_lossesterm is safe.- Component-level
status_parametersare also propagated viaComponentModel, so users can still attach richer status constraints at the component level.Minor design caveat (no immediate bug): the auto-created
StatusParameters()on flows are instantiated aftertransform_datahas already run, so they never go throughStatusParameters.transform_data. That’s fine today because all their fields stayNone(no coordinates needed), but if you later introduce nontrivial defaults intoStatusParameters, you’ll want to either:
- create these defaults earlier (before
transform_data), or- explicitly call
flow.status_parameters.transform_data(...)for the auto-created ones.Also applies to: 632-647, 657-660, 674-683, 739-744, 775-777
tests/test_flow.py (1)
976-1032: test_switch_on_constraints: startup tracking and costs reflect the new status semanticsUnder
StatusParameters(startup_limit=5, effects_per_startup={'costs': 100}), the test asserts:
- Presence of
startup,shutdown, andstartup_countvariables.- Constraints for switching (
switch|transition,switch|initial,switch|mutex) plusstartup_counttrackingstartup.sum('time').- Temporal cost constraint proportional to
startup * 100.This cleanly expresses startup counting and startup-cost accrual based on status transitions. The remaining
switch|*short_names are legacy but harmless.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
-
CHANGELOG.md(1 hunks) -
docs/user-guide/core-concepts.md(1 hunks) -
docs/user-guide/mathematical-notation/dimensions.md(1 hunks) -
docs/user-guide/mathematical-notation/effects-penalty-objective.md(6 hunks) -
docs/user-guide/mathematical-notation/elements/Flow.md(3 hunks) -
docs/user-guide/mathematical-notation/elements/Storage.md(2 hunks) -
docs/user-guide/mathematical-notation/features/InvestParameters.md(8 hunks) -
docs/user-guide/mathematical-notation/features/OnOffParameters.md(0 hunks) -
docs/user-guide/mathematical-notation/features/StatusParameters.md(1 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/bounds-and-states.md(9 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/duration-tracking.md(8 hunks) -
docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md(10 hunks) -
examples/02_Complex/complex_example.py(4 hunks) -
examples/03_Calculation_types/example_calculation_types.py(2 hunks) -
examples/04_Scenarios/scenario_example.py(2 hunks) -
examples/05_Two-stage-optimization/two_stage_optimization.py(1 hunks) -
flixopt/components.py(13 hunks) -
flixopt/elements.py(25 hunks) -
flixopt/features.py(3 hunks) -
flixopt/flow_system.py(2 hunks) -
flixopt/interface.py(5 hunks) -
flixopt/linear_converters.py(25 hunks) -
flixopt/modeling.py(17 hunks) -
flixopt/plotting.py(1 hunks) -
mkdocs.yml(1 hunks) -
tests/conftest.py(8 hunks) -
tests/test_component.py(16 hunks) -
tests/test_flow.py(20 hunks) -
tests/test_functional.py(19 hunks) -
tests/test_linear_converter.py(7 hunks) -
tests/test_scenarios.py(3 hunks) -
tests/test_storage.py(2 hunks)
💤 Files with no reviewable changes (1)
- docs/user-guide/mathematical-notation/features/OnOffParameters.md
✅ Files skipped from review due to trivial changes (1)
- docs/user-guide/mathematical-notation/effects-penalty-objective.md
🚧 Files skipped from review as they are similar to previous changes (10)
- docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md
- docs/user-guide/mathematical-notation/elements/Flow.md
- docs/user-guide/mathematical-notation/modeling-patterns/bounds-and-states.md
- mkdocs.yml
- flixopt/flow_system.py
- examples/03_Calculation_types/example_calculation_types.py
- docs/user-guide/mathematical-notation/dimensions.md
- tests/conftest.py
- examples/04_Scenarios/scenario_example.py
- docs/user-guide/mathematical-notation/modeling-patterns/duration-tracking.md
🧰 Additional context used
🪛 LanguageTool
docs/user-guide/mathematical-notation/features/StatusParameters.md
[style] ~126-~126: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...r at least
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
🔇 Additional comments (55)
docs/user-guide/core-concepts.md (1)
31-31: LGTM! Terminology update aligns with StatusParameters rename.The documentation correctly updates the constraint description from "on/off" to "active/inactive status," consistent with the project-wide terminology shift.
CHANGELOG.md (1)
62-124: Comprehensive and well-structured breaking change documentation.The CHANGELOG provides excellent migration guidance with:
- Clear parameter mapping tables
- Complete variable/model class renames
- Internal property changes documented
- Migration instructions
The documentation correctly shows the rename from
on_off_parameters→status_parametersin Flow and Component constructors (lines 100-101), resolving the previous review concern.tests/test_storage.py (1)
409-426: Test updates correctly reflect StatusParameters rename.The test properly updates variable names from
|onto|statusand validates the constraint using the new naming convention. The test logic remains unchanged, ensuring consistent behavior.docs/user-guide/mathematical-notation/elements/Storage.md (1)
56-71: Documentation enhancements improve clarity and completeness.The additions provide:
- Clear cross-reference to the "Bounds with State" modeling pattern (consistent with StatusParameters documentation)
- Documentation of final charge state parameters
- Improved parameter organization
These changes enhance the documentation's usefulness and align with the broader StatusParameters terminology updates.
docs/user-guide/mathematical-notation/features/InvestParameters.md (1)
236-243: Clear parameter rename documentation aids migration.The documentation properly highlights the new parameter names and explicitly notes which deprecated names they replace:
effects_of_investment(wasfix_effects)effects_of_investment_per_size(wasspecific_effects)piecewise_effects_of_investment(waspiecewise_effects)effects_of_retirement(wasdivest_effects)This clarity helps users migrate their code.
examples/05_Two-stage-optimization/two_stage_optimization.py (1)
58-65: Example correctly demonstrates StatusParameters usage.The two-stage optimization example properly migrates to the new API:
on_off_parameters→status_parameterseffects_per_switch_on→effects_per_startupconsecutive_on_hours_min→min_uptimeconsecutive_off_hours_min→min_downtimeThis provides users with a clear reference implementation.
docs/user-guide/mathematical-notation/features/StatusParameters.md (1)
1-312: Excellent comprehensive documentation for StatusParameters.This new documentation provides:
- Clear mathematical formulations for all constraints
- Practical examples covering diverse use cases (power plants, batch processes, HVAC, backup generators)
- Strong cross-references to modeling patterns
- Implementation guidance with parameter mappings
The flow-bounds formulation is now consistent between the main text (line 210) and the summary (line 238), both using
max(ε, rel_lower), resolving the previous review concern.tests/test_scenarios.py (2)
141-171: StatusParameters mapping for Boiler and thermal_flow looks consistentThe mapping from the former on/off semantics to
StatusParameters(effects_per_active_hour,active_hours_min/max,min_uptime/max_uptime,max_downtime,effects_per_startup,startup_limit) is internally consistent and matches the expected rename pattern. No issues from a modeling perspective.
370-379: Clarified single-segment constraint comment is accurateThe updated comment documenting that the single-segment RHS is
1when nostatus_parametersare provided correctly reflects the model expression that follows and the new status-aware behavior elsewhere.tests/test_linear_converter.py (2)
137-193: StatusParameters test for LinearConverter is sound and hits key semanticsThe new
test_linear_converter_with_statuscorrectly checks:
- Existence of
Converter|statusandConverter|active_hoursvariables.- The aggregate
active_hoursconstraint:active_hours == (status * hours_per_step).sum('time').- The temporal cost effect:
Converter->costs(temporal) == status * hours_per_step * 5.This gives good coverage of the core StatusModel behavior on a simple converter.
370-379: Single-segment constraint comment now reflects status-aware behaviorThe clarification that the RHS is
1when nostatus_parametersare used is correct and keeps this test aligned with the more general status-aware behavior checked in the following test.examples/02_Complex/complex_example.py (1)
49-116: Complex example correctly migrated to StatusParametersThe complex example’s boiler and CHP configurations now consistently use
StatusParameters:
Boiler:
- Component-level
effects_per_active_hour={Costs.label: 0, CO2.label: 1000}for pure emission tracking.- Thermal-flow
StatusParameterswithactive_hours_min/max, array-valuedmin_uptime, scalarmax_uptime/max_downtime, andeffects_per_startup/startup_limitkeyed byCosts.label.Both CHP variants:
- Use
status_parameters=fx.StatusParameters(effects_per_startup={Costs.label: 0.01}), aligning startup costs with the cost effect.The mappings from former on/off fields to status fields look correct and keep the example in sync with the new API.
tests/test_component.py (5)
72-177: Component status/active_hours variables and constraints for multiple flows are coherent
test_on_with_multiple_flowsnow expects:
- Per-flow variables:
(... )|statusand(... )|active_hoursalongside flow_rate/total_flow_hours.- Component-level
TestComponent|statusandTestComponent|active_hours.- Flow-rate bounds tied to
Out2’s status.- Aggregator constraints
TestComponent|status|lb/|ubthat bind component status to the average/sum of per-flow statuses.This matches the intended behavior of a shared status flag controlling flow-rate bounds and a component-wide status. The expectations look consistent and should catch regressions in the status model.
178-238: Single-flow component status behavior is correctly constrainedFor the single-input case, the test now:
- Requires only one flow-level
status/active_hoursplus component-levelstatus/active_hours.- Asserts that
TestComponent|statusis binary and equalsTestComponent(In1)|status.- Keeps flow-rate bounds conditioned on the input status.
This is a clean specialization of the multi-flow logic and should behave as expected.
239-357: Previous-state + status tests retain expected behavior with StatusParametersIn
test_previous_states_with_multiple_flows:
- The component uses
status_parameters=fx.StatusParameters()with per-flow status/active_hours and a component-level status.- Flow-rate bounds for
Out2and the component status aggregation constraints are the same structure as in the non-previous-state test, ensuring consistent status semantics regardless of previous flows.The expectations look internally consistent; no issues.
358-417: Uptime initialization constraint test covers important StatusModel edge caseThe parameterized
test_previous_states_with_multiple_flows_parameterizednow:
- Applies
StatusParameters(min_uptime=3)both on the main input flow and on the component.- Verifies that the
TestComponent|uptime|initialconstraint setsuptime(time=0) == status(time=0) * (previous_on_hours + 1)across different previous-flow patterns.This gives good coverage for how previous operating hours feed into the new uptime state, which is crucial for correctness of min_uptime logic.
448-618: Transmission tests correctly updated to use status submodel outputsAcross the three transmission tests, assertions now read from
transmission.in1.submodel.status.status.solution.valuesinstead of legacy on/off fields, while keeping the expected patterns unchanged. This keeps the tests aligned with the new StatusParameters/StatusModel naming without altering their intent around loss modeling or investment coupling.tests/test_functional.py (7)
1-18: Top-level docstring now accurately references status behavior testsThe updated documentation mentioning “operational constraints like status behavior” and listing
test_startup_shutdown/test_consecutive_uptime_downtimereflects the new focus on StatusParameters-based logic. The narrative remains consistent with the rest of the file.
372-413: Inactive-state coverage intest_offmatches new status/inactive splitWith
status_parameters=fx.StatusParameters(max_downtime=100), the test now:
- Checks
status.statusmatches[0, 1, 1, 0, 1].- Confirms
status.inactive == 1 - status.- Leaves flow-rate expectations unchanged.
This gives direct coverage for the new
inactivefield and its relationship tostatus.
423-479: Startup/shutdown tracking test exercises new StatusParameters flag
test_startup_shutdownnow drives:
status_parameters=fx.StatusParameters(force_startup_tracking=True)on the thermal flow.- Assertions for
status.status,status.startup, andstatus.shutdown, with patterns[0,1,1,0,1],[0,1,0,0,1],[0,0,0,1,0].This is a strong regression test for startup/shutdown bookkeeping under the new API.
481-521: Total active-hours max test is consistent withactive_hours_maxsemantics
test_on_total_maxnow constrains the primary boiler viaStatusParameters(active_hours_max=1)and keeps the backup unconstrained. The expected pattern (statusactive only once, backup filling the rest) and resulting total costs match the intended mapping fromon_hours_total_maxtoactive_hours_max.
531-603: Active-hours min/max bounds test is cleanly translated to StatusParametersIn
test_on_total_bounds:
- Primary boiler uses
active_hours_max=2, backup usesactive_hours_min=3.- Load profile is fixed to avoid nondeterminism.
- Assertions on status sums, patterns, and flow rates reflect the new constraint names while preserving the original behavioral expectations.
This gives good coverage of both lower and upper active-hours bounds.
605-663: Consecutive uptime test correctly usesmin_uptime/max_uptime
test_consecutive_uptime_downtimereplaces consecutive-on constraints with:
status_parameters=fx.StatusParameters(max_uptime=2, min_uptime=2)for the main boiler.- A fixed backup boiler.
The expected status pattern
[1, 1, 0, 1, 1]and flow-rate split between main and backup boilers are consistent with “exactly 2 consecutive hours when on” behavior, so the migration from on/off terminology to uptime/downtime looks correct.
666-725: Consecutive downtime test correctly usesmin_downtime/max_downtime
test_consecutive_offnow:
- Applies
StatusParameters(max_downtime=2, min_downtime=2)to the backup boiler with a seeded previous flow rate.- Verifies
status.status[0, 0, 1, 0, 0],status.inactive[1, 1, 0, 1, 1], and the expected small nonzero flow when on.This is a faithful translation of the old consecutive-off semantics to the new downtime parameters.
flixopt/components.py (3)
18-18: LinearConverter: StatusParameters integration is consistent with the new status featureImporting
StatusParameters, exposing astatus_parameterskwarg onLinearConverter.__init__, and passing it through toComponent.__init__wires the component-level status feature correctly. Docs clearly warn about inducing binaries on all connected flows, which matches the behavior implemented inComponentModel. No functional issues spotted here.Also applies to: 51-55, 170-176
810-811: LinearConverterModel: using component status as zero_point for PiecewiseModel is reasonableUsing
zero_point=self.status.status if self.status is not None else Falseties piecewise activation to the component-level status model when present, and cleanly falls back to a constantFalseotherwise. This matches the intent of having a single binary governing piecewise segments for the converter.Also applies to: 816-823
981-982: StorageModel.investment docstring rename is harmlessChanging the property docstring to “Investment feature” aligns with the pattern used for the status feature and doesn’t affect behavior.
flixopt/interface.py (3)
2-3: Module docstring correctly reflects Investment + Status responsibilitiesUpdating the module description to cover both investment and status parameter classes is accurate given
InvestParametersandStatusParametersnow live here.
415-417: PiecewiseConversion doc “on/inactive” wording aligns with status terminologyThe updated description of “fixed operating points (e.g., on/inactive, discrete speeds)” is consistent with the new active/inactive status language used elsewhere. No behavioral impact.
1009-1067: StatusParameters: constructor, transforms, and helper flags look coherent
- The new fields (
effects_per_startup,effects_per_active_hour,active_hours_min/max,min_/max_uptime,min_/max_downtime,startup_limit,force_startup_tracking) cover startup, active-hours, uptime/downtime, and cycling controls in a clean, unit-commitment-style API.transform_datauses_fit_effect_coordswith suffixesper_startupandper_active_hourand_fit_coordswith appropriate dims (time vs period/scenario), which matches how tests intests/test_flow.pyexpect these arrays to behave.- Helper properties:
use_uptime_trackinganduse_downtime_trackingcorrectly flip on when respective min/max fields are set.use_startup_trackinghonoringforce_startup_trackingor any non-emptyeffects_per_startup/startup_limitmatches the intent to avoid unnecessary binaries while still tracking starts when needed.use_inactivedelegating to downtime tracking is a reasonable compatibility proxy.Overall, the StatusParameters API looks internally consistent and matches its documented semantics.
Also applies to: 1190-1237, 1239-1265
tests/test_flow.py (8)
521-582: test_flow_on: baseline status integration and bounds look correctThe test now:
- Enables status behavior via
status_parameters=fx.StatusParameters().- Expects variables
statusandactive_hours, plus corresponding constraints linkingflow_ratebounds tostatusandactive_hoursto the time-sum ofstatus.This matches the FlowModel logic (
with_statusbranch using a binary status and tracking active hours), so the expectations are aligned.
585-642: test_effects_per_active_hour: temporal effects now correctly driven by statusUsing
StatusParameters(effects_per_active_hour=...)and asserting that:
- status and active_hours variables/constraints exist, and
- temporal effects are
status * hours_per_step * effect_rateis consistent with the idea that these effects accrue while the unit is active, independent of actual flow rate. The dims checks against
model.get_coords()are also appropriate.
648-725: test_consecutive_on_hours: uptime constraints align with status-based designThe test now:
- Requires
min_uptime=2,max_uptime=8viaStatusParameters.- Checks existence of
uptimeandstatusvariables and a full set of uptime constraints (ub,forward,backward,initial,lb), with formulas matching cumulative-time and big‑M style logic.This is consistent with modeling consecutive active hours and appears to match the intended semantics.
727-805: test_consecutive_on_hours_previous: previous uptime handling is still coherentWith
previous_flow_rateproviding prior activity andmin_uptime/max_uptimeset, the test verifies:
- Same uptime constraint set as above.
- Adjusted
megaincluding prior uptime.- Initial uptime and lower-bound constraints incorporating previous status.
Given
FlowModel.previous_statusconvertsprevious_flow_rateinto a binary time-series, these expectations look consistent with how prior uptime should be folded in.
808-887: test_consecutive_off_hours: downtime variables and constraints are consistentHere
StatusParameters(min_downtime=4, max_downtime=12)drives:
- Creation of
downtimeandinactivevariables.- A full constraint set (
ub,forward,backward,initial,lb) mirroring the uptime structure but for inactivity.The formulas (cumulative downtime, big‑M with
inactive, min/max durations) match the intended minimum/maximum consecutive inactive-hours semantics.
890-970: test_consecutive_off_hours_previous: previous inactivity is handled analogously to uptimeWith previous inactivity encoded via
previous_flow_rate, the test correctly:
- Adjusts the big‑M term (
mega) to account for earlier downtime.- Checks that all downtime constraints now include the extended horizon.
This mirrors the previous-uptime tests and aligns with the new inactive/downtime naming.
1035-1068: test_on_hours_limits: active_hours min/max limits are wired as expectedUsing
StatusParameters(active_hours_min=20, active_hours_max=100), the test checks:
statusandactive_hoursvariables exist.active_hoursbounds match the configured min/max.- The constraint links
active_hoursto the time-sum ofstatus.This matches the definition of total active-hours limits.
1074-1175: TestFlowOnInvestModel: combined investment + status behavior is validated thoroughlyFor both optional and mandatory investment cases, the tests now:
- Require
status_parameters=fx.StatusParameters().- Expect
statusandactive_hoursvariables alongside investment variables.- Check:
- Size/investment bounds,
- Flow-rate lower/upper bounds conditioned on both size and status (
lb1/ub1,lb2/ub2),- Active-hours tracking via status.
These assertions are consistent with the
FlowModel._constraint_flow_ratebranches that usescaled_bounds_with_statewhen both investment and status are present.flixopt/elements.py (5)
17-19: Imports of StatusModel and StatusParameters are appropriateBringing
StatusModelfrom.featuresandStatusParametersfrom.interfaceintoelements.pyis necessary since bothFlowModelandComponentModelnow construct and expose status submodels. No issues here.
65-84: Component: component-level StatusParameters lifecycle is well-integrated
- Adding
status_parameters: StatusParameters | NonetoComponent.__init__and storing it enables component-scoped status constraints._set_flow_systemandtransform_datacorrectly propagateflow_systemand callstatus_parameters._set_flow_system/.transform_datawhen present.- Docstrings clearly state that component-level status induces binaries on all connected flows and recommend using Flow-level
StatusParameterswhen possible to reduce binary counts.This matches how
ComponentModellater useselement.status_parametersto create a component-levelStatusModel.Also applies to: 93-106, 118-130
321-333: Flow: status_parameters handling and new plausibility warnings are sensible
Flow.__init__now accepts and storesstatus_parameters, and_set_flow_system/transform_datacorrectly forward calls when present.- The new plausibility checks:
- Warn when both
fixed_relative_profileandstatus_parametersare set, since that makes the “fixed” profile effectively switchable.- Warn when
relative_minimum > 0butstatus_parameters is None, because this forbids ever reaching zero flow and suggests status-based modeling instead.These warnings help users avoid surprising behavior without changing existing semantics. The additional
status_parameters.transform_data(prefix)call is also correctly placed alongside other flow-level transforms.Also applies to: 452-487, 510-517, 543-548, 564-575
672-736: FlowModel: status creation, constraint branching, and previous_status look correct
_create_status_model:
- Creates a binary
statusvariable and attaches aStatusModelsubmodel with the flow’s ownstatus_parametersandprevious_status.
_constraint_flow_rate:
- No-invest/no-status: relies solely on variable bounds (as before).
- Status only: creates a status model and applies
BoundingPatterns.bounds_with_state, so flow can be zero when inactive even ifrelative_minimum > 0.- Investment only: unchanged scaled bounds.
- Investment + status: creates both investment and status models and uses
scaled_bounds_with_stategated by status; this matches the tests intests/test_flow.py.
with_statussimply checkselement.status_parameters is not None, which works both for user-specified and auto-assigned parameters.
absolute_flow_rate_boundsnow keepslb=0wheneverwith_statusis true, so the binary can genuinely shut the flow off; lower bounds are still enforced via the bounding patterns when status=1.The
statusandprevious_statusproperties are thin, clear accessors and match howComponentModeland higher-level features use them.Overall, the status-related branching in
FlowModelis consistent and avoids over-constraining flows when status is active.Also applies to: 741-743, 815-828, 831-836, 850-863
918-937: ComponentModel: component-level status aggregation and previous_status composition are well-structured
__init__initializesself.statusand later, in_do_modeling, only creates a component-levelStatusModelwhenelement.status_parametersis provided.Before that, it:
- Auto-attaches empty
StatusParameters()to flows when the component hasstatus_parametersor whenprevent_simultaneous_flowsis used, ensuring every such flow has a status binary to work with while keeping component-level constraints governed by the component’s own parameters.The component status variable:
- Equals the single flow’s status for one-flow components.
- For multi-flow components, is constrained via
status <= sum(flow_statuses)+epsandstatus >= sum(flow_statuses)/(len(flow_statuses)+eps), effectively implementing an OR over flow statuses while keeping binary semantics intact.
previous_statusaggregates each flow’sstatus._previous_status:
- Filters out missing entries,
- Pads varying lengths using negative time indices, and
- ORs across flows to get a single component-level previous-status series.
This design cleanly separates per-flow gating from component-level operational constraints, while still allowing prior status information to drive component-level StatusParameters.
Also applies to: 943-968, 985-1002
flixopt/linear_converters.py (1)
17-21: Converters: StatusParameters refactor is clean and non-intrusiveAcross
Boiler,Power2Heat,HeatPump,CoolingTower,CHP, andHeatPumpWithSource:
- Each class now accepts an optional
status_parameters: StatusParameters | Noneand forwards it toLinearConverter.__init__, so converter-specific wrappers participate in the same status feature as the base component.- The use of
from __future__ import annotationsplus TYPE_CHECKING-only import ofStatusParametersavoids runtime import cycles while keeping type hints accurate.- All docstrings and examples have been updated to the new names (
effects_per_startup,effects_per_active_hour,min_uptime, etc.) without altering the underlying conversion factor logic.No functional regressions are apparent; the changes are purely in parameter naming and feature wiring.
Also applies to: 76-99, 171-199, 266-290, 363-389, 465-495, 596-621
flixopt/features.py (4)
152-158: LGTM! Type hint correctly updated.The
previous_statustype hint has been narrowed fromNumeric_TPS | Nonetoxr.DataArray | None, addressing the past review comment. This accurately reflects that the helper methods_get_previous_uptime()and_get_previous_downtime()use xarray APIs that require a DataArray.
226-237: LGTM! Correct use ofhours_per_stepproperty.The use of
self.hours_per_step(lines 234, 247) is correct. As noted in past reviews, thehours_per_stepproperty exists on theSubmodelbase class, so this is the appropriate way to access it.
254-273: LGTM! Effects methods correctly refactored.The effects methods have been properly renamed (
effects_per_active_hour,effects_per_startup) and the implementation correctly uses the renamed variables (self.status,self.startup) with appropriate scaling by time steps.
277-326: LGTM! Properties and helper methods consistently renamed.All properties and helper methods have been consistently refactored to use the new active/inactive/uptime/downtime/startup/shutdown terminology. The use of
.get()for optional properties safely returnsNonewhen variables don't exist.flixopt/modeling.py (7)
50-174: LGTM! Documentation consistently updated.All comments and docstrings in the utility classes have been correctly updated to use "active/inactive" terminology instead of "on/off". No logic changes, maintaining backward compatibility.
196-239: LGTM! Improved type hints and documentation.The addition of explicit type hints for
tracked_expression(line 199) and the enhanced docstring with mathematical formulation improve code clarity and maintainability.
351-392: LGTM! Clearer documentation for mutual exclusivity.The enhanced docstring clearly explains the mathematical formulation and behavior. The terminology update ("active (=1)") is consistent with the broader refactoring.
571-623: Parameter renames verified—no issues found.The call site in
flixopt/features.py:207-215correctly uses all renamed parameters (state,activate,deactivate,previous_state). No calls use the old parameter names. The enhanced docstring accurately reflects the implementation.
430-473: Parameter rename verified and correctly propagated.All call sites in
flixopt/elements.py(line 707) andflixopt/features.py(line 77) are using the newstateparameter name in their keyword arguments. No issues found.
241-349: All call sites verified and properly updated.The parameter rename from
state_variabletostatehas been successfully applied throughout the codebase. Both call sites inflixopt/features.py(lines 228-235 and 241-248) already use the new parameter namestate=, confirming the breaking change has been properly handled. The enhanced docstring with mathematical formulations is also clear and accurate.
625-692: Parameter renames verified—no call sites found in codebase.Verification confirms the parameter renames (
switch_on→activate,switch_off→deactivate) pose no breaking changes. The function has no call sites in the codebase, making the rename safe. The enhanced docstring with Big-M explanation and behavior description is well-documented.
# Conflicts: # docs/user-guide/mathematical-notation/effects-penalty-objective.md
…r=inf when active_hours_max is not specified
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
Breaking Changes
OnOffParameterstoStatusParameterswith updated field names:effects_per_switch_on→effects_per_startup, on-hours → active-hours, consecutive durations → uptime/downtimestatus_parametersinstead ofon_off_parametersNew Features
minimal_final_charge_stateandmaximal_final_charge_stateparameters to Storageeffects_of_investment,effects_of_investment_per_size,piecewise_effects_of_investment,effects_of_retirementDocumentation
✏️ Tip: You can customize this high-level summary in your review settings.