Skip to content

Conversation

@FBumann
Copy link
Member

@FBumann FBumann commented Nov 17, 2025

Description

Brief description of the changes in this PR.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Code refactoring

Related Issues

Closes #(issue number)

Testing

  • I have tested my changes
  • Existing tests still pass

Checklist

  • My code follows the project style
  • I have updated documentation if needed
  • I have added tests for new functionality (if applicable)

Summary by CodeRabbit

Documentation

  • Updated Guides and Examples: All user documentation, guides, and example code updated to reflect the new terminology and naming conventions.

Refactor

  • API Terminology Update: Renamed on/off nomenclature to activity/inactive terminology across all component APIs. OnOffParameters replaced with ActivityParameters; related parameters renamed to reflect active/inactive operational states.

…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
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

Walkthrough

This PR systematically renames the on/off feature API across the codebase to activity-based terminology. OnOffParameters becomes ActivityParameters, OnOffModel becomes ActivityModel, and related terminology shifts from "switch_on," "on_hours," "consecutive_on_hours" to "startup," "active_hours," "consecutive_active_hours" respectively. Changes propagate through core interfaces, components, documentation, examples, and tests.

Changes

Cohort / File(s) Summary
Core API/Interface Changes
flixopt/interface.py, flixopt/features.py
Class rename OnOffParametersActivityParameters and OnOffModelActivityModel; parameter renames: effects_per_switch_oneffects_per_startup, on_hours_total_min/maxactive_hours_total_min/max, consecutive_on_hours_min/maxconsecutive_active_hours_min/max, consecutive_off_hours_min/maxconsecutive_inactive_hours_min/max, switch_on_total_maxstartup_total_max, force_switch_onforce_startup; property renames: on_hours_totalactive_hours_total, switch_onstartup, switch_offshutdown, consecutive_on_hoursconsecutive_active_hours, consecutive_off_hoursconsecutive_inactive_hours.
Package Exports
flixopt/__init__.py
Updated public API exports from OnOffParameters to ActivityParameters.
Element & Component Integration
flixopt/elements.py, flixopt/components.py
Updated constructor signatures and all references from on_off_parameters to activity_parameters; replaced on_off state tracking with active_inactive; updated property and method names to reflect active/inactive semantics.
Specialized Converters
flixopt/linear_converters.py
Updated TYPE_CHECKING imports and constructor signatures across Boiler, Power2Heat, HeatPump, CoolingTower, CHP, and HeatPumpWithSource classes from on_off_parameters: OnOffParameters to activity_parameters: ActivityParameters.
Documentation
docs/user-guide/mathematical-notation/elements/Flow.md, docs/user-guide/mathematical-notation/features/OnOffParameters.md, docs/user-guide/mathematical-notation/index.md, docs/user-guide/mathematical-notation/modeling-patterns/*
Updated all references from OnOffParameters to ActivityParameters, revised parameter documentation to reflect new naming (e.g., effects_per_startup, active_hours_total_min/max), and updated cross-references and "Used In" links throughout modeling patterns documentation.
CHANGELOG
CHANGELOG.md
Updated public API and variable naming references to reflect the renaming from on/off terminology to active/inactive terminology.
Examples
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
Updated Boiler and CHP component instantiations from on_off_parameters=fx.OnOffParameters(...) to activity_parameters=fx.ActivityParameters(...); updated nested parameter names (e.g., on_hours_total_minactive_hours_total_min, effects_per_switch_oneffects_per_startup).
Tests
tests/conftest.py, tests/test_component.py, tests/test_flow.py, tests/test_functional.py, tests/test_linear_converter.py, tests/test_scenarios.py
Updated all component/flow instantiations to use activity_parameters instead of on_off_parameters; renamed variable and constraint expectations (e.g., on_hours_totalactive_hours_total, consecutive_on_hoursconsecutive_active_hours); updated submodel access paths from .on_off.* to .active_inactive.*; updated test function names and assertions to match new terminology.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Areas requiring extra attention:

  • flixopt/features.py and flixopt/interface.py: Verify consistency in internal variable naming, constraint generation logic, and property accessor implementations for the new active/inactive semantics.
  • flixopt/components.py: Ensure piecewise conversion zero-point references (active_inactive.active) and binary constraint wiring are correct throughout LinearConverterModel and TransmissionModel.
  • Test files: Confirm all variable/constraint name mappings in model assertions match the renamed public API (e.g., variable names in model.variables['...'] and constraint references).
  • Documentation cross-references: Verify all internal links and "Used In" sections point to the correct renamed feature documentation pages.
  • Public API consistency: Ensure no mixed usage of old and new parameter names exists across codebase.

Possibly related PRs

  • Feature/v3/update #352: Modifies transform_data signatures and OnOffParameters transform logic; overlaps with renaming and transform_data updates to ActivityParameters.
  • Feature/types direct #465: Modifies OnOffParameters/OnOffModel constructor signatures and type hints in interface.py and features.py; related through shared class-level changes.
  • V3.0.0/main #284: Refactors OnOffParameters/OnOffModel integration into FlowSystemModel/Submodel architecture; overlaps with renamed class and API usage across interface and features modules.

Poem

🐰 From "on" and "off" we hop away,
To "active" days and "inactive" play,
With "startup" bounds and "shutdown" cheer,
Our naming's fresh throughout the year!
No logic changed, just words made bright—
The rabbit's refactor shines so right!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is a template placeholder with no actual content describing the changes, objectives, or rationale. Critical sections like 'Description', 'Type of Change', and 'Related Issues' are either empty or contain only placeholder text. Fill in the description with: (1) clear explanation of renaming OnOff to Activity terminology; (2) select 'Code refactoring' as the change type; (3) specify related issue number; (4) confirm testing status; (5) briefly explain breaking API changes and migration path.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.28% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Feature/rename on off' is vague and does not clearly convey the comprehensive scope of changes, which involve a systematic API rename from OnOff/on-off terminology to Activity/active-inactive terminology across multiple modules and public interfaces. Consider a more descriptive title such as 'Rename OnOff API to Activity with active/inactive semantics' or 'Refactor: OnOff→Activity terminology across public API' to better communicate the significant API changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/rename-on-off

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@FBumann
Copy link
Member Author

FBumann commented Nov 17, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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_count

The test expectations are correct, but the implementation has a critical naming mismatch. The test expects variables named switch|on, switch|off, and switch|count, but the current code in flixopt/features.py creates variables with names startup, shutdown, and startup_count.

Specific issue:

  • Line 197-198: Creates variables with short_name='startup' and short_name='shutdown' → should be renamed to create switch|on and switch|off
  • Line 215: Creates variable with short_name='startup_count' → should be renamed to create switch|count

When add_variables(short_name='X') is called, it generates a variable name as {label_of_model}|X. So the current code produces Sink(Wärme)|startup instead of the expected Sink(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 to state_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 emit Converter|on

Switching to fx.ActivityParameters with active_hours_total_min/max and per‑running‑hour effects, and asserting both Converter|on and Converter|active_hours_total (plus the aggregate constraint) matches the intended ActivityModel behavior.

The CI failure about Converter|on being absent indicates the LinearConverter implementation likely isn’t yet creating the ActivityModel when activity_parameters is passed (e.g., still checking an obsolete on_off_parameters attribute). Once that wiring is fixed, this test should be valid. As a small clean‑up, consider renaming the local on_off_params variable to activity_params to avoid confusion.

Also applies to: 151-157, 169-178


381-401: Piecewise+ActivityParameters test correctly combines ActivityModel and piecewise behavior

This test now:

  • Uses fx.ActivityParameters with active_hours_total_* and per‑running‑hour costs.
  • Passes activity_parameters=on_off_params into the converter.
  • Verifies Converter|on as the zero‑point for the piecewise model and checks the Converter|active_hours_total constraint 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|on again points to the LinearConverter model not yet instantiating the ActivityModel for activity_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 constraints

Based 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2033d52 and 22b925c.

📒 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_parameters to activity_parameters is correctly applied, with proper parameter name updates (effects_per_switch_oneffects_per_startup, consecutive_on_hours_minconsecutive_active_hours_min, consecutive_off_hours_minconsecutive_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 old on_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.md instead of the old OnOffParameters.md.

docs/user-guide/mathematical-notation/modeling-patterns/state-transitions.md (1)

224-224: LGTM! Documentation reference updated.

Correctly updates the link to reference ActivityParameters in 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 ActivityParameters in 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 reference ActivityParameters and activity_parameters

The feature description and “User API” text now point to ActivityParameters and the activity_parameters keyword, which matches the new public API and terminology.


100-101: Cross‑reference tables are consistent with ActivityParameters rename

The features and class lookup tables correctly list ActivityParameters in 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 exposes ActivityParameters instead of OnOffParameters

Import and __all__ have been updated to re‑export ActivityParameters, aligning the top‑level API with the new naming. Please just confirm that dropping OnOffParameters from 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 |on var

These tests now pass activity_parameters=fx.ActivityParameters() into Component and assert the presence of ...|on and ...|active_hours_total variables/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 obsolete on_off_parameters attribute 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 test

The single‑flow variant mirrors the multi‑flow expectations: component and flow‑level |on and |active_hours_total variables/constraints are asserted and linked in the same way. This is coherent and should pass once the underlying Component model creates the ActivityModel for activity_parameters.


260-305: Previous‑states test updates align with active‑hours terminology

Using activity_parameters=fx.ActivityParameters(consecutive_active_hours_min=3) and checking for ...|active_hours_total constraints and the TestComponent|active_hours_total aggregate fits the rename from “on_hours” to “active_hours”. Constraint names look self‑consistent across flows and component.

Again, CI failures about missing |on variables 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 semantics

The new check on TestComponent|consecutive_active_hours|initial tying consecutive_active_hours at time=0 to on * (previous_on_hours + 1) at time=0 is consistent with how a consecutive‑active‑hours tracker should initialize with prior activity. Naming and indexing look correct.


438-441: Transmission tests correctly use active_inactive.active after rename

Accessing transmission.in1.submodel.active_inactive.active.solution.values to assert the on/off pattern matches the ActivityModel rename (from something like on_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 consistently

The boiler and KWK setup now attach fx.ActivityParameters using 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 (was effects_per_switch_on)
  • active_hours_total_min/max (was on_hours_total_min/max)
  • consecutive_active_hours_min/max (was consecutive_on_hours_min/max)
  • consecutive_inactive_hours_min/max (was consecutive_off_hours_min/max)
  • startup_total_max (was switch_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_parameters parameter 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_model to _create_active_inactive_model is correct, and the implementation properly creates the ActivityModel with the active_variable parameter.

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/max
  • consecutive_active_hours_min/max
  • consecutive_inactive_hours_max
  • effects_per_startup
  • startup_total_max
examples/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.active and 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 = None and 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 ActivityParameters and active_variable
  • Clear docstring explaining active/inactive state modeling
  • Proper initialization of the active attribute

196-217: LGTM! Startup/shutdown tracking correctly implements state transitions.

The switch tracking has been properly updated:

  • switch_onstartup
  • switch_offshutdown
  • switch_on_nrstartup_count
  • Uses parameters.use_startup check
  • Applies BoundingPatterns.state_transition_bounds with renamed variables

272-304: LGTM! Property accessors correctly renamed.

All property accessors match the new API naming:

  • active_hours_total
  • inactive
  • startup
  • shutdown
  • startup_nr
  • consecutive_active_hours
  • consecutive_inactive_hours
flixopt/components.py (1)

18-21: ActivityParameters wiring in components looks consistent; just verify FlowModel integration.

The replacements of OnOffParameters with ActivityParameters in LinearConverter, Transmission, TransmissionModel, and LinearConverterModel are internally consistent: constructor signatures, super calls, and the use of active_inactive.active for 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 populate active_inactive. Please double‑check that the underlying FlowModel/ActivityModel has been fully updated to drive active_inactive for 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 in test_flow_on looks correct; ensure binary creation logic matches.

Switching from on_off_parameters to activity_parameters=fx.ActivityParameters() and from on_hours_total to active_hours_total in this test aligns with the new API and with flow.submodel.active_inactive.active. The expectations for variable/constraint sets and the active_hours_total aggregation are consistent.

Given the CI complaints about missing Sink(Wärme)|on in some scenarios, please confirm that providing a bare ActivityParameters() 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 from flow.activity_parameters.effects_per_running_hour to 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_coords still returns effect arrays with the same coordinate structure as model.get_coords(), as assumed in the assertions.


1041-1071: Total active hours limits tests match the new active_hours_total_* fields.

The test_on_hours_limits test has been correctly adapted to:

  • Pass ActivityParameters(active_hours_total_min=20, active_hours_total_max=100).
  • Expect the Sink(Wärme)|active_hours_total variable 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/max fields and their transform in ActivityParameters. As with other tests, ensure the ActivityModel actually uses these fields when deciding to create the active_hours_total variable 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)|on binary, Sink(Wärme)|active_hours_total variable, 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_total in 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_hours feature is fully wired and operational:

  • ActivityParameters.use_consecutive_inactive_hours property exists and correctly identifies when min/max parameters are set
  • use_inactive (interface.py:1315) properly proxies to use_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 ActivityParameters class 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.

Comment on lines +82 to 83
activity_parameters=fx.ActivityParameters(effects_per_startup=1000),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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 md

Length 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.py contains 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:74 has outdated example documentation using on_off_parameters
  • flixopt/flow_system.py:53 references 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.

Comment on lines +647 to 808
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,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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_hours variable alongside Sink(Wärme)|on.
  • Forward/backward/initial/lb/ub constraints with the renamed consecutive_active_hours labels.
  • 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/max from ActivityParameters.
  • 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 -100

Length of output: 6662


🏁 Script executed:

#!/bin/bash
# Find Flow class to understand its structure
fd -t f -e py "flow.py" | head -20

Length 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 2

Length 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 3

Length 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 -80

Length of output: 4735


🏁 Script executed:

#!/bin/bash
# Look for use_consecutive_active_hours reference
rg -n "use_consecutive_active_hours" --type py

Length 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"
fi

Length 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 -50

Length 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 3

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for ActivityParameters definition to check available fields
rg -n "class ActivityParameters" --type py -A 30 | head -80

Length of output: 2692


🏁 Script executed:

#!/bin/bash
# Look for where on variable is created
rg -n "\|on" --type py -C 2 | head -100

Length 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 -50

Length 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 -60

Length 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 -100

Length of output: 5646


🏁 Script executed:

#!/bin/bash
# Find Flow implementation in detail
rg -n "class Flow" --type py -A 50 | head -150

Length 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 -150

Length 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 -80

Length 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 -80

Length 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 -120

Length 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 -200

Length of output: 13486


🏁 Script executed:

#!/bin/bash
# Look for ActivityModel class directly
rg -n "^class ActivityModel" --type py -A 40 | head -150

Length 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 py

Length 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 -i

Length of output: 705


🏁 Script executed:

#!/bin/bash
# Final check: look for all references to consecutive in codebase
rg -n "consecutive" --type py | head -200

Length 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 -i

Length 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"
fi

Length 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 -200

Length 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"
fi

Length 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 -150

Length 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 -150

Length 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 -100

Length 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 -200

Length 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 -150

Length of output: 4263


🏁 Script executed:

#!/bin/bash
# Look for where FlowModel creates ActivityModel
rg -n "_create_active_inactive_model" --type py -C 10 | head -150

Length 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 -100

Length 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 -100

Length 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 30

Length 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 -100

Length 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 3

Length 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 -200

Length of output: 3381


🏁 Script executed:

#!/bin/bash
# Search for where 'on' variable should be created
rg -n "short_name.*=.*['\"]on['\"]" --type py

Length 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 -150

Length 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.py

Length 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 -100

Length 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 -250

Length 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 -100

Length 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 -80

Length 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' to short_name='on'
  • Line 865 (ComponentModel._do_modeling): Change short_name='active' to short_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),
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 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.py

Length 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.py

Length 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 -80

Length of output: 5046



🏁 Script executed:

#!/bin/bash
# Find the __init__ method signature in ActivityParameters
rg -n "def __init__" flixopt/interface.py | head -5

Length of output: 294



🏁 Script executed:

#!/bin/bash
# Get the full __init__ signature starting at line 871
sed -n '871,920p' flixopt/interface.py

Length of output: 2661



🏁 Script executed:

#!/bin/bash
# Find the line number where ActivityParameters class is defined
rg -n "^class ActivityParameters" flixopt/interface.py

Length 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.

@FBumann
Copy link
Member Author

FBumann commented Nov 18, 2025

CLosed in favor of #477

@FBumann FBumann closed this Nov 18, 2025
@FBumann FBumann deleted the feature/rename-on-off branch November 19, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants