-
Notifications
You must be signed in to change notification settings - Fork 9
Feature/rename on off #476
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
Conversation
…bles (on→active, off→inactive, switch_on→startup, switch_off→shutdown) 2. interface.py - Renamed OnOffParameters → ActiveInactiveParameters, all parameters and properties updated 3. elements.py - Updated Component, Flow, FlowModel, and ComponentModel classes with new names 4. components.py - Updated LinearConverter, Transmission, and all model classes 5. modeling.py - No changes needed (no references found)
ActiveInactiveModel -> ActivityModel
WalkthroughThis PR systematically renames the on/off feature API across the codebase to activity-based terminology. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_flow.py (1)
975-1035: Update variable naming in ActivityModel to use switch|on/off/count pattern instead of startup/shutdown/startup_countThe test expectations are correct, but the implementation has a critical naming mismatch. The test expects variables named
switch|on,switch|off, andswitch|count, but the current code inflixopt/features.pycreates variables with namesstartup,shutdown, andstartup_count.Specific issue:
- Line 197-198: Creates variables with
short_name='startup'andshort_name='shutdown'→ should be renamed to createswitch|onandswitch|off- Line 215: Creates variable with
short_name='startup_count'→ should be renamed to createswitch|countWhen
add_variables(short_name='X')is called, it generates a variable name as{label_of_model}|X. So the current code producesSink(Wärme)|startupinstead of the expectedSink(Wärme)|switch|on. This causes the test assertion at line 993-995 to fail immediately.The constraints are correctly named (using
name='{label}|switch'passed tostate_transition_bounds()), so only the variable naming needs correction.
🧹 Nitpick comments (4)
tests/test_linear_converter.py (2)
137-148: LinearConverter ActivityParameters test is sound; core model needs to emitConverter|onSwitching to
fx.ActivityParameterswithactive_hours_total_min/maxand per‑running‑hour effects, and asserting bothConverter|onandConverter|active_hours_total(plus the aggregate constraint) matches the intended ActivityModel behavior.The CI failure about
Converter|onbeing absent indicates the LinearConverter implementation likely isn’t yet creating the ActivityModel whenactivity_parametersis passed (e.g., still checking an obsoleteon_off_parametersattribute). Once that wiring is fixed, this test should be valid. As a small clean‑up, consider renaming the localon_off_paramsvariable toactivity_paramsto avoid confusion.Also applies to: 151-157, 169-178
381-401: Piecewise+ActivityParameters test correctly combines ActivityModel and piecewise behaviorThis test now:
- Uses
fx.ActivityParameterswithactive_hours_total_*and per‑running‑hour costs.- Passes
activity_parameters=on_off_paramsinto the converter.- Verifies
Converter|onas the zero‑point for the piecewise model and checks theConverter|active_hours_totalconstraint and cost effect.These expectations are coherent with the ActivityModel design and with the basic ActivityParameters test above. The same CI failure about missing
Converter|onagain points to the LinearConverter model not yet instantiating the ActivityModel foractivity_parameters, not an issue in this test code.Also applies to: 404-410, 431-433, 483-488
CHANGELOG.md (1)
767-770: Optional: Fix markdown list indentation.The static analyzer reports indentation issues. While not functionally critical, consider adjusting the indentation for consistency with project style guidelines.
Apply this diff to fix the indentation:
- - Variable renaming: `...|consecutive_active_hours` → `...|ConsecutiveOn|hours` - - Variable renaming: `...|consecutive_inactive_hours` → `...|ConsecutiveOff|hours` - - Constraint renaming: `...|consecutive_active_hours_con1` → `...|ConsecutiveOn|con1` - - Similar pattern for all consecutive on/off constraints + - Variable renaming: `...|consecutive_active_hours` → `...|ConsecutiveOn|hours` + - Variable renaming: `...|consecutive_inactive_hours` → `...|ConsecutiveOff|hours` + - Constraint renaming: `...|consecutive_active_hours_con1` → `...|ConsecutiveOn|con1` + - Similar pattern for all consecutive on/off constraintsBased on static analysis hints.
flixopt/interface.py (1)
1-4: Module docstring still refers to “OnOff decisions”.The module‑level description still mentions “OnOff decisions” while the public interface has been renamed to
ActivityParameters. Consider updating this docstring in a follow‑up so user‑facing docs are consistent with the new terminology.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
CHANGELOG.md(6 hunks)docs/user-guide/mathematical-notation/elements/Flow.md(3 hunks)docs/user-guide/mathematical-notation/features/OnOffParameters.md(5 hunks)docs/user-guide/mathematical-notation/index.md(3 hunks)docs/user-guide/mathematical-notation/modeling-patterns/bounds-and-states.md(2 hunks)docs/user-guide/mathematical-notation/modeling-patterns/duration-tracking.md(1 hunks)docs/user-guide/mathematical-notation/modeling-patterns/index.md(1 hunks)docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md(1 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/__init__.py(2 hunks)flixopt/components.py(13 hunks)flixopt/elements.py(21 hunks)flixopt/features.py(3 hunks)flixopt/interface.py(7 hunks)flixopt/linear_converters.py(20 hunks)tests/conftest.py(8 hunks)tests/test_component.py(14 hunks)tests/test_flow.py(19 hunks)tests/test_functional.py(16 hunks)tests/test_linear_converter.py(7 hunks)tests/test_scenarios.py(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Python Package CI/CD
tests/test_component.py
[error] 1-1: TestComponentModel failures indicate missing 'on' variable for components with multiple and single flows (expected TestComponent|on).
tests/test_functional.py
[error] 1-1: TypeError: ActivityParameters.init() got an unexpected keyword argument 'force_switch_on' used in tests; API does not accept this parameter.
tests/test_linear_converter.py
[error] 1-1: LinearConverter on/off variable not present. Expected 'Converter|on' (and related on/off flow) in model variables for on/off behavior.
tests/test_flow.py
[error] 1-1: Flow on/off switches and consecutive active hours constraints not surfaced: missing 'Sink(Wärme)|on' and 'Sink(Wärme)|consecutive_active_hours' variables for various scenarios.
[error] 1-1: Test switch_on_off tests fail due to missing binary switch variables (e.g., 'Sink(Wärme)|switch|on', 'Sink(Wärme)|switch|off', 'Sink(Wärme)|switch|count') when startups are constrained.
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
767-767: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
768-768: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
769-769: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
770-770: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
816-816: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
817-817: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
818-818: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (37)
examples/05_Two-stage-optimization/two_stage_optimization.py (1)
58-58: LGTM! Consistent API rename.The transition from
on_off_parameterstoactivity_parametersis correctly applied, with proper parameter name updates (effects_per_switch_on→effects_per_startup,consecutive_on_hours_min→consecutive_active_hours_min,consecutive_off_hours_min→consecutive_inactive_hours_min).Also applies to: 65-67
examples/04_Scenarios/scenario_example.py (1)
124-124: LGTM! Consistent parameter rename.Both flow configurations correctly use
activity_parameters=fx.ActivityParameters()instead of the oldon_off_parameters.Also applies to: 135-137
docs/user-guide/mathematical-notation/modeling-patterns/duration-tracking.md (1)
157-157: LGTM! Documentation link updated correctly.The "Used In" section now correctly references
ActivityParameters.mdinstead of the oldOnOffParameters.md.docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md (1)
224-224: LGTM! Documentation reference updated.Correctly updates the link to reference
ActivityParametersin the "Used In" section.docs/user-guide/mathematical-notation/modeling-patterns/index.md (1)
46-46: LGTM! Usage reference updated.The "Usage in Components" section now correctly links to
ActivityParameters.md.docs/user-guide/mathematical-notation/modeling-patterns/bounds-and-states.md (1)
48-48: LGTM! Documentation references updated.Both "Used in" sections now correctly reference
ActivityParameters.md, maintaining proper cross-documentation links.Also applies to: 111-111
docs/user-guide/mathematical-notation/elements/Flow.md (1)
26-27: LGTM! Comprehensive documentation update.The Flow documentation has been thoroughly updated to reference
ActivityParametersin all relevant sections: introduction, mathematical patterns, key parameters, and cross-references.Also applies to: 37-37, 51-51, 59-59
docs/user-guide/mathematical-notation/index.md (2)
59-62: Docs correctly referenceActivityParametersandactivity_parametersThe feature description and “User API” text now point to
ActivityParametersand theactivity_parameterskeyword, which matches the new public API and terminology.
100-101: Cross‑reference tables are consistent with ActivityParameters renameThe features and class lookup tables correctly list
ActivityParametersin both the documentation and Python‑class columns; link targets and dotted paths look consistent with the new interface.Also applies to: 122-123
flixopt/__init__.py (1)
31-31: Public API now exposesActivityParametersinstead ofOnOffParametersImport and
__all__have been updated to re‑exportActivityParameters, aligning the top‑level API with the new naming. Please just confirm that droppingOnOffParametersfrom the public surface is an intentional breaking change (i.e., you don’t want a temporary alias for backward compatibility).Also applies to: 51-51
tests/test_component.py (5)
85-107: Component tests correctly target ActivityParameters behavior; core model likely missing|onvarThese tests now pass
activity_parameters=fx.ActivityParameters()intoComponentand assert the presence of...|onand...|active_hours_totalvariables/constraints at both flow and component level. That’s consistent with the intended ActivityModel semantics.Given the pipeline failure complaining about missing
TestComponent|on, the issue is almost certainly in the component modeling code (not these tests) — e.g., it may still be keyed off an obsoleteon_off_parametersattribute or not instantiating the ActivityModel for components. Once the core model wiring is updated, these expectations look correct.Also applies to: 111-129
183-210: Single‑flow component ActivityParameters test is consistent with multi‑flow testThe single‑flow variant mirrors the multi‑flow expectations: component and flow‑level
|onand|active_hours_totalvariables/constraints are asserted and linked in the same way. This is coherent and should pass once the underlying Component model creates the ActivityModel foractivity_parameters.
260-305: Previous‑states test updates align with active‑hours terminologyUsing
activity_parameters=fx.ActivityParameters(consecutive_active_hours_min=3)and checking for...|active_hours_totalconstraints and theTestComponent|active_hours_totalaggregate fits the rename from “on_hours” to “active_hours”. Constraint names look self‑consistent across flows and component.Again, CI failures about missing
|onvariables point to missing ActivityModel wiring in the component implementation rather than an issue in these expectations.
381-409: Initial consecutive active hours assertion matches renamed counter semanticsThe new check on
TestComponent|consecutive_active_hours|initialtyingconsecutive_active_hoursattime=0toon * (previous_on_hours + 1)attime=0is consistent with how a consecutive‑active‑hours tracker should initialize with prior activity. Naming and indexing look correct.
438-441: Transmission tests correctly useactive_inactive.activeafter renameAccessing
transmission.in1.submodel.active_inactive.active.solution.valuesto assert the on/off pattern matches the ActivityModel rename (from something likeon_off.on). The expected activity patterns in the balanced/unbalanced tests remain unchanged and still validate that the transmission element is active only when it should be.Also applies to: 499-502, 577-580
tests/test_scenarios.py (1)
143-170: Scenario fixtures use ActivityParameters with the new field names consistentlyThe boiler and KWK setup now attach
fx.ActivityParametersusing the renamed fields (active_hours_total_*,consecutive_active_hours_*,consecutive_inactive_hours_max,effects_per_startup,startup_total_max) on both the converter and the key thermal flow. This keeps the scenario tests aligned with the new API while preserving the original behavioral intent.Also applies to: 231-232
CHANGELOG.md (1)
139-139: LGTM! Documentation reference updated correctly.The changelog correctly documents the improvement to ActivityParameters attribute documentation, aligning with the rename from OnOffParameters.
docs/user-guide/mathematical-notation/features/OnOffParameters.md (2)
1-1: LGTM! Documentation title updated correctly.The document title and all references have been properly updated from OnOffParameters to ActivityParameters.
238-246: LGTM! Parameter documentation is accurate.All parameter names in the documentation correctly reflect the new API:
effects_per_startup(waseffects_per_switch_on)active_hours_total_min/max(wason_hours_total_min/max)consecutive_active_hours_min/max(wasconsecutive_on_hours_min/max)consecutive_inactive_hours_min/max(wasconsecutive_off_hours_min/max)startup_total_max(wasswitch_on_total_max)flixopt/elements.py (3)
17-18: LGTM! Import statements updated correctly.The imports now reference the renamed ActivityModel and ActivityParameters classes.
59-99: LGTM! Component constructor updated with clear documentation.The
activity_parametersparameter is properly documented with clear terminology:
- "binary operation constraints and costs"
- "discrete active/inactive states"
- Creates binary variables for all connected Flows
The note correctly explains the performance tradeoffs.
600-612: LGTM! Method renamed and implementation updated.The method rename from
_create_on_off_modelto_create_active_inactive_modelis correct, and the implementation properly creates the ActivityModel with theactive_variableparameter.tests/conftest.py (1)
167-175: LGTM! Test fixture parameters updated correctly.The complex boiler test fixture correctly uses the new ActivityParameters API with all renamed fields:
active_hours_total_min/maxconsecutive_active_hours_min/maxconsecutive_inactive_hours_maxeffects_per_startupstartup_total_maxexamples/02_Complex/complex_example.py (1)
72-80: LGTM! Example demonstrates correct ActivityParameters usage.The example properly demonstrates the new API with all renamed parameters. This provides good reference material for users migrating to the new API.
tests/test_functional.py (1)
357-369: LGTM! Test assertions updated to active_inactive terminology.The test correctly accesses
boiler.Q_th.submodel.active_inactive.activeand validates the binary state variable values.flixopt/linear_converters.py (2)
18-18: LGTM! Import statement updated correctly.The TYPE_CHECKING import now references ActivityParameters instead of OnOffParameters.
75-91: LGTM! Boiler class updated correctly.The Boiler constructor signature and implementation properly use
activity_parameters: ActivityParameters | None = Noneand pass it to the parent LinearConverter.flixopt/features.py (3)
145-172: LGTM! ActivityModel class properly renamed and documented.The class rename from OnOffModel to ActivityModel is complete with:
- Updated constructor signature using
ActivityParametersandactive_variable- Clear docstring explaining active/inactive state modeling
- Proper initialization of the
activeattribute
196-217: LGTM! Startup/shutdown tracking correctly implements state transitions.The switch tracking has been properly updated:
switch_on→startupswitch_off→shutdownswitch_on_nr→startup_count- Uses
parameters.use_startupcheck- Applies
BoundingPatterns.state_transition_boundswith renamed variables
272-304: LGTM! Property accessors correctly renamed.All property accessors match the new API naming:
active_hours_totalinactivestartupshutdownstartup_nrconsecutive_active_hoursconsecutive_inactive_hoursflixopt/components.py (1)
18-21: ActivityParameters wiring in components looks consistent; just verify FlowModel integration.The replacements of
OnOffParameterswithActivityParametersinLinearConverter,Transmission,TransmissionModel, andLinearConverterModelare internally consistent: constructor signatures, super calls, and the use ofactive_inactive.activefor piecewise zero points and absolute-loss activation all line up. Behavior should match the previous on/off logic as long as the Flow/Activity submodels correctly populateactive_inactive. Please double‑check that the underlying FlowModel/ActivityModel has been fully updated to driveactive_inactivefor these components so that absolute losses and piecewise activity behave as expected.Also applies to: 165-176, 658-677, 732-771, 800-815
tests/test_flow.py (5)
521-585: Basic ActivityParameters usage intest_flow_onlooks correct; ensure binary creation logic matches.Switching from
on_off_parameterstoactivity_parameters=fx.ActivityParameters()and fromon_hours_totaltoactive_hours_totalin this test aligns with the new API and withflow.submodel.active_inactive.active. The expectations for variable/constraint sets and theactive_hours_totalaggregation are consistent.Given the CI complaints about missing
Sink(Wärme)|onin some scenarios, please confirm that providing a bareActivityParameters()still triggers creation of the active/inactive binary in the FlowModel (as the test assumes).
587-646: Running‑hour effects test is aligned with ActivityParameters.Using
activity_parameters=fx.ActivityParameters(effects_per_running_hour=...)and then pulling the processed arrays back fromflow.activity_parameters.effects_per_running_hourto form the temporal effect constraints is consistent with the new design. The expectations on variable/constraint sets and the dimensionality checks look appropriate.No changes needed here; just ensure
fit_effects_to_model_coordsstill returns effect arrays with the same coordinate structure asmodel.get_coords(), as assumed in the assertions.
1041-1071: Total active hours limits tests match the newactive_hours_total_*fields.The
test_on_hours_limitstest has been correctly adapted to:
- Pass
ActivityParameters(active_hours_total_min=20, active_hours_total_max=100).- Expect the
Sink(Wärme)|active_hours_totalvariable with [20, 100] bounds.- Check the aggregation constraint
active_hours_total == (on * hours_per_step).sum('time').This is consistent with the
active_hours_total_min/maxfields and their transform inActivityParameters. As with other tests, ensure the ActivityModel actually uses these fields when deciding to create theactive_hours_totalvariable and constraint; otherwise this test will continue to fail even though the expectations are correct.
1077-1266: Flow‑with‑investment tests using ActivityParameters look fine; rely on upstream active_hours/active‑binary wiring.The investment‑related “on” tests now:
- Attach
activity_parameters=fx.ActivityParameters()to flows with InvestParameters.- Expect the
Sink(Wärme)|onbinary,Sink(Wärme)|active_hours_totalvariable, and the usual total‑hours aggregation constraint.- Combine the existing investment constraints with the activity‑based lower/upper bounds on flow_rate.
Structurally these expectations are consistent with the intended ActivityParameters behavior and the rest of the test suite. The remaining work is to ensure that the Flow/Activity model creates the active/inactive binary and
active_hours_totalin all invested‑flow scenarios when ActivityParameters are present, so these tests don’t see missing variables/constraints at runtime.
810-973: Implementation is already complete; review comment is outdated.The
consecutive_inactive_hoursfeature is fully wired and operational:
ActivityParameters.use_consecutive_inactive_hoursproperty exists and correctly identifies when min/max parameters are setuse_inactive(interface.py:1315) properly proxies touse_consecutive_inactive_hours- Features module (line 177) gates the creation of the inactive binary variable on
use_inactive- Features module (line 233) calls
ModelingPrimitives.consecutive_duration_tracking()with the inactive state variable and generates all expected constraints (|ub,|forward,|backward,|initial,|lb)- Constraint naming matches test expectations (
Sink(Wärme)|consecutive_inactive_hours|{constraint_type})The tests should pass as-is since all required variables and constraints are being created with the correct names and logic.
Likely an incorrect or invalid review comment.
flixopt/interface.py (1)
1080-1345: ActivityParameters is fully integrated and working correctly—no changes needed.The new
ActivityParametersclass and its helper properties (use_inactive,use_consecutive_active_hours,use_consecutive_inactive_hours,use_startup) are properly wired into the ActivityModel (flixopt/features.py:177, 196, 220, 233). All expected variables (on,off,consecutive_active_hours,consecutive_inactive_hours,switch) are created and tested. The old OnOffParameters API has been completely removed. No integration issues remain.
| activity_parameters=fx.ActivityParameters(effects_per_startup=1000), | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
LGTM! Consistent API usage.
Both component definitions correctly use activity_parameters=fx.ActivityParameters(effects_per_startup=...) with the new naming convention.
Since this is a draft PR with a systematic API rename, please verify that all references to the old naming have been updated across the entire codebase:
This will help ensure the rename is complete and no references to the old API remain.
Also applies to: 91-91
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to the old naming convention
echo "=== Searching for old parameter names ==="
rg -n "on_off_parameters" --type py
rg -n "OnOffParameters" --type py --type md
echo ""
echo "=== Searching for old field names ==="
rg -n "effects_per_switch_on" --type py
rg -n "consecutive_on_hours" --type py
rg -n "consecutive_off_hours" --type py
rg -n "on_hours_total" --type py
rg -n "switch_on_total" --type py
echo ""
echo "=== Checking documentation ==="
rg -n "OnOffParameters\.md" --type mdLength of output: 1448
Incomplete API migration detected across codebase.
While the changes in example_calculation_types.py correctly use the new activity_parameters naming, the verification reveals remaining old API references that need updating:
flixopt/linear_converters.pycontains multiple instances of old field names:effects_per_switch_on(lines 63, 148, 420, 531)consecutive_on_hours_min(lines 62, 147, 235, 322, 419, 530)on_hours_total_max(line 421)
flixopt/flow_system.py:74has outdated example documentation usingon_off_parametersflixopt/flow_system.py:53references old field name in comment
These locations require updating to complete the systematic rename and ensure consistency across the codebase.
🤖 Prompt for AI Agents
In examples/03_Calculation_types/example_calculation_types.py (lines 82-83) and
across the codebase, update remaining old API field names to the new
activity_parameters API: in flixopt/linear_converters.py at lines
62,63,147,148,235,322,419,420,421,530,531 replace effects_per_switch_on ->
effects_per_startup, consecutive_on_hours_min -> consecutive_startup_hours_min,
and on_hours_total_max -> startup_hours_total_max (use the exact new field names
used elsewhere, e.g., effects_per_startup), and in flixopt/flow_system.py update
the example/doc and comment at lines 53 and 74 to use activity_parameters
instead of on_off_parameters and update any inline field-name mentions to the
new names; ensure imports/types align with the renamed API and run tests to
catch any remaining references.
| def test_consecutive_active_hours(self, basic_flow_system_linopy_coords, coords_config): | ||
| """Test flow with minimum and maximum consecutive on hours.""" | ||
| flow_system, coords_config = basic_flow_system_linopy_coords, coords_config | ||
|
|
||
| flow = fx.Flow( | ||
| 'Wärme', | ||
| bus='Fernwärme', | ||
| size=100, | ||
| on_off_parameters=fx.OnOffParameters( | ||
| consecutive_on_hours_min=2, # Must run for at least 2 hours when turned on | ||
| consecutive_on_hours_max=8, # Can't run more than 8 consecutive hours | ||
| activity_parameters=fx.ActivityParameters( | ||
| consecutive_active_hours_min=2, # Must run for at least 2 hours when turned on | ||
| consecutive_active_hours_max=8, # Can't run more than 8 consecutive hours | ||
| ), | ||
| ) | ||
|
|
||
| flow_system.add_elements(fx.Sink('Sink', inputs=[flow])) | ||
| model = create_linopy_model(flow_system) | ||
|
|
||
| assert {'Sink(Wärme)|consecutive_on_hours', 'Sink(Wärme)|on'}.issubset(set(flow.submodel.variables)) | ||
| assert {'Sink(Wärme)|consecutive_active_hours', 'Sink(Wärme)|on'}.issubset(set(flow.submodel.variables)) | ||
|
|
||
| assert_sets_equal( | ||
| { | ||
| 'Sink(Wärme)|consecutive_on_hours|ub', | ||
| 'Sink(Wärme)|consecutive_on_hours|forward', | ||
| 'Sink(Wärme)|consecutive_on_hours|backward', | ||
| 'Sink(Wärme)|consecutive_on_hours|initial', | ||
| 'Sink(Wärme)|consecutive_on_hours|lb', | ||
| 'Sink(Wärme)|consecutive_active_hours|ub', | ||
| 'Sink(Wärme)|consecutive_active_hours|forward', | ||
| 'Sink(Wärme)|consecutive_active_hours|backward', | ||
| 'Sink(Wärme)|consecutive_active_hours|initial', | ||
| 'Sink(Wärme)|consecutive_active_hours|lb', | ||
| } | ||
| & set(flow.submodel.constraints), | ||
| { | ||
| 'Sink(Wärme)|consecutive_on_hours|ub', | ||
| 'Sink(Wärme)|consecutive_on_hours|forward', | ||
| 'Sink(Wärme)|consecutive_on_hours|backward', | ||
| 'Sink(Wärme)|consecutive_on_hours|initial', | ||
| 'Sink(Wärme)|consecutive_on_hours|lb', | ||
| 'Sink(Wärme)|consecutive_active_hours|ub', | ||
| 'Sink(Wärme)|consecutive_active_hours|forward', | ||
| 'Sink(Wärme)|consecutive_active_hours|backward', | ||
| 'Sink(Wärme)|consecutive_active_hours|initial', | ||
| 'Sink(Wärme)|consecutive_active_hours|lb', | ||
| }, | ||
| msg='Missing consecutive on hours constraints', | ||
| ) | ||
|
|
||
| assert_var_equal( | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'], | ||
| model.add_variables(lower=0, upper=8, coords=model.get_coords()), | ||
| ) | ||
|
|
||
| mega = model.hours_per_step.sum('time') | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|ub'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'] <= model.variables['Sink(Wärme)|on'] * mega, | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|ub'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'] <= model.variables['Sink(Wärme)|on'] * mega, | ||
| ) | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|forward'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(1, None)) | ||
| <= model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(None, -1)) | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|forward'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(1, None)) | ||
| <= model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(None, -1)) | ||
| + model.hours_per_step.isel(time=slice(None, -1)), | ||
| ) | ||
|
|
||
| # eq: duration(t) >= duration(t - 1) + dt(t) + (On(t) - 1) * BIG | ||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|backward'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(1, None)) | ||
| >= model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(None, -1)) | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|backward'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(1, None)) | ||
| >= model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(None, -1)) | ||
| + model.hours_per_step.isel(time=slice(None, -1)) | ||
| + (model.variables['Sink(Wärme)|on'].isel(time=slice(1, None)) - 1) * mega, | ||
| ) | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|initial'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=0) | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|initial'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=0) | ||
| == model.variables['Sink(Wärme)|on'].isel(time=0) * model.hours_per_step.isel(time=0), | ||
| ) | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|lb'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'] | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|lb'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'] | ||
| >= ( | ||
| model.variables['Sink(Wärme)|on'].isel(time=slice(None, -1)) | ||
| - model.variables['Sink(Wärme)|on'].isel(time=slice(1, None)) | ||
| ) | ||
| * 2, | ||
| ) | ||
|
|
||
| def test_consecutive_on_hours_previous(self, basic_flow_system_linopy_coords, coords_config): | ||
| def test_consecutive_active_hours_previous(self, basic_flow_system_linopy_coords, coords_config): | ||
| """Test flow with minimum and maximum consecutive on hours.""" | ||
| flow_system, coords_config = basic_flow_system_linopy_coords, coords_config | ||
|
|
||
| flow = fx.Flow( | ||
| 'Wärme', | ||
| bus='Fernwärme', | ||
| size=100, | ||
| on_off_parameters=fx.OnOffParameters( | ||
| consecutive_on_hours_min=2, # Must run for at least 2 hours when turned on | ||
| consecutive_on_hours_max=8, # Can't run more than 8 consecutive hours | ||
| activity_parameters=fx.ActivityParameters( | ||
| consecutive_active_hours_min=2, # Must run for at least 2 hours when turned on | ||
| consecutive_active_hours_max=8, # Can't run more than 8 consecutive hours | ||
| ), | ||
| previous_flow_rate=np.array([10, 20, 30, 0, 20, 20, 30]), # Previously on for 3 steps | ||
| ) | ||
|
|
||
| flow_system.add_elements(fx.Sink('Sink', inputs=[flow])) | ||
| model = create_linopy_model(flow_system) | ||
|
|
||
| assert {'Sink(Wärme)|consecutive_on_hours', 'Sink(Wärme)|on'}.issubset(set(flow.submodel.variables)) | ||
| assert {'Sink(Wärme)|consecutive_active_hours', 'Sink(Wärme)|on'}.issubset(set(flow.submodel.variables)) | ||
|
|
||
| assert_sets_equal( | ||
| { | ||
| 'Sink(Wärme)|consecutive_on_hours|lb', | ||
| 'Sink(Wärme)|consecutive_on_hours|forward', | ||
| 'Sink(Wärme)|consecutive_on_hours|backward', | ||
| 'Sink(Wärme)|consecutive_on_hours|initial', | ||
| 'Sink(Wärme)|consecutive_active_hours|lb', | ||
| 'Sink(Wärme)|consecutive_active_hours|forward', | ||
| 'Sink(Wärme)|consecutive_active_hours|backward', | ||
| 'Sink(Wärme)|consecutive_active_hours|initial', | ||
| } | ||
| & set(flow.submodel.constraints), | ||
| { | ||
| 'Sink(Wärme)|consecutive_on_hours|lb', | ||
| 'Sink(Wärme)|consecutive_on_hours|forward', | ||
| 'Sink(Wärme)|consecutive_on_hours|backward', | ||
| 'Sink(Wärme)|consecutive_on_hours|initial', | ||
| 'Sink(Wärme)|consecutive_active_hours|lb', | ||
| 'Sink(Wärme)|consecutive_active_hours|forward', | ||
| 'Sink(Wärme)|consecutive_active_hours|backward', | ||
| 'Sink(Wärme)|consecutive_active_hours|initial', | ||
| }, | ||
| msg='Missing consecutive on hours constraints for previous states', | ||
| ) | ||
|
|
||
| assert_var_equal( | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'], | ||
| model.add_variables(lower=0, upper=8, coords=model.get_coords()), | ||
| ) | ||
|
|
||
| mega = model.hours_per_step.sum('time') + model.hours_per_step.isel(time=0) * 3 | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|ub'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'] <= model.variables['Sink(Wärme)|on'] * mega, | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|ub'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'] <= model.variables['Sink(Wärme)|on'] * mega, | ||
| ) | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|forward'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(1, None)) | ||
| <= model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(None, -1)) | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|forward'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(1, None)) | ||
| <= model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(None, -1)) | ||
| + model.hours_per_step.isel(time=slice(None, -1)), | ||
| ) | ||
|
|
||
| # eq: duration(t) >= duration(t - 1) + dt(t) + (On(t) - 1) * BIG | ||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|backward'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(1, None)) | ||
| >= model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=slice(None, -1)) | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|backward'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(1, None)) | ||
| >= model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=slice(None, -1)) | ||
| + model.hours_per_step.isel(time=slice(None, -1)) | ||
| + (model.variables['Sink(Wärme)|on'].isel(time=slice(1, None)) - 1) * mega, | ||
| ) | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|initial'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'].isel(time=0) | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|initial'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'].isel(time=0) | ||
| == model.variables['Sink(Wärme)|on'].isel(time=0) * (model.hours_per_step.isel(time=0) * (1 + 3)), | ||
| ) | ||
|
|
||
| assert_conequal( | ||
| model.constraints['Sink(Wärme)|consecutive_on_hours|lb'], | ||
| model.variables['Sink(Wärme)|consecutive_on_hours'] | ||
| model.constraints['Sink(Wärme)|consecutive_active_hours|lb'], | ||
| model.variables['Sink(Wärme)|consecutive_active_hours'] | ||
| >= ( | ||
| model.variables['Sink(Wärme)|on'].isel(time=slice(None, -1)) | ||
| - model.variables['Sink(Wärme)|on'].isel(time=slice(1, None)) | ||
| ) | ||
| * 2, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Consecutive active hours tests expose missing modeling of consecutive_active_hours.
These tests correctly express the new semantics using ActivityParameters(consecutive_active_hours_min/max=...) and expect:
- A
Sink(Wärme)|consecutive_active_hoursvariable alongsideSink(Wärme)|on. - Forward/backward/initial/lb/ub constraints with the renamed
consecutive_active_hourslabels. - Handling of previous activity via
previous_flow_rate.
Given the pipeline error mentioning missing Sink(Wärme)|consecutive_active_hours and Sink(Wärme)|on, it looks like the Activity/Flow model hasn’t yet been updated to:
- Trigger creation of the consecutive‑active tracking variable based on
consecutive_active_hours_min/maxfromActivityParameters. - Ensure the on/off (active) binary exists whenever consecutive‑active constraints are requested.
Please wire the ActivityModel/FlowModel to use ActivityParameters.use_consecutive_active_hours (and the new fields) when constructing these variables and constraints so these tests can pass.
I'll verify the review comment by examining the codebase structure and implementation details.
🏁 Script executed:
#!/bin/bash
# Find ActivityParameters class definition
fd -t f -e py | xargs grep -l "class ActivityParameters"Length of output: 20
🏁 Script executed:
#!/bin/bash
# Search for consecutive_active_hours field usage
rg -n "consecutive_active_hours" --type py -A 2 -B 2 | head -100Length of output: 6662
🏁 Script executed:
#!/bin/bash
# Find Flow class to understand its structure
fd -t f -e py "flow.py" | head -20Length of output: 18
🏁 Script executed:
#!/bin/bash
# Check for existing constraints related to consecutive active hours
rg -n "consecutive_active_hours.*constraint" --type py -C 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for Activity model implementation
fd -t f -e py | xargs grep -l "class.*Activity.*Model"Length of output: 19
🏁 Script executed:
#!/bin/bash
# Search for on variable creation to understand pattern
rg -n "consecutive_active_hours.*on.*variable" --type py -C 3Length of output: 864
🏁 Script executed:
#!/bin/bash
# Find test file to confirm location and content
fd -t f "test_flow.py"Length of output: 18
🏁 Script executed:
#!/bin/bash
# Check what variables/constraints exist in Flow.submodel
rg -n "flow\.submodel\.(variables|constraints)" --type py -C 2 | head -80Length of output: 4735
🏁 Script executed:
#!/bin/bash
# Look for use_consecutive_active_hours reference
rg -n "use_consecutive_active_hours" --type pyLength of output: 158
🏁 Script executed:
#!/bin/bash
# Check git diff for recent changes to flow model
git diff HEAD~1 --stat 2>/dev/null || echo "Git not available"Length of output: 1075
🏁 Script executed:
#!/bin/bash
# Search for lines around 647-808 in test_flow.py to verify the code exists
if fd test_flow.py > /dev/null; then
FILE=$(fd test_flow.py)
wc -l "$FILE"
sed -n '640,660p' "$FILE"
fiLength of output: 896
🏁 Script executed:
#!/bin/bash
# Look for Sink class and how it handles inputs
rg -n "class Sink" --type py -A 10 | head -50Length of output: 1372
🏁 Script executed:
#!/bin/bash
# Check if consecutive_active_hours constraint logic exists anywhere
rg -n "forward.*backward.*consecutive\|consecutive.*forward.*backward" --type py -C 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for ActivityParameters definition to check available fields
rg -n "class ActivityParameters" --type py -A 30 | head -80Length of output: 2692
🏁 Script executed:
#!/bin/bash
# Look for where on variable is created
rg -n "\|on" --type py -C 2 | head -100Length of output: 6894
🏁 Script executed:
#!/bin/bash
# Check for any tests that are currently passing with consecutive_active_hours
rg -n "test_consecutive_active" --type py -A 5 | head -50Length of output: 875
🏁 Script executed:
#!/bin/bash
# Search for linopy model constraint creation patterns
rg -n "add_variables.*lower.*upper" --type py -C 1 | head -60Length of output: 3891
🏁 Script executed:
#!/bin/bash
# Look for where submodel variables/constraints are added
rg -n "submodel\.(variables|constraints)" --type py -C 3 | head -100Length of output: 5646
🏁 Script executed:
#!/bin/bash
# Find Flow implementation in detail
rg -n "class Flow" --type py -A 50 | head -150Length of output: 10216
🏁 Script executed:
#!/bin/bash
# Search for ActivityParameters fields definition
rg -n "consecutive_active_hours_(min|max)" --type py -B 5 -A 5 | head -150Length of output: 10532
🏁 Script executed:
#!/bin/bash
# Check if constraints dict lookup is used in tests
rg -n "model.constraints\[.*consecutive_active_hours" --type py -C 2 | head -80Length of output: 4350
🏁 Script executed:
#!/bin/bash
# Look for create_linopy_model function to understand test setup
rg -n "def create_linopy_model" --type py -A 20 | head -80Length of output: 1179
🏁 Script executed:
#!/bin/bash
# Find where Flow/Activity creates constraints
rg -n "def.*constraint\|add.*constraint" --type py -C 3 -i | head -120Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the actual pytest test execution or conftest setup
fd -t f "conftest.py"Length of output: 17
🏁 Script executed:
#!/bin/bash
# Search for Flow model method that creates consecutive hours variables
rg -n "consecutive_active_hours" --type py --context 10 | head -200Length of output: 13486
🏁 Script executed:
#!/bin/bash
# Look for ActivityModel class directly
rg -n "^class ActivityModel" --type py -A 40 | head -150Length of output: 2840
🏁 Script executed:
#!/bin/bash
# Check constraints that should exist according to test
rg -n "consecutive_active_hours\|ub\|consecutive_active_hours\|lb\|consecutive_active_hours\|forward" --type pyLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for the actual constraint definition or pattern
rg -n "def.*_add.*constraint|def.*_create.*constraint" --type py -C 2 -iLength of output: 705
🏁 Script executed:
#!/bin/bash
# Final check: look for all references to consecutive in codebase
rg -n "consecutive" --type py | head -200Length of output: 20693
🏁 Script executed:
#!/bin/bash
# Check if there's a feature flag or attribute for consecutive active hours
rg -n "use_consecutive\|enable_consecutive" --type py -iLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the exact test assertions to understand what's being verified
if fd test_flow.py > /dev/null; then
FILE=$(fd test_flow.py)
sed -n '647,680p' "$FILE"
fiLength of output: 1578
🏁 Script executed:
#!/bin/bash
# Check if ActivityParameters class has the fields used in test
rg -n "class ActivityParameters" --type py -A 100 | head -200Length of output: 8592
🏁 Script executed:
#!/bin/bash
# Verify what imports are used in test_flow.py
if fd test_flow.py > /dev/null; then
FILE=$(fd test_flow.py)
head -50 "$FILE"
fiLength of output: 1942
🏁 Script executed:
#!/bin/bash
# Look at where Flow submodel is populated with variables and constraints
rg -n "\.submodel\.variables\[|\.submodel\.constraints\[" --type py -C 4 | head -150Length of output: 9238
🏁 Script executed:
#!/bin/bash
# Find where add_variables or add_constraints methods are called
rg -n "add_variables|add_constraints" --type py -C 2 | head -150Length of output: 8946
🏁 Script executed:
#!/bin/bash
# Look for any constraint factory or builder for consecutive active hours
rg -n "consecutive.*constraint|constraint.*consecutive" --type py -i -C 3 | head -100Length of output: 6534
🏁 Script executed:
#!/bin/bash
# Search for ModelingPrimitives class and consecutive_duration_tracking
rg -n "class ModelingPrimitives|def consecutive_duration_tracking" --type py -A 15 | head -200Length of output: 1986
🏁 Script executed:
#!/bin/bash
# Check full features.py around the consecutive_duration_tracking call
rg -n "consecutive_duration_tracking" --type py -B 5 -A 10 | head -150Length of output: 4263
🏁 Script executed:
#!/bin/bash
# Look for where FlowModel creates ActivityModel
rg -n "_create_active_inactive_model" --type py -C 10 | head -150Length of output: 3997
🏁 Script executed:
#!/bin/bash
# Check if Flow has activity_parameters attribute and usage
rg -n "self.element.activity_parameters" --type py -C 5 | head -100Length of output: 4351
🏁 Script executed:
#!/bin/bash
# Search for where on variable is created in ActivityModel
rg -n "short_name='on'\|'on'" --type py -C 3 | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find the complete _do_modeling method in ActivityModel
fd -t f -e py -path "*/features.py" | xargs cat | sed -n '145,350p'Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check what happens in Flow._create_active_inactive_model
rg -n "def _create_active_inactive_model" --type py -A 30Length of output: 1922
🏁 Script executed:
#!/bin/bash
# Look for when Flow triggers ActivityModel creation
rg -n "activity_parameters" --type py -C 3 | grep -A 3 -B 3 "FlowModel\|_create_active_inactive_model" | head -100Length of output: 377
🏁 Script executed:
#!/bin/bash
# Search for all uses of consecutive_active_hours variable creation
rg -n "consecutive_active_hours.*add_variables\|add_variables.*consecutive_active_hours" --type py -C 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check what ModelingPrimitives provides
fd -t f -e py | xargs grep -l "class ModelingPrimitives"Length of output: 19
🏁 Script executed:
#!/bin/bash
# Look at the full consecutive_duration_tracking implementation if it exists
rg -n "def consecutive_duration_tracking" --type py -A 50 | head -200Length of output: 3381
🏁 Script executed:
#!/bin/bash
# Search for where 'on' variable should be created
rg -n "short_name.*=.*['\"]on['\"]" --type pyLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check ActivityModel for any property or alias that renames active to on
rg -n "\.on\b|property.*on" --type py -C 3 | head -150Length of output: 748
🏁 Script executed:
#!/bin/bash
# Look at test_component.py test to see what it checks
sed -n '370,410p' tests/test_component.pyLength of output: 1550
🏁 Script executed:
#!/bin/bash
# Check if there's an 'on' alias in Submodel or base classes
rg -n "def on\|self\.on\s*=" --type py -C 3 | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the actual ActivityModel._do_modeling to see full implementation
rg -n "class ActivityModel" --type py -A 100 | head -250Length of output: 7205
🏁 Script executed:
#!/bin/bash
# Check if FlowModel has a property that provides 'on' from 'active'
rg -n "active_inactive\.on\|active_inactive\.active" --type py -C 2 | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at what the test references in flow.submodel.variables
rg -n "flow.submodel.*on\|flow.submodel.variables\['.*on" --type py -C 2 | head -80Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check modeling.py for where consecutive_duration_tracking returns variables
rg -n "consecutive_duration_tracking" --type py -A 100 | sed -n '1,150p'Length of output: 9624
Fix variable naming in Flow/Component active state variable from 'active' to 'on'.
The consecutive_active_hours feature is implemented but tests fail due to a naming mismatch. The binary state variable is created with short_name='active' while tests expect short_name='on'.
Required changes in flixopt/elements.py:
- Line 601 (FlowModel._create_active_inactive_model): Change
short_name='active'toshort_name='on' - Line 865 (ComponentModel._do_modeling): Change
short_name='active'toshort_name='on'
The ActivityModel correctly implements consecutive_active_hours tracking via ModelingPrimitives.consecutive_duration_tracking, which creates the required variable and all five constraints (ub, forward, backward, initial, lb) as expected by tests.
🤖 Prompt for AI Agents
In flixopt/elements.py around line 601 and line 865, the binary state variable
for Flow/Component is created with short_name='active' but the tests and other
modeling code expect short_name='on'; update both occurrences
(FlowModel._create_active_inactive_model at ~601 and ComponentModel._do_modeling
at ~865) to use short_name='on' so the created variable names match the tests
and the consecutive_active_hours constraints.
| bus='Fernwärme', | ||
| size=100, | ||
| on_off_parameters=fx.OnOffParameters(force_switch_on=True), | ||
| activity_parameters=fx.ActivityParameters(force_switch_on=True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Critical: Verify force_switch_on parameter exists.
This test uses force_switch_on=True, which matches the pipeline failure error: "ActivityParameters.init() got an unexpected keyword argument 'force_switch_on'".
The parameter appears in the documentation (line 244 of OnOffParameters.md) but may not be implemented in the actual ActivityParameters class. Verify that this parameter still exists in the interface or update the test accordingly.
Run the following script to check if force_switch_on exists in ActivityParameters:
🏁 Script executed:
#!/bin/bash
# Check if force_switch_on is defined in ActivityParameters
rg -n "force_switch_on" --type=py flixopt/interface.pyLength of output: 0
The web search returned general Python class documentation, not specific information about the ActivityParameters class in this codebase. I need to search the codebase directly.
🏁 Script executed:
#!/bin/bash
# Find ActivityParameters class definition
fd ActivityParameters --type=py -x grep -l "class ActivityParameters" {} \;Length of output: 169
🏁 Script executed:
#!/bin/bash
# Search for ActivityParameters class definition more broadly
find . -name "*.py" -type f -exec grep -l "class ActivityParameters" {} \;Length of output: 22
🏁 Script executed:
#!/bin/bash
# Extract ActivityParameters class definition with __init__ method
rg -A 20 "class ActivityParameters" flixopt/interface.pyLength of output: 1262
🏁 Script executed:
#!/bin/bash
# Extract ActivityParameters class __init__ method
rg -A 50 "class ActivityParameters" flixopt/interface.py | grep -A 40 "def __init__"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get a larger context of ActivityParameters class to find __init__
rg -A 100 "class ActivityParameters" flixopt/interface.py | head -80Length of output: 5046
🏁 Script executed:
#!/bin/bash
# Find the __init__ method signature in ActivityParameters
rg -n "def __init__" flixopt/interface.py | head -5Length of output: 294
🏁 Script executed:
#!/bin/bash
# Get the full __init__ signature starting at line 871
sed -n '871,920p' flixopt/interface.pyLength of output: 2661
🏁 Script executed:
#!/bin/bash
# Find the line number where ActivityParameters class is defined
rg -n "^class ActivityParameters" flixopt/interface.pyLength of output: 41
🏁 Script executed:
#!/bin/bash
# Get ActivityParameters __init__ method starting from line 1080
sed -n '1080,1300p' flixopt/interface.py | grep -A 50 "def __init__"Length of output: 2334
Change force_switch_on=True to force_startup=True at line 435.
The ActivityParameters class defines the parameter as force_startup (line 1080 of flixopt/interface.py), but the test uses the incorrect name force_switch_on. This causes the pipeline error: "ActivityParameters.init() got an unexpected keyword argument 'force_switch_on'".
🤖 Prompt for AI Agents
In tests/test_functional.py at line 435, the test passes an incorrect keyword
argument name to ActivityParameters: it uses force_switch_on but the class
defines force_startup (see flixopt/interface.py line 1080). Replace
force_switch_on=True with force_startup=True on that line so the constructor
receives the correct parameter name.
|
CLosed in favor of #477 |
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
Documentation
Refactor
OnOffParametersreplaced withActivityParameters; related parameters renamed to reflect active/inactive operational states.