-
Notifications
You must be signed in to change notification settings - Fork 9
Add Multi-Period-modeling and stochastic modeling to flixopt #348
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
* Bugfix plot_node_balance_pie() * Scenarios/fixes (#252) * BUGFIX missing conversion to TimeSeries * BUGFIX missing conversion to TimeSeries * Bugfix node_balance with flow_hours: Negate correctly * Scenarios/filter (#253) * Add containts and startswith to filter_solution * Scenarios/drop suffix (#251) Drop suffixes in plots and add the option to drop suffixes to sanitize_dataset() * Scenarios/bar plot (#254) * Add stacked bar style to plotting methods * Rename mode to style (line, bar, area, ...) * Bugfix plotting * Fix example_calculation_types.py * Scenarios/fixes (#255) * Fix indexing issue with only one scenario * Bugfix Cooling Tower * Add option for balanced Storage Flows (equalize size of charging and discharging) * Add option for balanced Storage Flows * Change error to warning (non-fixed size with piecewise conversion AND fixed_flow_rate with OnOff) * Bugfix in DataConverter * BUGFIX: Typo (total_max/total_min in Effect) * Bugfix in node_balance() (negating did not work when using flow_hours mode * Scenarios/effects (#256) * Add methods to track effect shares of components and Flows * Add option to include flows when retrieving effects * Add properties and methods to store effect results in a dataset * Reorder methods * Rename and improve docs * Bugfix test class name * Fix the Network algorithm to calculate the sum of parallel paths, and be independent on nr of nodes and complexity of the network * Add tests for the newtork chaining and the results of effect shares * Add methods to check for circular references * Add test to check for circular references * Update cycle checker to return the found cycles * Add checks in results to confirm effects are computed correctly * BUGFIX: Remove +1 from prior testing * Add option for grouped bars to plotting.with_plotly() and make lines of stacked bar plots invisible * Reconstruct FlowSystem in CalculationResults on demand. DEPRECATION in CalculationResults * ruff check * Bugfix: save flow_system data, not the flow_system * Update tests * Scenarios/datasets results (#257) * Use dataarray instead of dataset * Change effects dataset to dataarray and use nan when no share was found * Add method for flow_rates dataset * Add methods to get flow_rates and flow_hours as datasets * Rename the dataarrays to the flow * Preserve index order * Improve filter_edges_dataset() * Simplify _create_flow_rates_dataarray() * Add dataset for sizes of Flows * Extend results structure to contain flows AND start/end infos * Add FlowResults Object * BUGFIX:Typo in _ElementResults.constraints * Add flows to results of Nodes * Simplify dataarray creation and improve FlowResults * Add nice docstrings * Improve filtering of flow results * Improve filtering of flow results. Add attribute of component * Add big dataarray with all variables but indexed * Revert "Add big dataarray with all variables but indexed" This reverts commit 08cd8a1. * Improve filtering method for coords filter and add error handling for restoring the flow system * Remove unnecessary methods in results .from_json() * Ensure consistent coord ordering in Effects dataarray * Rename get_effects_per_component() * Make effects_per_component() a dataset instead of a dataarray * Improve backwards compatability * ruff check * ruff check * Scenarios/deprecation (#258) * Deprecate .active_timesteps * Improve logger warning * Starting release notes * Bugfix in plausibility_check: Index 0 * Set bargap to 0 in stacked bars * Ensure the size is always properly indexed in results. * ruff check * BUGFIX in extract data, that causes coords in linopy to be incorrect (scalar xarray.DataArrays) * Improve yaml formatting for model documentation (#259) * Make the size/capacity a TimeSeries (#260) * Scenarios/plot network (#262) * Catch bug in plot_network with 2D arrays * Add plot_network() to test_io.py * Update deploy-docs.yaml: Run on Release publishing instead of creation and only run for stable releases (vx.y.z) * Bugfix DataConverter and add tests (#263) * Fix doc deployment to not publish on non stable releases * Remove unused code * Remove legend placing for better auto placing in plotly * Fix plotly dependency * Improve validation when adding new effects * Moved release notes to CHANGELOG.md * Try to add to_dataset to Elements * Remove TimeSeries * Remove TimeSeries * Rename conversion method to pattern: to_... * Move methods to FlowSystem * Drop nan values across time dimension if present * Allow lists of values to create DataArray * Update resolving of FlowSystem * Simplify TimeSeriesData * Move TImeSeriesData to Structure and simplyfy to inherrit from xarray.DataArray * Adjust IO * Move TimeSeriesData back to core.py and fix Conversion * Adjust IO to account for attrs of DataArrays in a Dataset * Rename transforming and connection methods in FlowSystem * Compacted IO methods * Remove infos() * remove from_dict() and to_dict() * Update __str__ of Interface * Improve str and repr * Improve str and repr * Add docstring * Unify IO stuff in Interface class * Improve test tu utilize __eq__ method * Make Interface class more robust and improve exceptions * Add option to copy Interfaces (And the FlowSystem) * Make a copy of a FLowSytsem that gets reused in a second Calculation * Remove test_timeseries.py * Reorganizing Datatypes * Remove TImeSeries and TimeSeriesCollection entirely * Remove old method * Add option to get structure with stats of dataarrays * Change __str__ method * Remove old methods * remove old imports * Add isel, sel and resample methods to FlowSystem * Remove need for timeseries with extra timestep * Simplify IO of FLowSystem * Remove parameter timesteps from IO * Improve Exceptions and Docstrings * Improve isel sel and resample methods * Change test * Bugfix * Improve * Improve * Add test for Storage Bounds * Add test for Storage Bounds * CHANGELOG.md * ruff check * Improve types * CHANGELOG.md * Bugfix in Storage * Revert changes in example_calculation_types.py * Revert changes in simple_example.py * Add convenient access to Elements in FlowSystem * Get Aggregated Calculation Working * Segmented running with wrong results * Use new persistent FLowSystem to create Calculations upfront * Improve SegmentedCalcualtion * Improve SegmentedCalcualtion * Fix SegmentedResults IO * ruff check * Update example * Updated logger essages to use .label_full instead of .label * Re-add parameters. Use deprecation warning instead * Update changelog * Improve warning message * Merge * Merge * Fit scenario weights to model coords when transforming * Merge * Removing logic between minimum, maximum and fixed size from InvestParameters * Remove selected_timesteps * Improve TypeHints * New property on InvestParameters for min/max/fixed size * Move logic for InvestParameters in Transmission to from Model to Interface * Make transformation of data more hierarchical (Flows after Components) * Add scenario validation * Change Transmission to have a "balanced" attribute. Change Tests accordingly * Improve index validations * rename method in tests * Update DataConverter * Add DataFrame Support back * Add copy() to DataConverter * Update fit_to_model_coords to take a list of coords * Make the DataConverter more universal by accepting a list of coords/dims * Update DataConverter for n-d arrays * Update DataConverter for n-d arrays * Add extra tests for 3-dims * Add FLowSystemDimension Type * Revert some logic about the fit_to_model coords * Adjust FLowSystem IO for scenarios * BUGFIX: Raise Exception instead of logging * Change usage of TimeSeriesData * Adjust logic to handle non scalars * Adjust logic to _resolve_dataarray_reference into separate method * Update IO of FlowSystem * Improve get_coords() * Adjust FlowSystem init for correct IO * Add scenario to sel and isel methods, and dont normalize scenario weights * Improve scenario_weights_handling * Add warning for not scaled weights * Update test_scenarios.py * Improve util method * Add objective to solution dataset. * Update handling of scenario_weights update tests * Ruff check. Fix type hints * Fix type hints and improve None handling * Fix coords in AggregatedCalculation * Improve Error Messages of DataConversion * Allow multi dim data conversion and broadcasting by length * Improve DataConverter to handle multi-dim arrays * Rename methods and remove unused code * Improve DataConverter by better splitting handling per datatype. Series only matches index (for one dim). Numpy matches shape * Add test for error handling * Update scenario example * Fix Handling of TimeSeriesData * Improve DataConverter * Fix resampling of the FlowSystem * Improve Warning Message * Add example that leverages resampling * Add example that leverages resampling adn fixing of Investments * Add flag to Calculation if its modeled * Make flag for connected_and_transformed FLowSystem public * Make Calcualtion Methods return themselfes to make them chainable * Improve example * Improve Unreleased CHANGELOG.md * Add year coord to FlowSystem * Improve dimension handling * Change plotting to use an indexer instead * Change plotting to use an indexer instead * Use tuples to set dimensions in Models * Bugfix in validation logic and test * Improve Errors * Improve weights handling and rescaling if None * Fix typehint * Update Broadcasting in Storage Bounds and improve type hints * Make .get_model_coords() return an actual xr.Coordinates Object * Improve get_coords() * Rename SystemModel to FlowSystemModel * First steps * Improve Feature Patterns * Improve acess to variables via short names * Improve * Add naming options to big_m_binary_bounds() * Fix and improve FLowModeling with Investment * Improve * Tyring to improve the Methods for bounding variables in different scenarios * Improve BoundingPatterns * Improve BoundingPatterns * Improve BoundingPatterns * Fix duration Modeling * Fix On + Size * Fix InvestmentModel * Fix Models * Update constraint names in test * Fix OnOffModel for multiple Flows * Update constraint names in tests * Simplify * Improve handling of vars/cons and models * Revising the basic structure of a class Model * Revising the basic structure of a class Model * Simplify and focus more on own Model class * Update tests * Improve state computation in ModelingUtilities * Improve handling of previous flowrates * Imropove repr and submodel acess * Update access pattern in tests * Fix PiecewiseEffects and StorageModel * Fix StorageModel and Remove PreventSimultaniousUseModel * Fix Aggregation and SegmentedCalculation * Update tests * Loosen precision in tests * Update test_on_hours_computation.py and some types * Rename class Model to Submodel * rename sub_model to submodel everywhere * rename self.model to self.submodel everywhere * Rename .model with .submodel if its only a submodel * Rename .sub_models with .submodels * Improve repr * Improve repr * Include def do_modeling() into __init__() of models * Make properties private * Improve Inheritance of Models * V3.0.0/plotting (#285) * Use indexer to reliably plot solutions with and wihtout scenarios/years * ruff check * Improve typehints * Update CHANGELOG.md * Bugfix from renaming to .submodel * Bugfix from renaming to .submodel * Improve indexer in results plotting * rename register_submodel() to .add_submodels() adn add SUbmodels collection class * Add nice repr to FlowSystemModel and Submodel * Bugfix .variables and .constraints * Add type checks to modeling.py * Improve assertion in tests * Improve docstrings and register ElementModels directly in FlowSystemModel * Improve __repr__() * ruff check * Use new method to compare sets in tests * ruff check * Update Contribute.md, some dependencies and add pre-commit * Pre commit hook * Run Pre-Commit Hook for the first time * Fix link in README.md * Update Effect name in tests to be 'costs' instead of 'Costs' Everywhere Simplify testing by creating a Element Library * Improve some of the modeling and coord handling * Add tests with years and scenarios * Update tests to run with multiple coords * Fix Effects dataset computation in case of empty effects * Update Test for multiple dims Fix Dim order in scaled_bounds_with_state Bugfix logic in .use_switch_on * Fix test with multiple dims * Fix test with multiple dims * New test * New test for previous flow_rates * V3.0.0/main fit to model coords improve (#295) * Change fit_to_model_coords to work with a Collection of dims * Improve fit_to_model_coords * Improve CHANGELOG.md
# Conflicts: # CHANGELOG.md # flixopt/network_app.py
# Conflicts: # CHANGELOG.md # flixopt/network_app.py
|
Caution Review failedThe pull request is closed. WalkthroughMajor v3.0.0 refactor: introduces FlowSystemModel/Submodel architecture, multi-dimension support (time/period/scenario), xarray-based TimeSeriesData/DataConverter, redesigned effects/shares API, dataset/NetCDF/JSON IO and FlowResults, many public API renames, plotting style param, logging/config changes, and widespread test/example/docs updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant FS as FlowSystem
participant FM as FlowSystemModel
participant SM as Submodels
participant Calc as Calculation
participant Solver as Solver
participant Res as CalculationResults
User->>FS: define elements, timesteps, periods, scenarios
FS->>FS: connect_and_transform()
User->>Calc: FullCalculation(FS)
Calc->>FS: create_model(normalize_weights)
FS-->>FM: FlowSystemModel created
FM->>SM: add_submodels(Flows/Components/Effects)
Calc->>FM: do_modeling() -> Calculation (returned)
Calc->>Solver: solve()
Solver-->>Calc: solution
Calc->>Res: CalculationResults.from_calculation(Calc)
Res->>FS: flow_system_data = FS.to_dataset()
Res-->>User: results (flows/components/effects)
sequenceDiagram
autonumber
actor User
participant Res as CalculationResults
participant DS as xr.Dataset
participant FS as FlowSystem
User->>Res: from_file(folder)
Res->>DS: load solution + flow_system_data
User->>Res: access .flow_system
Res->>FS: FlowSystem.from_dataset(DS)
FS-->>Res: FlowSystem (reconstructed)
Res-->>User: FlowSystem object
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
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 |
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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/test_storage.py (1)
481-487: Fix boolean comparison on xarray variables.
var.upper == 1 and var.lower == 1is ambiguous for arrays and will error. Use array-wise checks.Apply this diff:
- if 'InvestStorage|is_invested' in model.variables: - var = model.variables['InvestStorage|is_invested'] - # Check if the lower and upper bounds are both 1 - assert var.upper == 1 and var.lower == 1, ( - 'is_invested variable should be fixed to 1 when optional=False' - ) + if 'InvestStorage|is_invested' in model.variables: + var = model.variables['InvestStorage|is_invested'] + import numpy as np + assert np.allclose(var.upper, 1) and np.allclose(var.lower, 1), \ + 'is_invested variable should be fixed to 1 when optional=False'flixopt/aggregation.py (1)
288-305: Modeling hook not executed: renamedo_modelingto_do_modeling.
Submodelcalls_do_modeling()in its constructor; your override is nameddo_modeling, so nothing runs and no constraints/variables are created.Apply this diff:
-class AggregationModel(Submodel): +class AggregationModel(Submodel): @@ - def do_modeling(self): + def _do_modeling(self):Also applies to: 310-316
flixopt/io.py (1)
198-206: Confirm attr roundtrip compatibility with existing artifacts.load_dataset_from_netcdf already supports both formats: it json.loads when a top-level 'attrs' key exists and leaves plain attrs untouched; save_dataset_to_netcdf writes all attrs as JSON under the 'attrs' key—so the two functions are symmetric for JSON-serializable attrs (flixopt/io.py: save_dataset_to_netcdf, load_dataset_from_netcdf).
- Risk: save_dataset_to_netcdf uses json.dumps on attrs — this will fail or lose fidelity for non‑JSON‑serializable values (numpy dtypes/arrays, pandas Timestamps/Indices, bytes, sets, custom objects). This is a breaking change for datasets with such attrs.
- Action: convert/normalize attrs before json.dumps (numpy→Python scalars, arrays→lists, datetimes→ISO strings) or provide a robust serializer (json.dumps(..., default=...), or a safe fallback like pickle) and add unit tests that save+load datasets including nested attrs with numpy/pandas types and an older-format file load test.
Location: flixopt/io.py — save_dataset_to_netcdf / load_dataset_from_netcdf.
flixopt/linear_converters.py (1)
608-614: Fix: check_bounds fails on xr.DataArray (ambiguous truth).When value/lower/upper are xr.DataArray (not TimeSeriesData), np.all(...) returns a DataArray and the if condition raises "truth value of a DataArray is ambiguous". Convert xr.DataArray to numpy as done for TimeSeriesData.
Apply this diff and add the missing import:
@@ - if isinstance(value, TimeSeriesData): + if isinstance(value, TimeSeriesData): value = value.data - if isinstance(lower_bound, TimeSeriesData): + elif isinstance(value, xr.DataArray): + value = value.data + if isinstance(lower_bound, TimeSeriesData): lower_bound = lower_bound.data - if isinstance(upper_bound, TimeSeriesData): + elif isinstance(lower_bound, xr.DataArray): + lower_bound = lower_bound.data + if isinstance(upper_bound, TimeSeriesData): upper_bound = upper_bound.data + elif isinstance(upper_bound, xr.DataArray): + upper_bound = upper_bound.dataOutside this hunk, add:
+import xarray as xr
🧹 Nitpick comments (51)
CHANGELOG.md (7)
36-39: Fix heading level and tighten wording for multi-period sectionUse h3 (###), not h4, and simplify phrasing.
-#### Multi-period-support -A flixopt model might be modeled with a "year" dimension. -This enables to model transformation pathways over multiple years. +### Multi-period support +A flixopt model can be modeled with a 'year' dimension. +This enables modeling transformation pathways over multiple years.
40-46: Fix heading level and phrasing in stochastic sectionNormalize heading and wording.
-#### Stochastic modeling -A flixopt model can be modeled with a scenario dimension. -Scenarios can be weighted and variables can be equated across scenarios. This enables to model uncertainties in the flow system, such as: +### Stochastic modeling +A flixopt model can include a 'scenario' dimension. +Scenarios can be weighted and variables can be linked across scenarios. This enables modeling uncertainties in the flow system, such as:
53-66: Fix heading level and list indentation (markdownlint MD001/MD007)Promote to h3 and use consistent 2-space indentation for nested bullets.
-#### Improved Data handling: IO, resampling and more through xarray -* IO for all Interfaces and the FlowSystem with round-trip serialization support - * NetCDF export/import capabilities for all Interface objects and FlowSystem - * JSON export for documentation purposes - * Recursive handling of nested Interface objects -* FlowSystem data manipulation methods - * `sel()` and `isel()` methods for temporal data selection - * `resample()` method for temporal resampling - * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data - * `__eq__()` method for FlowSystem comparison - -* Core data handling improvements - * `get_dataarray_stats()` function for statistical summaries - * Enhanced `DataConverter` class with better TimeSeriesData support +### Improved data handling: I/O, resampling, and more through xarray +* I/O for all Interfaces and the FlowSystem with round-trip serialization support + * NetCDF export/import capabilities for all Interface objects and FlowSystem + * JSON export for documentation purposes + * Recursive handling of nested Interface objects +* FlowSystem data manipulation methods + * `sel()` and `isel()` methods for temporal data selection + * `resample()` method for temporal resampling + * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data + * `__eq__()` method for FlowSystem comparison +* Core data handling improvements + * `get_dataarray_stats()` function for statistical summaries + * Enhanced `DataConverter` class with better TimeSeriesData support
69-78: Correct typos and naming in “Added” bulletsFix spelling, casing, and clarity.
-### Added -* FlowSystem Restoring: The used FlowSystem will now get restired from the results (lazily). ALll Parameters can be safely acessed anytime after the solve. -* FLowResults added as a new class to store the results of Flows. They can now be accessed directly. -* Added precomputed DataArrays for `size`s, `flow_rate`s and `flow_hour`s. +### Added +* FlowSystem restoring: The used FlowSystem will now get restored from the results (lazily). All parameters can be safely accessed anytime after the solve. +* FlowResults added as a new class to store the results of flows. They can now be accessed directly. +* Added precomputed DataArrays for `sizes`, `flow_rates`, and `flow_hours`. * Added `effects_per_component()`-Dataset to Results that stores the direct (and indirect) effects of each component. This greatly improves the evaluation of the impact of individual Components, even with many and complex effects. * Improved filter methods for Results -* Balanced storage - Storage charging and discharging sizes can now be forced to be equal in when optimizing their size. -* Added Example for 2-stage Investment decisions leveraging the resampling of a FlowSystem -* New Storage Parameter: `relative_minimum_final_charge_state` and `relative_maximum_final_charge_state` parameter for final state control +* Balanced storage – storage charging and discharging sizes can now be forced to be equal when optimizing their size. +* Added example for 2‑stage investment decisions leveraging resampling of a FlowSystem +* New storage parameters: `relative_minimum_final_charge_state` and `relative_maximum_final_charge_state` for final state control
89-91: Minor typo in “Changed” bulletsFix “acess” -> “access”.
- * Submodel: The base class for all submodels. Each is a subset of the Model, for simpler acess and clearer code. + * Submodel: The base class for all submodels. Each is a subset of the Model, for simpler access and clearer code.
105-107: Polish “Known issues” text and fix typosClarify and correct spelling.
-* IO for single Interfaces/Elemenets to Datasets might not work properly if the Interface/Element is not part of a fully transformed and connected FlowSystem. This arrises from Numeric Data not being stored as xr.DataArray by the user. To avoid this, always use the `to_dataset()` on Elements inside a FlowSystem thats connected and transformed. +* I/O for single Interfaces/Elements to Datasets might not work properly if the Interface/Element is not part of a fully transformed and connected FlowSystem. This arises from numeric data not being stored as `xr.DataArray` by the user. To avoid this, always use `to_dataset()` on Elements inside a FlowSystem that’s connected and transformed.
108-121: Deduplicate items and fix markdown emphasis (MD050) in “Development”Remove repeated renames; fix spacing and wrap repr in backticks; normalize list indent.
### *Development* * **BREAKING**: Calculation.do_modeling() now returns the Calculation object instead of its linopy.Model -* **BREAKING**: Renamed class `SystemModel` to `FlowSystemModel` -* **BREAKING**: Renamed class `Model` to `Submodel` * FlowSystem data management simplified - removed `time_series_collection` pattern in favor of direct timestep properties * Change modeling hierarchy to allow for more flexibility in future development. This leads to minimal changes in the access and creation of Submodels and their variables. -* Added new module `.modeling`that contains Modelling primitives and utilities +* Added new module `.modeling` that contains modeling primitives and utilities * Clearer separation between the main Model and "Submodels" * Improved access to the Submodels and their variables, constraints and submodels -* Added __repr__() for Submodels to easily inspect its content +* Added `__repr__()` for Submodels to easily inspect its content * Enhanced data handling methods - * `fit_to_model_coords()` method for data alignment - * `fit_effects_to_model_coords()` method for effect data processing - * `connect_and_transform()` method replacing several operations + * `fit_to_model_coords()` method for data alignment + * `fit_effects_to_model_coords()` method for effect data processing + * `connect_and_transform()` method replacing several operationsexamples/05_Two-stage-optimization/two_stage_optimization.py (2)
2-8: Fix typos in module docstringMinor grammar/typos.
-This can be very useful when working with large models or during developement state, +This can be very useful when working with large models or during development, @@ -A common use case is to do optimize the investments of a model with a downsampled version of the original model, and than fix the computed sizes when calculating th actual dispatch. +A common use case is to optimize the investments of a model with a downsampled version of the original model, and then fix the computed sizes when calculating the actual dispatch.
127-131: xarray boolean comparison needs scalar conversion; fix log textUse .all().item() for a Python bool; fix “where”→“were”.
-if (calculation_dispatch.results.sizes().round(5) == calculation_sizing.results.sizes().round(5)).all(): - logger.info('Sizes where correctly equalized') +if (calculation_dispatch.results.sizes().round(5) == calculation_sizing.results.sizes().round(5)).all().item(): + logger.info('Sizes were correctly equalized') else: - raise RuntimeError('Sizes where not correctly equalized') + raise RuntimeError('Sizes were not correctly equalized')examples/03_Calculation_types/example_calculation_types.py (1)
136-141: Use effect label key instead of Effect object in mapping (deprecated)Align with deprecation notice; use CO2.label.
- source=fx.Flow( - 'P_el', bus='Strom', size=1000, effects_per_flow_hour={costs.label: TS_electricity_price_buy, CO2: 0.3} - ), + source=fx.Flow( + 'P_el', bus='Strom', size=1000, effects_per_flow_hour={costs.label: TS_electricity_price_buy, CO2.label: 0.3} + ),tests/run_all_tests.py (1)
10-10: Confirm intention to only run test_integrationIf not intentional, run full suite by default.
- pytest.main(['test_integration.py', '--disable-warnings']) + pytest.main(['-q', '--disable-warnings'])tests/test_functional.py (1)
217-249: Verify the capitalization change in error messages.The error messages have been changed from
'costs doesnt match expected value'to'costs doesnt match expected value'with inconsistent capitalization. While this is minor, it would be better to maintain consistency.Consider standardizing the error messages:
- 'costs doesnt match expected value' + 'costs does not match expected value'tests/test_integration.py (1)
309-316: Fix inconsistent error message formatting.The error messages use different formats: lowercase "costs do not match" vs other patterns. Consider standardizing.
- f'costs do not match for {modeling_type} modeling type', + f'Costs do not match for {modeling_type} modeling type',tests/conftest.py (1)
44-63: Consider adding docstring to the coords_config fixture.This fixture provides critical test configurations for different coordinate combinations. Adding a docstring would improve maintainability.
@pytest.fixture( params=[ {'timesteps': pd.date_range('2020-01-01', periods=10, freq='h', name='time'), 'years': None, 'scenarios': None}, { 'timesteps': pd.date_range('2020-01-01', periods=10, freq='h', name='time'), 'years': pd.Index([2020, 2030, 2040], name='year'), 'scenarios': None, }, { 'timesteps': pd.date_range('2020-01-01', periods=10, freq='h', name='time'), 'years': pd.Index([2020, 2030, 2040], name='year'), 'scenarios': pd.Index(['A', 'B'], name='scenario'), }, ], ids=['time_only', 'time+years', 'time+years+scenarios'], ) def coords_config(request): - """Coordinate configurations for parametrized testing.""" + """ + Coordinate configurations for parametrized testing. + + Returns: + dict: Configuration with 'timesteps', 'years', and 'scenarios' keys. + Tests will run with each parameter combination. + """ return request.paramtests/test_effects_shares_summation.py (1)
95-126: Fix incorrect comment on line 124.The comment shows the wrong total value. The comment should reflect the correct calculation.
# Effect1 -> Effect3 has two paths: # 1. Effect1 -> Effect2 -> Effect3 = 1.1 * 5.0 = 5.5 # 2. Effect1 -> Effect3 = 1.2 - # Total = 0.6 + 2.75 = 3.35 + # Total = 5.5 + 1.2 = 6.7 assert result[('Effect1', 'Effect3')].item() == 1.2 + 1.1 * 5.0examples/04_Scenarios/scenario_example.py (3)
1-9: Consider adding more comprehensive module docstring.The docstring could be expanded to better describe what the example demonstrates.
""" -This script shows how to use the flixopt framework to model a simple energy system. +This script demonstrates how to use the flixopt framework to model a simple energy system +with multiple scenarios, years, and various components including: +- Energy buses (electricity, heat, gas) +- Conversion components (Boiler, CHP) +- Storage systems +- Sources and sinks +- Multi-period optimization with scenarios """
122-129: Remove duplicate plot_heatmap call.Line 122 and line 128 contain the same plot call for
'CHP(Q_th)|flow_rate'.calculation.results.plot_heatmap('CHP(Q_th)|flow_rate') # --- Analyze Results --- calculation.results['Fernwärme'].plot_node_balance_pie() calculation.results['Fernwärme'].plot_node_balance(style='stacked_bar') calculation.results['Storage'].plot_node_balance() - calculation.results.plot_heatmap('CHP(Q_th)|flow_rate')
134-138: Consider handling plot return values consistently.Line 137 assigns the plot result to
fig, axbut doesn't use them. Either use the variables or remove the assignment for consistency.# Save results to file for later usage calculation.results.to_file() - fig, ax = calculation.results['Storage'].plot_charge_state(engine='matplotlib') + calculation.results['Storage'].plot_charge_state(engine='matplotlib')flixopt/plotting.py (4)
201-205: Broaden exception handling for invalid colormap names.
px.colors.get_colorscaletypically raises ValueError for unknown scales, not PlotlyError. Catch both to ensure fallback works.Apply this diff:
- try: - colorscale = px.colors.get_colorscale(colormap_name) - except PlotlyError as e: + try: + colorscale = px.colors.get_colorscale(colormap_name) + except (PlotlyError, ValueError) as e: logger.warning(f"Colorscale '{colormap_name}' not found in Plotly. Using {self.default_colormap}: {e}") colorscale = px.colors.get_colorscale(self.default_colormap)Also applies to: 213-216
330-336: Add a deprecatedmodealias for backward compatibility.Public API changed from
modetostyle. Keep a soft-landing path to avoid breaking external users.Apply this diff in the signature and early in the function:
def with_plotly( data: pd.DataFrame, - style: Literal['stacked_bar', 'line', 'area', 'grouped_bar'] = 'stacked_bar', + style: Literal['stacked_bar', 'line', 'area', 'grouped_bar'] = 'stacked_bar', + mode: Literal['bar', 'line', 'area'] | None = None, colors: ColorType = 'viridis', title: str = '', ylabel: str = '', xlabel: str = 'Time in h', fig: go.Figure | None = None, ) -> go.Figure: @@ - if style not in ('stacked_bar', 'line', 'area', 'grouped_bar'): + if mode is not None: + import warnings + warnings.warn("Parameter 'mode' is deprecated; use 'style' instead.", DeprecationWarning, stacklevel=2) + style = {'bar': 'stacked_bar', 'line': 'line', 'area': 'area'}.get(mode, style) + if style not in ('stacked_bar', 'line', 'area', 'grouped_bar'): raise ValueError(f"'style' must be one of {{'stacked_bar','line','area', 'grouped_bar'}}, got {style!r}")Also applies to: 357-358
366-393: Useeliffor mutually exclusive styles and fix misleading comments.
- Switch second
iftoelifto avoid unnecessary checks.- Comments contradict values:
bargap=0.2is not “No space”.Apply this diff:
- if style == 'grouped_bar': + elif style == 'grouped_bar': @@ - fig.update_layout( - barmode='group', - bargap=0.2, # No space between bars - bargroupgap=0, # space between grouped bars - ) + fig.update_layout( + barmode='group', + bargap=0.2, # Small space between bars + bargroupgap=0, # No space within grouped bars + )
470-507: Potential width unit issue for datetime x-axis in Matplotlib bars.
width = data.index.to_series().diff().dropna().min()yields a Timedelta for datetime indexes; Matplotlib bar expects numeric in axis units. Convert Timedelta to days (or use a reasonable default) to avoid backend-dependent errors.Suggested change:
- width = data.index.to_series().diff().dropna().min() # Minimum time difference + delta = data.index.to_series().diff().dropna().min() + if isinstance(delta, pd.Timedelta): + width = delta / pd.Timedelta(days=1) # Matplotlib date units are days + else: + width = delta if delta else 0.8Also applies to: 514-518
tests/test_scenarios.py (2)
14-112: Unused fixturetest_system.This fixture isn’t referenced by tests. Remove or wire it into a test to avoid dead code and reduce CI time.
282-287: Redundant re-solve after IO roundtrip.The second
create_calculation_and_solvecall re-runs the same model immediately after loading, without validating against persisted results. Drop it or assert equivalence to reduce runtime.Apply this diff:
- calc = create_calculation_and_solve( - flow_system_piecewise_conversion_scenarios, - solver=fx.solvers.GurobiSolver(mip_gap=0.01, time_limit_seconds=60), - name='test_full_scenario', - ) + # Optionally compare against persisted results here if needed.flixopt/io.py (2)
66-66: Limit YAML representer to the custom dumper to avoid global side effects.Registering the representer on
yaml.SafeDumperis global. Attach it toCustomDumperto prevent affecting other dumps.Apply this diff:
- yaml.add_representer(str, represent_str, Dumper=yaml.SafeDumper) + class CustomDumper(yaml.SafeDumper): + def increase_indent(self, flow=False, indentless=False): + return super().increase_indent(flow, False) + CustomDumper.add_representer(str, represent_str)Also applies to: 69-72
229-235: Make attribute JSON serialization robust.
json.dumpswill fail on non-JSONable attrs (e.g., numpy types). Provide a fallback viadefault=str.Apply this diff:
- ds.attrs = {'attrs': json.dumps(ds.attrs)} + ds.attrs = {'attrs': json.dumps(ds.attrs, default=str)} @@ - if data_var.attrs: # Only if there are attrs - ds[var_name].attrs = {'attrs': json.dumps(data_var.attrs)} + if data_var.attrs: # Only if there are attrs + ds[var_name].attrs = {'attrs': json.dumps(data_var.attrs, default=str)} @@ - if hasattr(coord_var, 'attrs') and coord_var.attrs: - ds[coord_name].attrs = {'attrs': json.dumps(coord_var.attrs)} + if hasattr(coord_var, 'attrs') and coord_var.attrs: + ds[coord_name].attrs = {'attrs': json.dumps(coord_var.attrs, default=str)}Also applies to: 237-240
tests/test_storage.py (2)
14-15: Simplify fixture unpacking for clarity.
flow_system, coords_config = basic_flow_system_linopy_coords, coords_configis redundant. Assign directly.Example:
- flow_system, coords_config = basic_flow_system_linopy_coords, coords_config + flow_system = basic_flow_system_linopy_coordsApply similarly to other tests.
Also applies to: 85-88, 170-172, 256-257, 308-309, 353-354, 385-387, 442-443
53-62: Avoid mutating the model by creating comparison variables.Using
model.add_variables(...)inside tests mutates the model under test. Prefer constructing a separate (temporary) model or building expected linopy.Variable objects without registering them.Also applies to: 128-138, 211-231
flixopt/aggregation.py (2)
326-334: Indexing linopy.Variables with a set can be unstable.Sets are unordered. Convert to a sorted list to ensure determinism.
Apply this diff:
- all_variables_of_component = set(component.submodel.variables) + all_variables_of_component = set(component.submodel.variables) @@ - relevant_variables = component.submodel.variables[all_variables_of_component & time_variables] + relevant_variables = component.submodel.variables[sorted(all_variables_of_component & time_variables)] else: - relevant_variables = component.submodel.variables[all_variables_of_component & binary_time_variables] + relevant_variables = component.submodel.variables[sorted(all_variables_of_component & binary_time_variables)]
144-169: Update plotting calls to newstylekeyword for readability.Positional
'line'works but is brittle. Prefer explicitstyle='line'for both calls.Apply this diff:
- fig = plotting.with_plotly(df_org, 'line', colors=colormap) + fig = plotting.with_plotly(df_org, style='line', colors=colormap) @@ - fig = plotting.with_plotly(df_agg, 'line', colors=colormap, fig=fig) + fig = plotting.with_plotly(df_agg, style='line', colors=colormap, fig=fig)Also applies to: 153-157
flixopt/linear_converters.py (2)
76-96: Initialize eta via setter to enforce bounds on creation.Currently eta bounds aren't checked at construction. Set via the property to run check_bounds.
self.Q_fu = Q_fu self.Q_th = Q_th + # ensure bounds check on initialization + self.eta = eta
163-171: Initialize eta via setter to enforce bounds on creation.Mirror Boiler: call the setter so eta is validated at init.
self.P_el = P_el self.Q_th = Q_th + # ensure bounds check on initialization + self.eta = etaAlso applies to: 181-192
tests/test_component.py (1)
31-34: Remove redundant fixture re-assignment.The tuple assignment rebinds variables to themselves and is a no-op. Drop it to reduce noise.
Example (apply similarly to all occurrences):
- flow_system, coords_config = basic_flow_system_linopy_coords, coords_config + flow_system = basic_flow_system_linopy_coordsAlso applies to: 72-76, 175-178, 235-237, 370-372
flixopt/core.py (1)
315-321: Optional: accept 1D DataArray with matching coordinates but different dim name.Current _broadcast_to_target requires source dim names to match target dims exactly. Many users construct xr.DataArray(v, coords=(time_index,)) which creates dim_0. Consider auto-mapping a single source dim to a target dim if coordinates match by values.
If you want to pursue this, I can propose a minimal patch that attempts dim renaming when len(data.dims)==1 and data.coords[dim].equals(coords[target_dim]).
tests/test_linear_converter.py (2)
53-54: Name the time dimension explicitly to reduce fragility.Construct the DataArray with dim 'time' so downstream conversion/broadcasting doesn't rely on dim-name relaxation.
- efficiency_series = xr.DataArray(varying_efficiency, coords=(timesteps,)) + efficiency_series = xr.DataArray(varying_efficiency, coords={'time': timesteps}, dims='time')
248-256: Same here: prefer explicit dim when building fluctuating_cop.Keeps consistency with the rest of the suite.
- conversion_factors = [{input_flow.label: fluctuating_cop, output_flow.label: np.ones(len(timesteps))}] + conversion_factors = [ + {input_flow.label: xr.DataArray(fluctuating_cop, coords={'time': timesteps}, dims='time'), + output_flow.label: xr.DataArray(np.ones(len(timesteps)), coords={'time': timesteps}, dims='time')} + ]flixopt/elements.py (1)
488-490: Nit: typo in error message."notsupported" -> "not yet supported".
- f'Different values in different years or scenarios are not yetsupported.' + f'Different values in different years or scenarios are not yet supported.'flixopt/modeling.py (1)
522-524: Fix error message: wrong function nameCopy-paste from another method.
Apply:
- if not isinstance(model, Submodel): - raise ValueError('BoundingPatterns.active_bounds_with_state() can only be used with a Submodel') + if not isinstance(model, Submodel): + raise ValueError('BoundingPatterns.scaled_bounds_with_state() can only be used with a Submodel')flixopt/interface.py (2)
921-924: Fix typos in warning textMinor text quality.
Apply:
- 'When using investment_scenarios, minimum_size and fixed_size should only ne used if optional is True.' - 'Otherwise the investment cannot be 0 incertain scenarios while being non-zero in others.' + 'When using investment_scenarios, minimum_size and fixed_size should only be used if optional is True. ' + 'Otherwise, the investment cannot be 0 in certain scenarios while being non-zero in others.'
1186-1189: Nit: spelling in docstring“wether” → “whether”.
Apply:
- """Determines wether a Variable for SWITCH-ON is needed or not""" + """Determines whether a variable for SWITCH-ON is needed or not"""flixopt/calculation.py (3)
160-168: Correct deprecation message in active_timesteps propertyMessage self-references; point to FlowSystem.sel/isel.
Apply:
- warnings.warn( - 'active_timesteps is deprecated. Use active_timesteps instead.', + warnings.warn( + "active_timesteps is deprecated. Use flow_system.sel(time=...) or flow_system.isel(time=...) instead.", DeprecationWarning, stacklevel=2, )
326-338: Prefer robust scalar extraction for timestep sizes in aggregationUse xarray reductions and cast to float to avoid ndarray/DataArray arithmetic pitfalls.
Apply:
- dt_min, dt_max = ( - np.min(self.flow_system.hours_per_timestep), - np.max(self.flow_system.hours_per_timestep), - ) + dt_min = float(self.flow_system.hours_per_timestep.min().item()) + dt_max = float(self.flow_system.hours_per_timestep.max().item()) @@ - steps_per_period = self.aggregation_parameters.hours_per_period / self.flow_system.hours_per_timestep.max() - is_integer = ( - self.aggregation_parameters.hours_per_period % self.flow_system.hours_per_timestep.max() - ).item() == 0 - if not (steps_per_period.size == 1 and is_integer): + steps_per_period = self.aggregation_parameters.hours_per_period / dt_max + is_integer = (self.aggregation_parameters.hours_per_period % dt_max) == 0 + if not is_integer: raise ValueError( f'The selected {self.aggregation_parameters.hours_per_period=} does not match the time ' f'step size of {dt_min} hours). It must be a multiple of {dt_min} hours.' )
145-146: Use already-rounded main_resultsYou call round_nested_floats in main_results; duplicate rounding here is redundant.
flixopt/components.py (1)
860-865: Balanced sizes: remove no-op multiplications
* 1is redundant and can be dropped.flixopt/effects.py (4)
499-501: Clarify error message for per-hour bounds without time-dimThe condition triggers if either bound is provided, not “both”. Update message to avoid confusion.
- raise ValueError('Both max_per_hour and min_per_hour cannot be used when has_time_dim is False') + raise ValueError("max_per_hour and/or min_per_hour cannot be used when 'time' is not in dims")
562-597: Avoid O(n²) queue by using deque in BFSUsing list.pop(0) makes the traversal quadratic. Switch to deque for linear-time iteration.
+from collections import deque @@ - queue = [(origin, 1, [origin])] + queue = deque([(origin, 1, [origin])]) @@ - while queue: - current_domain, factor, path = queue.pop(0) + while queue: + current_domain, factor, path = queue.popleft() @@ - queue.append((target, indirect_factor, new_path)) + queue.append((target, indirect_factor, new_path))
337-357: Guard scalar-effect input when no standard effect is setIf a scalar is provided but no standard effect exists, this will raise in the property access. Fail fast with a clear error.
if effect_values_user is None: return None - if isinstance(effect_values_user, dict): + if isinstance(effect_values_user, dict): return {get_effect_label(effect): value for effect, value in effect_values_user.items()} - return {self.standard_effect.label: effect_values_user} + if self._standard_effect is None: + raise KeyError( + 'Scalar effect value provided but no standard effect is configured. ' + 'Either set an effect as is_standard=True or provide a mapping {effect_label: value}.' + ) + return {self.standard_effect.label: effect_values_user}
12-12: Remove duplicate Iterator importIterator is imported at runtime and under TYPE_CHECKING. Keep type-only import to reduce runtime surface.
-from collections.abc import Iteratorflixopt/features.py (1)
169-179: Pass coordinates, not dims list, to tracking variableModelingPrimitives likely expects coords (Coordinates), not a raw list. Use model.get_coords for ['year','scenario'].
- short_name='on_hours_total', - coords=['year', 'scenario'], + short_name='on_hours_total', + coords=self._model.get_coords(['year', 'scenario']),flixopt/flow_system.py (1)
467-474: Align attribute naming: use .model consistentlyFlowSystem.init defines self.model but create_model assigns self.submodel. Prefer one attribute to avoid confusion and downstream bugs.
- self.submodel = FlowSystemModel(self) - return self.submodel + self.model = FlowSystemModel(self) + return self.modelflixopt/results.py (1)
295-301: Avoid shadowing FlowSystem importRedundant re-import risks shadowing; use the module-level import already present.
- from . import FlowSystem - current_logger_level = logger.getEffectiveLevel() logger.setLevel(logging.CRITICAL) self._flow_system = FlowSystem.from_dataset(self.flow_system_data) self._flow_system._connect_network() logger.setLevel(current_logger_level)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/images/flixopt-icon.svgis excluded by!**/*.svgexamples/05_Two-stage-optimization/Zeitreihen2020.csvis excluded by!**/*.csvpics/flixopt-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (40)
CHANGELOG.md(1 hunks)examples/01_Simple/simple_example.py(1 hunks)examples/03_Calculation_types/example_calculation_types.py(4 hunks)examples/04_Scenarios/scenario_example.py(1 hunks)examples/05_Two-stage-optimization/two_stage_optimization.py(1 hunks)flixopt/aggregation.py(6 hunks)flixopt/calculation.py(14 hunks)flixopt/components.py(12 hunks)flixopt/core.py(3 hunks)flixopt/effects.py(10 hunks)flixopt/elements.py(9 hunks)flixopt/features.py(7 hunks)flixopt/flow_system.py(10 hunks)flixopt/interface.py(8 hunks)flixopt/io.py(5 hunks)flixopt/linear_converters.py(10 hunks)flixopt/modeling.py(1 hunks)flixopt/plotting.py(8 hunks)flixopt/results.py(30 hunks)flixopt/structure.py(6 hunks)flixopt/utils.py(1 hunks)tests/conftest.py(11 hunks)tests/run_all_tests.py(1 hunks)tests/test_bus.py(4 hunks)tests/test_component.py(5 hunks)tests/test_cycle_detection.py(1 hunks)tests/test_dataconverter.py(1 hunks)tests/test_effect.py(6 hunks)tests/test_effects_shares_summation.py(1 hunks)tests/test_flow.py(16 hunks)tests/test_functional.py(11 hunks)tests/test_integration.py(4 hunks)tests/test_io.py(4 hunks)tests/test_linear_converter.py(14 hunks)tests/test_on_hours_computation.py(1 hunks)tests/test_plots.py(1 hunks)tests/test_results_plots.py(1 hunks)tests/test_scenarios.py(1 hunks)tests/test_storage.py(10 hunks)tests/test_timeseries.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_timeseries.py
🧰 Additional context used
🧬 Code graph analysis (34)
tests/test_plots.py (1)
flixopt/plotting.py (2)
with_plotly(328-466)with_matplotlib(469-562)
tests/test_results_plots.py (1)
flixopt/results.py (1)
plot_charge_state(1134-1206)
examples/05_Two-stage-optimization/two_stage_optimization.py (3)
flixopt/results.py (3)
flow_system(290-307)size(1283-1291)sizes(434-461)flixopt/flow_system.py (3)
FlowSystem(46-879)resample(825-875)sel(755-788)flixopt/calculation.py (5)
FullCalculation(174-258)do_modeling(182-190)do_modeling(303-317)solve(219-258)fix_sizes(192-217)
tests/test_effects_shares_summation.py (1)
flixopt/effects.py (2)
effects(411-412)calculate_all_conversion_paths(533-601)
tests/test_cycle_detection.py (1)
flixopt/effects.py (2)
effects(411-412)detect_cycles(604-651)
tests/test_integration.py (3)
flixopt/structure.py (2)
solution(106-129)values(991-992)tests/conftest.py (1)
assert_almost_equal_numeric(669-681)flixopt/results.py (6)
solution_without_overlap(1458-1471)to_file(776-823)to_file(1509-1532)SegmentedCalculationResults(1294-1532)from_file(119-150)from_file(1402-1426)
tests/test_io.py (3)
tests/conftest.py (2)
simple_flow_system_scenarios(412-440)simple_flow_system(382-408)flixopt/flow_system.py (4)
plot_network(475-512)to_json(330-344)to_dataset(229-241)FlowSystem(46-879)flixopt/structure.py (2)
to_json(631-648)to_dataset(494-517)
flixopt/io.py (2)
flixopt/structure.py (2)
items(985-986)copy(687-699)flixopt/flow_system.py (1)
coords(743-749)
tests/test_bus.py (4)
tests/conftest.py (5)
basic_flow_system_linopy_coords(645-660)coords_config(60-62)assert_conequal(705-721)assert_var_equal(724-758)create_linopy_model(699-702)flixopt/structure.py (4)
variables(894-900)constraints(885-891)get_coords(139-168)get_coords(840-845)flixopt/flow_system.py (2)
coords(743-749)add_elements(441-465)flixopt/elements.py (2)
Bus(119-207)Flow(224-504)
flixopt/linear_converters.py (1)
flixopt/core.py (1)
TimeSeriesData(41-149)
examples/03_Calculation_types/example_calculation_types.py (3)
flixopt/core.py (3)
TimeSeriesData(41-149)aggregation_weight(108-109)aggregation_group(104-105)flixopt/solvers.py (1)
HighsSolver(61-81)flixopt/plotting.py (1)
with_plotly(328-466)
flixopt/modeling.py (4)
flixopt/config.py (1)
CONFIG(98-153)flixopt/structure.py (12)
Submodel(757-933)values(991-992)hours_per_step(132-133)hours_per_step(928-929)add_variables(781-790)get_coords(139-168)get_coords(840-845)add_constraints(792-801)constraints(885-891)variables(894-900)get(833-838)get(998-1000)flixopt/flow_system.py (1)
coords(743-749)flixopt/elements.py (2)
previous_states(721-734)previous_states(849-866)
tests/test_storage.py (2)
tests/conftest.py (5)
basic_flow_system_linopy_coords(645-660)coords_config(60-62)assert_var_equal(724-758)create_linopy_model(699-702)assert_conequal(705-721)flixopt/structure.py (6)
get_coords(139-168)get_coords(840-845)variables(894-900)constraints(885-891)hours_per_step(132-133)hours_per_step(928-929)
flixopt/aggregation.py (1)
flixopt/structure.py (5)
FlowSystemModel(82-200)Submodel(757-933)variables(894-900)add_constraints(792-801)add_variables(781-790)
tests/test_scenarios.py (5)
flixopt/interface.py (5)
InvestParameters(663-932)PiecewiseEffects(450-659)Piecewise(83-224)Piece(26-79)PiecewiseConversion(228-446)flixopt/components.py (1)
Storage(235-516)tests/conftest.py (3)
Storage(248-296)create_calculation_and_solve(684-696)create_linopy_model(699-702)flixopt/flow_system.py (3)
FlowSystem(46-879)from_dataset(244-297)sel(755-788)flixopt/structure.py (7)
weights(171-178)label_full(732-733)label_full(871-872)values(991-992)variables(894-900)from_dataset(538-571)do_modeling(98-103)
examples/04_Scenarios/scenario_example.py (3)
flixopt/results.py (5)
flow_system(290-307)plot_network(754-774)plot_heatmap(688-752)plot_heatmap(1473-1507)plot_heatmap(1535-1593)flixopt/flow_system.py (3)
FlowSystem(46-879)add_elements(441-465)plot_network(475-512)flixopt/calculation.py (4)
FullCalculation(174-258)do_modeling(182-190)do_modeling(303-317)solve(219-258)
tests/test_dataconverter.py (1)
flixopt/core.py (5)
ConversionError(35-38)DataConverter(161-427)TimeSeriesData(41-149)to_dataarray(329-392)aggregation_group(104-105)
tests/test_component.py (2)
tests/conftest.py (9)
assert_almost_equal_numeric(669-681)assert_conequal(705-721)assert_sets_equal(761-779)assert_var_equal(724-758)create_calculation_and_solve(684-696)create_linopy_model(699-702)basic_flow_system_linopy_coords(645-660)coords_config(60-62)basic_flow_system(444-459)flixopt/structure.py (5)
variables(894-900)constraints(885-891)get_coords(139-168)get_coords(840-845)values(991-992)
tests/test_functional.py (5)
tests/conftest.py (1)
costs(99-100)flixopt/structure.py (2)
solution(106-129)values(991-992)flixopt/elements.py (3)
_investment(709-711)on_off(702-706)flow_rate(621-623)flixopt/features.py (5)
size(121-123)is_invested(126-130)off(259-261)switch_on(264-266)switch_off(269-271)flixopt/results.py (2)
size(1283-1291)flow_rate(1275-1276)
flixopt/interface.py (4)
flixopt/structure.py (4)
Interface(203-707)transform_data(221-230)values(991-992)items(985-986)flixopt/components.py (3)
transform_data(207-213)transform_data(423-463)transform_data(694-701)flixopt/flow_system.py (3)
FlowSystem(46-879)fit_to_model_coords(346-395)fit_effects_to_model_coords(397-421)tests/conftest.py (1)
piecewise(203-220)
tests/test_on_hours_computation.py (1)
flixopt/modeling.py (3)
ModelingUtilities(101-176)compute_consecutive_hours_in_state(103-125)compute_previous_states(128-129)
flixopt/features.py (6)
flixopt/interface.py (9)
InvestParameters(663-932)OnOffParameters(936-1197)Piecewise(83-224)minimum_or_fixed_size(927-928)maximum_or_fixed_size(931-932)use_off(1171-1173)use_switch_on(1186-1197)use_consecutive_on_hours(1176-1178)use_consecutive_off_hours(1181-1183)flixopt/modeling.py (8)
BoundingPatterns(361-587)ModelingPrimitives(179-358)ModelingUtilities(101-176)bounds_with_state(399-442)expression_tracking_variable(183-219)state_transition_bounds(545-587)consecutive_duration_tracking(222-313)compute_consecutive_hours_in_state(103-125)flixopt/structure.py (14)
FlowSystemModel(82-200)Submodel(757-933)_do_modeling(931-933)add_variables(781-790)get_coords(139-168)get_coords(840-845)add_submodels(71-79)add_constraints(792-801)hours_per_step(132-133)hours_per_step(928-929)get(833-838)get(998-1000)values(991-992)variables(894-900)flixopt/components.py (3)
_do_modeling(715-732)_do_modeling(755-791)_do_modeling(802-865)flixopt/effects.py (4)
_do_modeling(233-272)_do_modeling(500-513)effects(411-412)add_share_to_effects(473-493)flixopt/elements.py (5)
_do_modeling(513-542)_do_modeling(745-767)_do_modeling(791-838)previous_states(721-734)previous_states(849-866)
flixopt/structure.py (2)
flixopt/core.py (4)
TimeSeriesData(41-149)get_dataarray_stats(430-454)is_timeseries_data(127-129)from_dataarray(112-124)flixopt/flow_system.py (7)
create_model(467-473)coords(743-749)fit_to_model_coords(346-395)FlowSystem(46-879)_create_reference_structure(189-227)to_dataset(229-241)from_dataset(244-297)
flixopt/components.py (4)
flixopt/features.py (8)
InvestmentModel(25-130)PiecewiseModel(348-428)_do_modeling(50-53)_do_modeling(162-228)_do_modeling(322-345)_do_modeling(379-428)_do_modeling(453-484)_do_modeling(519-543)flixopt/modeling.py (2)
BoundingPatterns(361-587)scaled_bounds(445-485)flixopt/structure.py (12)
FlowSystemModel(82-200)create_model(728-729)values(991-992)items(985-986)_do_modeling(931-933)add_constraints(792-801)add_submodels(71-79)add_variables(781-790)get_coords(139-168)get_coords(840-845)hours_per_step(132-133)hours_per_step(928-929)flixopt/flow_system.py (5)
create_model(467-473)FlowSystem(46-879)fit_to_model_coords(346-395)flows(734-736)coords(743-749)
flixopt/calculation.py (5)
flixopt/core.py (6)
DataConverter(161-427)TimeSeriesData(41-149)drop_constant_arrays(457-470)to_dataarray(329-392)is_timeseries_data(127-129)from_dataarray(112-124)flixopt/flow_system.py (10)
FlowSystem(46-879)used_in_calculation(752-753)sel(755-788)connect_and_transform(423-439)create_model(467-473)to_dataset(229-241)coords(743-749)from_dataset(244-297)_connect_network(620-652)flows(734-736)flixopt/structure.py (15)
FlowSystemModel(82-200)copy(687-699)solution(106-129)values(991-992)label_full(732-733)label_full(871-872)do_modeling(98-103)create_model(728-729)items(985-986)variables(894-900)to_dataset(494-517)weights(171-178)from_dataset(538-571)get(833-838)get(998-1000)flixopt/utils.py (1)
round_nested_floats(16-56)flixopt/aggregation.py (3)
do_modeling(310-338)Aggregation(41-229)cluster(83-107)
flixopt/effects.py (3)
flixopt/features.py (8)
ShareAllocationModel(487-588)_do_modeling(50-53)_do_modeling(162-228)_do_modeling(322-345)_do_modeling(379-428)_do_modeling(453-484)_do_modeling(519-543)add_share(545-588)flixopt/structure.py (20)
Element(710-754)ElementModel(1003-1024)FlowSystemModel(82-200)Submodel(757-933)label_full(732-733)label_full(871-872)create_model(728-729)_plausibility_checks(723-726)_do_modeling(931-933)add_submodels(71-79)add_variables(781-790)get_coords(139-168)get_coords(840-845)add_constraints(792-801)values(991-992)items(985-986)weights(171-178)add(994-996)get(833-838)get(998-1000)flixopt/flow_system.py (4)
fit_to_model_coords(346-395)fit_effects_to_model_coords(397-421)create_model(467-473)coords(743-749)
flixopt/elements.py (6)
flixopt/features.py (9)
InvestmentModel(25-130)OnOffModel(133-302)size(121-123)_do_modeling(50-53)_do_modeling(162-228)_do_modeling(322-345)_do_modeling(379-428)_do_modeling(453-484)_do_modeling(519-543)flixopt/modeling.py (9)
ModelingPrimitives(179-358)BoundingPatterns(361-587)ModelingUtilitiesAbstract(14-98)expression_tracking_variable(183-219)bounds_with_state(399-442)scaled_bounds(445-485)scaled_bounds_with_state(488-542)to_binary(18-52)mutual_exclusivity_constraint(316-358)flixopt/interface.py (11)
InvestParameters(663-932)OnOffParameters(936-1197)_plausibility_checks(911-924)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(876-909)transform_data(1141-1168)minimum_or_fixed_size(927-928)maximum_or_fixed_size(931-932)flixopt/structure.py (19)
Element(710-754)ElementModel(1003-1024)FlowSystemModel(82-200)create_model(728-729)_plausibility_checks(723-726)transform_data(221-230)label_full(732-733)label_full(871-872)_do_modeling(931-933)add_variables(781-790)get_coords(139-168)get_coords(840-845)hours_per_step(132-133)hours_per_step(928-929)add_submodels(71-79)results_structure(1019-1024)add_constraints(792-801)values(991-992)register_variable(803-811)flixopt/effects.py (9)
effects(411-412)create_model(217-220)create_model(303-306)_plausibility_checks(222-224)_plausibility_checks(358-371)transform_data(177-215)_do_modeling(233-272)_do_modeling(500-513)add_share_to_effects(473-493)flixopt/flow_system.py (5)
create_model(467-473)FlowSystem(46-879)fit_to_model_coords(346-395)fit_effects_to_model_coords(397-421)coords(743-749)
tests/test_effect.py (3)
tests/conftest.py (7)
assert_conequal(705-721)assert_sets_equal(761-779)assert_var_equal(724-758)create_calculation_and_solve(684-696)create_linopy_model(699-702)basic_flow_system_linopy_coords(645-660)coords_config(60-62)flixopt/results.py (8)
flow_system(290-307)variables(269-273)variables(838-846)constraints(276-280)constraints(849-857)size(1283-1291)effect_share_factors(283-287)effects_per_component(351-367)flixopt/structure.py (8)
variables(894-900)constraints(885-891)get_coords(139-168)get_coords(840-845)hours_per_step(132-133)hours_per_step(928-929)items(985-986)values(991-992)
flixopt/flow_system.py (4)
flixopt/core.py (5)
ConversionError(35-38)DataConverter(161-427)TimeSeriesData(41-149)fit_to_coords(86-101)to_dataarray(329-392)flixopt/effects.py (8)
effects(411-412)Effect(32-224)EffectCollection(290-460)create_effect_values_dict(319-356)transform_data(177-215)create_model(217-220)create_model(303-306)add_effects(308-317)flixopt/elements.py (8)
Flow(224-504)transform_data(100-105)transform_data(192-195)transform_data(420-451)create_model(95-98)create_model(187-190)create_model(415-418)label_full(493-494)flixopt/structure.py (20)
FlowSystemModel(82-200)Interface(203-707)weights(171-178)_create_reference_structure(232-281)to_dataset(494-517)from_dataset(538-571)get(833-838)get(998-1000)_resolve_dataarray_reference(366-397)_resolve_reference_structure(400-456)to_netcdf(519-535)get_structure(594-613)to_json(631-648)values(991-992)transform_data(221-230)create_model(728-729)label_full(732-733)label_full(871-872)keys(988-989)copy(687-699)
tests/conftest.py (3)
flixopt/structure.py (2)
FlowSystemModel(82-200)weights(171-178)flixopt/flow_system.py (2)
FlowSystem(46-879)add_elements(441-465)flixopt/core.py (3)
TimeSeriesData(41-149)aggregation_weight(108-109)aggregation_group(104-105)
tests/test_linear_converter.py (4)
tests/conftest.py (4)
basic_flow_system_linopy_coords(645-660)coords_config(60-62)assert_conequal(705-721)assert_var_equal(724-758)flixopt/elements.py (2)
flow_rate(621-623)Flow(224-504)flixopt/structure.py (7)
constraints(885-891)variables(894-900)hours_per_step(132-133)hours_per_step(928-929)get_coords(139-168)get_coords(840-845)add_variables(781-790)flixopt/flow_system.py (1)
coords(743-749)
flixopt/results.py (4)
flixopt/flow_system.py (8)
FlowSystem(46-879)to_dataset(229-241)flows(734-736)calculate_hours_per_timestep(171-176)from_dataset(244-297)_connect_network(620-652)coords(743-749)sel(755-788)flixopt/io.py (2)
load_dataset_from_netcdf(249-275)save_dataset_to_netcdf(198-246)flixopt/structure.py (9)
to_dataset(494-517)solution(106-129)items(985-986)get(833-838)get(998-1000)from_dataset(538-571)values(991-992)variables(894-900)copy(687-699)flixopt/effects.py (2)
effects(411-412)calculate_effect_share_factors(438-460)
flixopt/core.py (1)
flixopt/flow_system.py (1)
coords(743-749)
tests/test_flow.py (4)
tests/conftest.py (8)
assert_conequal(705-721)assert_sets_equal(761-779)assert_var_equal(724-758)create_linopy_model(699-702)basic_flow_system_linopy_coords(645-660)coords_config(60-62)costs(99-100)co2(103-104)flixopt/structure.py (4)
variables(894-900)get_coords(139-168)get_coords(840-845)constraints(885-891)flixopt/elements.py (2)
Flow(224-504)on_off(702-706)flixopt/flow_system.py (2)
coords(743-749)add_elements(441-465)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
55-55: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
60-60: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
61-61: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
65-65: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
66-66: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
117-117: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
117-117: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
119-119: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
120-120: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
121-121: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
| if 'flow_system' in kwargs and flow_system_data is None: | ||
| flow_system_data = kwargs.pop('flow_system') | ||
| warnings.warn( | ||
| "The 'flow_system' parameter is deprecated. Use 'flow_system_data' instead." | ||
| "Acess is now by '.flow_system_data', while '.flow_system' returns the restored FlowSystem.", | ||
| DeprecationWarning, | ||
| stacklevel=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.
Backward-compat bug: accept old flow_system (object) by converting to dataset
Current fallback assigns a FlowSystem object to flow_system_data, leading to failures later. Convert when needed.
- if 'flow_system' in kwargs and flow_system_data is None:
- flow_system_data = kwargs.pop('flow_system')
+ if 'flow_system' in kwargs and flow_system_data is None:
+ old = kwargs.pop('flow_system')
+ try:
+ from .flow_system import FlowSystem as _FS # local import to avoid cycles
+ except Exception:
+ _FS = None
+ if _FS is not None and isinstance(old, _FS):
+ flow_system_data = old.to_dataset()
+ elif isinstance(old, xr.Dataset):
+ flow_system_data = old
+ else:
+ raise TypeError(
+ "Deprecated 'flow_system' parameter must be a FlowSystem or an xr.Dataset. "
+ f'Got {type(old)}'
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if 'flow_system' in kwargs and flow_system_data is None: | |
| flow_system_data = kwargs.pop('flow_system') | |
| warnings.warn( | |
| "The 'flow_system' parameter is deprecated. Use 'flow_system_data' instead." | |
| "Acess is now by '.flow_system_data', while '.flow_system' returns the restored FlowSystem.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| if 'flow_system' in kwargs and flow_system_data is None: | |
| old = kwargs.pop('flow_system') | |
| try: | |
| from .flow_system import FlowSystem as _FS # local import to avoid cycles | |
| except Exception: | |
| _FS = None | |
| if _FS is not None and isinstance(old, _FS): | |
| flow_system_data = old.to_dataset() | |
| elif isinstance(old, xr.Dataset): | |
| flow_system_data = old | |
| else: | |
| raise TypeError( | |
| "Deprecated 'flow_system' parameter must be a FlowSystem or an xr.Dataset. " | |
| f'Got {type(old)}' | |
| ) | |
| warnings.warn( | |
| "The 'flow_system' parameter is deprecated. Use 'flow_system_data' instead." | |
| "Acess is now by '.flow_system_data', while '.flow_system' returns the restored FlowSystem.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) |
🤖 Prompt for AI Agents
In flixopt/results.py around lines 195 to 203, the code assigns a provided
deprecated 'flow_system' object directly to flow_system_data causing later
failures; update the branch so after popping 'flow_system' you detect if the
value is already a dataset-like object and, if not, convert the FlowSystem
object to the expected dataset by calling its conversion method (e.g. call
flow_system.to_dataset() or another available converter), then assign that
resulting dataset to flow_system_data while keeping the DeprecationWarning and
stacklevel intact.
* Remove need for timeseries with extra timestep * Simplify IO of FLowSystem * Remove parameter timesteps from IO * Improve Exceptions and Docstrings * Improve isel sel and resample methods * Change test * Bugfix * Improve * Improve * Add test for Storage Bounds * Add test for Storage Bounds * CHANGELOG.md * ruff check * Improve types * CHANGELOG.md * Bugfix in Storage * Revert changes in example_calculation_types.py * Revert changes in simple_example.py * Add convenient access to Elements in FlowSystem * Get Aggregated Calculation Working * Segmented running with wrong results * Use new persistent FLowSystem to create Calculations upfront * Improve SegmentedCalcualtion * Improve SegmentedCalcualtion * Fix SegmentedResults IO * ruff check * Update example * Updated logger essages to use .label_full instead of .label * Re-add parameters. Use deprecation warning instead * Update changelog * Improve warning message * Merge * Merge * Fit scenario weights to model coords when transforming * Merge * Removing logic between minimum, maximum and fixed size from InvestParameters * Remove selected_timesteps * Improve TypeHints * New property on InvestParameters for min/max/fixed size * Move logic for InvestParameters in Transmission to from Model to Interface * Make transformation of data more hierarchical (Flows after Components) * Add scenario validation * Change Transmission to have a "balanced" attribute. Change Tests accordingly * Improve index validations * rename method in tests * Update DataConverter * Add DataFrame Support back * Add copy() to DataConverter * Update fit_to_model_coords to take a list of coords * Make the DataConverter more universal by accepting a list of coords/dims * Update DataConverter for n-d arrays * Update DataConverter for n-d arrays * Add extra tests for 3-dims * Add FLowSystemDimension Type * Revert some logic about the fit_to_model coords * Adjust FLowSystem IO for scenarios * BUGFIX: Raise Exception instead of logging * Change usage of TimeSeriesData * Adjust logic to handle non scalars * Adjust logic to _resolve_dataarray_reference into separate method * Update IO of FlowSystem * Improve get_coords() * Adjust FlowSystem init for correct IO * Add scenario to sel and isel methods, and dont normalize scenario weights * Improve scenario_weights_handling * Add warning for not scaled weights * Update test_scenarios.py * Improve util method * Add objective to solution dataset. * Update handling of scenario_weights update tests * Ruff check. Fix type hints * Fix type hints and improve None handling * Fix coords in AggregatedCalculation * Improve Error Messages of DataConversion * Allow multi dim data conversion and broadcasting by length * Improve DataConverter to handle multi-dim arrays * Rename methods and remove unused code * Improve DataConverter by better splitting handling per datatype. Series only matches index (for one dim). Numpy matches shape * Add test for error handling * Update scenario example * Fix Handling of TimeSeriesData * Improve DataConverter * Fix resampling of the FlowSystem * Improve Warning Message * Add example that leverages resampling * Add example that leverages resampling adn fixing of Investments * Add flag to Calculation if its modeled * Make flag for connected_and_transformed FLowSystem public * Make Calcualtion Methods return themselfes to make them chainable * Improve example * Improve Unreleased CHANGELOG.md * Add year coord to FlowSystem * Improve dimension handling * Change plotting to use an indexer instead * Change plotting to use an indexer instead * Use tuples to set dimensions in Models * Bugfix in validation logic and test * Improve Errors * Improve weights handling and rescaling if None * Fix typehint * Update Broadcasting in Storage Bounds and improve type hints * Make .get_model_coords() return an actual xr.Coordinates Object * Improve get_coords() * Rename SystemModel to FlowSystemModel * First steps * Improve Feature Patterns * Improve acess to variables via short names * Improve * Add naming options to big_m_binary_bounds() * Fix and improve FLowModeling with Investment * Improve * Tyring to improve the Methods for bounding variables in different scenarios * Improve BoundingPatterns * Improve BoundingPatterns * Improve BoundingPatterns * Fix duration Modeling * Fix On + Size * Fix InvestmentModel * Fix Models * Update constraint names in test * Fix OnOffModel for multiple Flows * Update constraint names in tests * Simplify * Improve handling of vars/cons and models * Revising the basic structure of a class Model * Revising the basic structure of a class Model * Simplify and focus more on own Model class * Update tests * Improve state computation in ModelingUtilities * Improve handling of previous flowrates * Imropove repr and submodel acess * Update access pattern in tests * Fix PiecewiseEffects and StorageModel * Fix StorageModel and Remove PreventSimultaniousUseModel * Fix Aggregation and SegmentedCalculation * Update tests * Loosen precision in tests * Update test_on_hours_computation.py and some types * Rename class Model to Submodel * rename sub_model to submodel everywhere * rename self.model to self.submodel everywhere * Rename .model with .submodel if its only a submodel * Rename .sub_models with .submodels * Improve repr * Improve repr * Include def do_modeling() into __init__() of models * Make properties private * Improve Inheritance of Models * V3.0.0/plotting (#285) * Use indexer to reliably plot solutions with and wihtout scenarios/years * ruff check * Improve typehints * Update CHANGELOG.md * Bugfix from renaming to .submodel * Bugfix from renaming to .submodel * Improve indexer in results plotting * rename register_submodel() to .add_submodels() adn add SUbmodels collection class * Add nice repr to FlowSystemModel and Submodel * Bugfix .variables and .constraints * Add type checks to modeling.py * Improve assertion in tests * Improve docstrings and register ElementModels directly in FlowSystemModel * Improve __repr__() * ruff check * Use new method to compare sets in tests * ruff check * Update Contribute.md, some dependencies and add pre-commit * Pre commit hook * Run Pre-Commit Hook for the first time * Fix link in README.md * Update Effect name in tests to be 'costs' instead of 'Costs' Everywhere Simplify testing by creating a Element Library * Improve some of the modeling and coord handling * Add tests with years and scenarios * Update tests to run with multiple coords * Fix Effects dataset computation in case of empty effects * Update Test for multiple dims Fix Dim order in scaled_bounds_with_state Bugfix logic in .use_switch_on * Fix test with multiple dims * Fix test with multiple dims * New test * New test for previous flow_rates * Add Model for YearAwareInvestments * Add FlowSystem.years_per_year attribute and "years_of_last_year" parameter to FlowSystem() * Add YearAwareInvestmentModel * Add new Interface * Improve YearAwareInvestmentModel * Rename and improve * Move piecewise_effects * COmbine TImingInvestment into a single interface * Add model tests for investment * Add size_changes variables * Add size_changes variables * Improve InvestmentModel * Improve InvestmentModel * Rename parameters * remove old code * Add a duration_in_years to the InvestTimingParameters * Improve handling of fixed_duration * Improve validation and make Investment/divestment optional by default * Rename some vars and improve previous handling * Add validation for previous size * Change fit_to_model_coords to work with a Collection of dims * Improve fit_to_model_coords * Improve test * Update transform_data() * Add new "year of investment" coord to FlowSystem * Add 'year_of_investment' dimension to FlowSystem * Improve InvestmentTiming * Improve InvestmentTiming * Add specific_effect back * add effects_by_investment_year back * Add year_of_investment to FLowSystem.sel() * Improve Interface * Handle selection of years properly again * Temp * Make ModelingPrimitives.consecutive_duration_tracking() dim-agnostic * Use new lifetime variable and constraining methods * Improve Plausibility check * Improve InvestmentTImingParameters * Improve weights * Adjust test * Remove old classes * V3.0.0/main fit to model coords improve (#295) * Change fit_to_model_coords to work with a Collection of dims * Improve fit_to_model_coords * ruff format * Revert changes * Update type hints * Revert changes introduced by new Multiperiod Invest parameters * Improve CHnagelog and docstring of Storage * Improve Changelog * Improve InvestmentModel * Improve InvestmentModel to have 2 cases. One without years and one with * Improve InvestmentModel to have 2 cases. One without years and one with years. Further, remove investment_scenarios parameter * Revert some changes regarding Investments * Typo * Remove Investment test file (only local testing) * More reverted changes * More reverted changes * Add years_of_last_year to docstring * Revert change from Investment * Revert change from Investment * Remove old todos.txt file * Fix typos in CHANGELOG.md * Improve usage of name_prefix to intelligently join with the label * Ensure IO of years_of_last_year * Typo * Typo
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: 4
🧹 Nitpick comments (15)
CHANGELOG.md (5)
38-70: Fix heading levels and list indentation to satisfy markdownlint (MD001/MD007).
- Use H3 (###) directly under “## [Unreleased]” instead of H4 (####).
- Fix nested list indentation to 2 spaces.
Example fixes:
-#### Multi-year-investments +### Multi-year investments @@ -#### Stochastic modeling +### Stochastic modeling @@ -#### Improved Data handling: IO, resampling and more through xarray +### Improved Data handling: I/O, resampling and more through xarray @@ - * NetCDF export/import capabilities for all Interface objects and FlowSystem + * NetCDF export/import capabilities for all Interface objects and FlowSystem - * JSON export for documentation purposes + * JSON export for documentation purposes
33-35: Polish release intro phrasing.-This Release brings Multi-year-investments and stochastic modeling to flixopt. -Further, IO methods were improved and resampling and selection of parts of the FlowSystem is now possible. +This release brings multi-year investments and stochastic modeling to flixopt. +Furthermore, I/O methods were improved, and resampling and selection of parts of the FlowSystem are now possible.
72-79: Fix typos and clarity in Added section.-* FlowSystem Restoring: The used FlowSystem is now accessible directly form the results without manual restoring (lazily). All Parameters can be safely accessed anytime after the solve. +* FlowSystem restoring: The used FlowSystem is now accessible directly from the results without manual restoring (lazily). All parameters can be safely accessed anytime after the solve.
86-88: Grammar fix (“cannot”).-* FlowSystems can not be shared across multiple Calculations anymore. A copy of the FlowSystem is created instead, making every Calculation independent +* FlowSystems cannot be shared across multiple Calculations anymore. A copy of the FlowSystem is created instead, making every Calculation independent
119-119: Avoid MD050 false positives by formatting dunder names as code.-* Added __repr__() for Submodels to easily inspect its content +* Added `__repr__()` for Submodels to easily inspect its contentflixopt/interface.py (1)
1169-1179: Fix typo in docstring (“whether”).- """Determines wether a Variable for SWITCH-ON is needed or not""" + """Determines whether a variable for SWITCH-ON is needed or not"""flixopt/flow_system.py (4)
79-83: Use validated timesteps when creating timesteps_extra.Avoid bypassing name/ordering validations by using self.timesteps.
- self.timesteps_extra = self._create_timesteps_with_extra(timesteps, hours_of_last_timestep) + self.timesteps_extra = self._create_timesteps_with_extra(self.timesteps, hours_of_last_timestep)
434-441: Typo in warning (“recommended”).- f'Scenario weights are not normalized to 1. This is recomended for a better scaled model. ' + f'Scenario weights are not normalized to 1. This is recommended for a better scaled model. '
472-479: Align attribute naming for created model (model vs submodel).create_model assigns to self.submodel while the class defines self.model earlier; consider setting both for consistency.
- self.submodel = FlowSystemModel(self) - return self.submodel + self.submodel = FlowSystemModel(self) + self.model = self.submodel + return self.submodel
560-563: Grammar nit (“Can't”).- logger.warning('No network app is currently running. Cant stop it') + logger.warning("No network app is currently running. Can't stop it")flixopt/elements.py (1)
487-489: Typo in error message.- f'Different values in different years or scenarios are not yetsupported.' + f'Different values in different years or scenarios are not yet supported.'flixopt/components.py (2)
202-205: Correct and clarify warning text.The message contradicts the condition (variable size) and has typos.
- logger.warning( - f'Using a FLow with a fixed size ({flow.label_full}) AND a piecewise_conversion ' - f'(in {self.label_full}) and variable size is uncommon. Please check if this is intended!' - ) + logger.warning( + f'Using a Flow with variable size (InvestParameters without fixed_size) ' + f'and a piecewise_conversion in {self.label_full} is uncommon. Please verify intent ' + f'({flow.label_full}).' + )
151-154: Fix typos in user-facing Note.“doest” → “doesn’t”; “THis” → “This”. Also clarify example directionality.
- Conversion factors define linear relationships. - `{flow1: a1, flow2: a2, ...}` leads to `a1×flow_rate1 + a2×flow_rate2 + ... = 0` - Unfortunately the current input format doest read intuitively: - {"electricity": 1, "H2": 50} means that the electricity_in flow rate is multiplied by 1 - and the hydrogen_out flow rate is multiplied by 50. THis leads to 50 electricity --> 1 H2. + Conversion factors define linear relationships: + `{flow1: a1, flow2: a2, ...}` yields `a1×flow_rate1 + a2×flow_rate2 + ... = 0`. + Note: The input format may be unintuitive. For example, + `{"electricity": 1, "H2": 50}` implies `1×electricity + 50×H2 = 0`, + i.e., 50 units of electricity produce 1 unit of H2.flixopt/effects.py (2)
320-336: Update docstring examples to reflect label keys (not None) and standard-effect requirement.Examples still show
{None: 20}, but implementation returns{standard_effect_label: 20}and requires a standard effect.- effect_values_user = 20 -> {None: 20} + effect_values_user = 20 -> {'<standard_effect_label>': 20} @@ - A dictionary with None or Effect as the key, or None if input is None. + A dictionary keyed by effect label, or None if input is None. + Note: a standard effect must be defined when passing scalars or None labels.
534-603: BFS queue should use deque; avoid O(n²) pops.Using
list.pop(0)is quadratic. Switch tocollections.dequefor efficiency on large graphs. Behavior unchanged.+ from collections import deque @@ - queue = [(origin, 1, [origin])] + queue = deque([(origin, 1, [origin])]) @@ - while queue: - current_domain, factor, path = queue.pop(0) + while queue: + current_domain, factor, path = queue.popleft() @@ - queue.append((target, indirect_factor, new_path)) + queue.append((target, indirect_factor, new_path))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CHANGELOG.md(1 hunks)flixopt/components.py(13 hunks)flixopt/effects.py(10 hunks)flixopt/elements.py(8 hunks)flixopt/features.py(7 hunks)flixopt/flow_system.py(10 hunks)flixopt/interface.py(10 hunks)flixopt/modeling.py(1 hunks)flixopt/structure.py(6 hunks)tests/todos.txt(0 hunks)
💤 Files with no reviewable changes (1)
- tests/todos.txt
🧰 Additional context used
🧬 Code graph analysis (8)
flixopt/flow_system.py (4)
flixopt/core.py (5)
ConversionError(35-38)DataConverter(161-427)TimeSeriesData(41-149)fit_to_coords(86-101)to_dataarray(329-392)flixopt/effects.py (5)
effects(412-413)Effect(32-225)EffectCollection(291-461)create_effect_values_dict(320-357)transform_data(177-216)flixopt/elements.py (7)
Bus(120-209)Component(31-116)Flow(226-503)transform_data(100-106)transform_data(193-197)transform_data(422-450)label_full(492-493)flixopt/structure.py (16)
Element(715-759)FlowSystemModel(82-204)Interface(207-712)weights(171-182)_create_reference_structure(237-286)items(990-991)to_dataset(499-522)from_dataset(543-576)get(838-843)get(1003-1005)_resolve_dataarray_reference(371-402)_resolve_reference_structure(405-461)values(996-997)transform_data(225-235)label_full(737-738)label_full(876-877)
flixopt/modeling.py (4)
flixopt/config.py (1)
CONFIG(98-153)flixopt/structure.py (12)
Submodel(762-938)values(996-997)hours_per_step(132-133)hours_per_step(933-934)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_constraints(797-806)constraints(890-896)variables(899-905)get(838-843)get(1003-1005)flixopt/flow_system.py (1)
coords(748-754)flixopt/elements.py (2)
previous_states(720-733)previous_states(848-865)
flixopt/interface.py (5)
flixopt/effects.py (2)
effects(412-413)transform_data(177-216)flixopt/components.py (3)
transform_data(207-214)transform_data(426-467)transform_data(698-702)flixopt/elements.py (3)
transform_data(100-106)transform_data(193-197)transform_data(422-450)flixopt/flow_system.py (3)
FlowSystem(46-888)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)tests/conftest.py (1)
piecewise(203-220)
flixopt/structure.py (6)
flixopt/core.py (4)
TimeSeriesData(41-149)get_dataarray_stats(430-454)is_timeseries_data(127-129)from_dataarray(112-124)flixopt/effects.py (6)
effects(412-413)create_model(218-221)create_model(304-307)transform_data(177-216)_plausibility_checks(223-225)_plausibility_checks(359-372)flixopt/components.py (9)
create_model(173-176)create_model(421-424)create_model(693-696)transform_data(207-214)transform_data(426-467)transform_data(698-702)_plausibility_checks(178-205)_plausibility_checks(469-520)_plausibility_checks(667-691)flixopt/elements.py (9)
create_model(95-98)create_model(188-191)create_model(417-420)transform_data(100-106)transform_data(193-197)transform_data(422-450)_plausibility_checks(115-116)_plausibility_checks(199-205)_plausibility_checks(452-489)flixopt/flow_system.py (8)
create_model(472-478)coords(748-754)fit_to_model_coords(364-402)FlowSystem(46-888)_create_reference_structure(206-244)to_dataset(246-258)to_netcdf(317-331)from_dataset(261-315)flixopt/io.py (4)
update(322-330)save_dataset_to_netcdf(198-246)load_dataset_from_netcdf(249-275)remove_none_and_empty(20-34)
flixopt/features.py (5)
flixopt/modeling.py (8)
BoundingPatterns(366-735)ModelingPrimitives(179-363)ModelingUtilities(101-176)bounds_with_state(404-447)expression_tracking_variable(183-219)state_transition_bounds(550-592)consecutive_duration_tracking(222-318)compute_consecutive_hours_in_state(103-125)flixopt/structure.py (15)
FlowSystemModel(82-204)Submodel(762-938)_do_modeling(936-938)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_submodels(71-79)items(990-991)add_constraints(797-806)hours_per_step(132-133)hours_per_step(933-934)get(838-843)get(1003-1005)values(996-997)variables(899-905)flixopt/interface.py (10)
InvestParameters(663-914)OnOffParameters(918-1179)Piecewise(83-224)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)items(435-442)use_off(1153-1155)use_switch_on(1168-1179)use_consecutive_on_hours(1158-1160)use_consecutive_off_hours(1163-1165)flixopt/components.py (3)
_do_modeling(716-733)_do_modeling(756-792)_do_modeling(803-866)flixopt/effects.py (4)
_do_modeling(234-273)_do_modeling(501-514)effects(412-413)add_share_to_effects(474-494)
flixopt/components.py (7)
flixopt/core.py (1)
PlausibilityError(29-32)flixopt/elements.py (17)
Component(31-116)ComponentModel(783-865)Flow(226-503)create_model(95-98)create_model(188-191)create_model(417-420)_plausibility_checks(115-116)_plausibility_checks(199-205)_plausibility_checks(452-489)label_full(492-493)transform_data(100-106)transform_data(193-197)transform_data(422-450)_do_modeling(512-541)_do_modeling(744-766)_do_modeling(790-837)on_off(701-705)flixopt/features.py (9)
InvestmentModel(24-130)PiecewiseModel(352-432)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/interface.py (17)
InvestParameters(663-914)OnOffParameters(918-1179)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(874-906)transform_data(1123-1150)has_time_dim(198-199)has_time_dim(202-205)has_time_dim(426-427)has_time_dim(430-433)has_time_dim(646-647)has_time_dim(650-654)items(435-442)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)flixopt/modeling.py (2)
BoundingPatterns(366-735)scaled_bounds(450-490)flixopt/structure.py (14)
FlowSystemModel(82-204)create_model(733-734)_plausibility_checks(728-731)label_full(737-738)label_full(876-877)transform_data(225-235)values(996-997)items(990-991)_do_modeling(936-938)add_constraints(797-806)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)flixopt/flow_system.py (5)
create_model(472-478)FlowSystem(46-888)fit_to_model_coords(364-402)flows(739-741)coords(748-754)
flixopt/elements.py (6)
flixopt/features.py (9)
InvestmentModel(24-130)OnOffModel(133-306)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/modeling.py (9)
ModelingPrimitives(179-363)BoundingPatterns(366-735)ModelingUtilitiesAbstract(14-98)expression_tracking_variable(183-219)bounds_with_state(404-447)scaled_bounds(450-490)scaled_bounds_with_state(493-547)to_binary(18-52)mutual_exclusivity_constraint(321-363)flixopt/interface.py (10)
InvestParameters(663-914)OnOffParameters(918-1179)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(874-906)transform_data(1123-1150)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)flixopt/structure.py (18)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)create_model(733-734)transform_data(225-235)label_full(737-738)label_full(876-877)_do_modeling(936-938)add_variables(786-795)get_coords(139-168)get_coords(845-850)hours_per_step(132-133)hours_per_step(933-934)add_submodels(71-79)results_structure(1024-1029)add_constraints(797-806)values(996-997)register_variable(808-816)flixopt/effects.py (8)
effects(412-413)create_model(218-221)create_model(304-307)transform_data(177-216)_do_modeling(234-273)_do_modeling(501-514)add_share_to_effects(474-494)add_share_to_penalty(496-499)flixopt/flow_system.py (5)
create_model(472-478)FlowSystem(46-888)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)coords(748-754)
flixopt/effects.py (3)
flixopt/features.py (8)
ShareAllocationModel(491-592)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)add_share(549-592)flixopt/structure.py (20)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)Submodel(762-938)label_full(737-738)label_full(876-877)create_model(733-734)_plausibility_checks(728-731)_do_modeling(936-938)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_constraints(797-806)values(996-997)items(990-991)weights(171-182)add(999-1001)get(838-843)get(1003-1005)flixopt/flow_system.py (5)
FlowSystem(46-888)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)create_model(472-478)coords(748-754)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
38-38: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
58-58: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
61-61: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
63-63: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
67-67: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
119-119: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
119-119: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
121-121: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
122-122: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
123-123: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
🔇 Additional comments (23)
flixopt/interface.py (3)
71-80: Piece dims logic looks sound.Using dims=None when has_time_dim=True and restricting to ['year', 'scenario'] otherwise keeps time-invariant piece bounds non-temporal. Good.
222-225: Propagate has_time_dim from Piecewise to pieces.Setter correctly cascades to contained pieces; prevents mismatched transforms later. Nice.
874-896: Good: InvestParameters effects/sizes aligned to ['year','scenario'].Non-temporal investment values map to planning axes; piecewise_effects correctly set to non-time dims.
flixopt/modeling.py (6)
98-126: Type-check hours_per_timestep and reuse epsilon default.Consider early-return on invalid hours_per_timestep types (already present) and pass epsilon to count_consecutive_states consistently. Current usage is fine after to_binary/count fixes.
74-99: Fix logic in count_consecutive_states (spurious .all(), DataArray vs ndarray).
.all()on a bool is invalid.- Use ndarray ops consistently.
- binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim]) + if binary_values.ndim > 1: + binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim]) @@ - if np.isclose(binary_values.isel({dim: -1}).item(), 0, atol=epsilon).all(): + if np.isclose(binary_values.isel({dim: -1}).item(), 0.0, atol=epsilon): return 0.0 @@ - is_zero = np.isclose(binary_values, 0, atol=epsilon) + arr = binary_values.values + is_zero = np.isclose(arr, 0.0, atol=epsilon) @@ - zero_indices = np.where(is_zero)[0] + zero_indices = np.where(is_zero)[0] @@ - consecutive_values = binary_values.isel({dim: slice(start_idx, None)}) - - return float(consecutive_values.sum().item()) # TODO: Som only over one dim? + return float(arr[start_idx:].sum())
310-315: Avoid ambiguous truth-value with xr.DataArray in consecutive_duration_tracking.Comparing with an xr.DataArray inside a Python “and” raises. Convert to scalars first.
- if previous_duration > 0 and previous_duration < minimum_duration.isel({duration_dim: 0}).max(): + prev = float(previous_duration) + min0 = float(minimum_duration.isel({duration_dim: 0}).max().item()) + if prev > 0 and prev < min0: constraints['initial_lb'] = model.add_constraints( state_variable.isel({duration_dim: 0}) == 1, name=f'{duration.name}|initial_lb' )
438-446: Correct equality detection in bounds_with_state.
np.all(lower - upper) < ...compares a boolean to float.- if np.all(lower_bound - upper_bound) < 1e-10: + if np.all(np.isclose(lower_bound, upper_bound, atol=1e-10)): fix_constraint = model.add_constraints(variable == variable_state * upper_bound, name=f'{name}|fix') return [fix_constraint]
484-486: Correct equality detection in scaled_bounds.- if np.abs(rel_lower - rel_upper).all() < 10e-10: + if np.all(np.isclose(rel_lower, rel_upper, atol=1e-10)): return [model.add_constraints(variable == scaling_variable * rel_lower, name=f'{name}|fixed')]
34-52: Handle scalars/empty arrays in to_binary and guard reduction dims.Current code errors on scalars (len(values)) and on empty .item() access; also collapse step should guard dims.
- if not isinstance(values, xr.DataArray): - values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))}) + if not isinstance(values, xr.DataArray): + if np.isscalar(values): + values = xr.DataArray(values) + else: + values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))}) @@ - if values.size == 0: - return xr.DataArray(0) if values.item() < epsilon else xr.DataArray(1) + if values.size == 0: + return xr.zeros_like(values, dtype=int) @@ - binary_states = binary_states.any(dim=[d for d in binary_states.dims if d not in dims]) + reduce_dims = [d for d in binary_states.dims if d not in dims] + if reduce_dims: + binary_states = binary_states.any(dim=reduce_dims)flixopt/flow_system.py (1)
659-667: Nice: concise repr with connection status.Helpful for quick diagnostics.
flixopt/elements.py (2)
568-607: Constraint composition is correct across on/off/investment cases.Good use of BoundingPatterns and Submodel wiring.
472-478: Fix .any() on boolean scalar (use np.any).
.any()on a Python bool raises when relative_minimum is scalar.- if (self.relative_minimum > 0).any() and self.on_off_parameters is None: + if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:flixopt/features.py (2)
64-77: Investment sizing/binary bounds wiring looks solid.
bounds_with_stateon (min,max) with is_invested cleanly enforces optional investments.
165-204: On/off tracking and switch counting: good use of primitives.State transition bounds and switch-on totals are consistent; previous state handling is correct.
flixopt/structure.py (2)
171-183: Weights defaulting and normalization are sensible.Good fallback to years_per_year and normalization.
655-676: Robust repr for Interface; truncation and fallbacks are helpful.Nice debugging ergonomics.
flixopt/components.py (3)
165-171: LinearConverter temporal data handling looks good.Type changes to TemporalDataUser and the transform path via fit_to_model_coords are consistent with model-coords. The conversion_factors transformation to xr.DataArray per-flow is appropriate.
Please confirm all callers updated to pass time-aligned data for conversion_factors and that piecewise_conversion.has_time_dim=True is intended for this component.
Also applies to: 207-215, 216-229
832-838: Fix discharge efficiency direction in storage balance (still incorrect).Docstring uses “- discharge / eta_discharge”, but code multiplies by eta_discharge, overstating discharge. Replace multiply with divide.
- + charge_rate * eff_charge * hours_per_step - - discharge_rate * eff_discharge * hours_per_step, + + charge_rate * eff_charge * hours_per_step + - discharge_rate * hours_per_step / eff_discharge,
728-733: ._investment is a deprecated alias — not a runtime bug; prefer .investment.
._investment is defined (returns self.investment) in elements.py and components.py, so these calls won't raise at runtime; still replace usages in flixopt/components.py (lines ~728–733 and ~861–867) with .investment for clarity.Likely an incorrect or invalid review comment.
flixopt/effects.py (4)
177-217: Effect.transform_data aligns with model coords.Converts temporal scalars/ranges and effect-share dicts using fit_to_model_coords/fit_effects_to_model_coords with correct dims. Looks solid.
231-274: Submodel decomposition for EffectModel is clean.Invest/operation shares separated with appropriate dims and bounds; total variable aggregates correctly.
496-500: Validatelinopy.LinearExpressionactually exposes.ndim.If
.ndimisn’t guaranteed on expressions, this TypeError guard could itself raise. Consider using a safer check (e.g.,hasattr(expression, "ndim")) or validating shapes at construction time.
439-462: Cycle detection and share-factor expansion: logic reads correctly.Computes (origin,target) shares and validates cycles pre-modeling; good guardrails for misconfigured cross-effects.
Also applies to: 361-373
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: 0
🧹 Nitpick comments (7)
.github/workflows/python-app.yaml (7)
8-9: Clarify PR branch filter:pull_request.branchesmatches base (target) branch, not the source.Including
feature/v3here only runs CI for PRs targeting that branch. If the intent was to ensure PRs from thefeature/v3/*source branches run when targetingmain/dev, it’s redundant—those are already covered bymain/devas bases. Simplify, or add head-branch gating at job-level if needed.Option A (simplify):
- pull_request: - branches: [main, dev, feature/v3] + pull_request: + branches: [main, dev]Option B (gate certain jobs by source branch):
test: runs-on: ubuntu-24.04 + if: ${{ github.event_name != 'pull_request' || startsWith(github.head_ref, 'feature/') }}Also applies to: 3-14
4-6: Consider running CI on pushes todevas well.Push triggers are limited to
main. Ifdevis an integration branch, enable push CI there too.on: push: - branches: [main] # Only main branch + branches: [main, dev]
152-157: Guardextract_release_notes.pyusage; provide fallback.If the script is missing or errors, the release job fails before creating the release.
- - name: Extract release notes - run: | - VERSION=${GITHUB_REF#refs/tags/v} - echo "Extracting release notes for version: $VERSION" - python scripts/extract_release_notes.py $VERSION > current_release_notes.md + - name: Extract release notes (with fallback) + run: | + set -euo pipefail + VERSION=${GITHUB_REF#refs/tags/v} + echo "Extracting release notes for version: $VERSION" + if [ -f scripts/extract_release_notes.py ]; then + python scripts/extract_release_notes.py "$VERSION" > current_release_notes.md || true + fi + if [ ! -s current_release_notes.md ]; then + echo "Using autogenerated release notes as fallback." > current_release_notes.md + fi
202-209: Adopt PyPI Trusted Publishing (OIDC) to remove API tokens.Replace Twine + long‑lived tokens with
pypa/gh-action-pypi-publish. Improves security and reduces secret management.Add OIDC permissions:
publish-pypi: name: Publish to PyPI runs-on: ubuntu-24.04 - needs: [publish-testpypi] + needs: [publish-testpypi] if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v') environment: name: pypi url: https://pypi.org/p/flixopt + permissions: + id-token: writeReplace upload steps:
- - name: Upload to TestPyPI - run: | - twine upload --repository-url https://test.pypi.org/legacy/ dist/* --verbose --skip-existing - env: - TWINE_USERNAME: __token__ - TWINE_PASSWORD: ${{ secrets.TEST_PYPI_API_TOKEN }} - TWINE_NON_INTERACTIVE: "1" + - name: Publish to TestPyPI (OIDC) + uses: pypa/gh-action-pypi-publish@release/v1 + with: + repository-url: https://test.pypi.org/legacy/ + skip-existing: true- - name: Upload to PyPI - run: | - twine upload dist/* --verbose --skip-existing - env: - TWINE_USERNAME: __token__ - TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }} - TWINE_NON_INTERACTIVE: "1" + - name: Publish to PyPI (OIDC) + uses: pypa/gh-action-pypi-publish@release/v1 + with: + skip-existing: trueNote: Configure PyPI/TestPyPI trusted publishers for this repo before merging.
Also applies to: 288-295, 256-264, 168-178, 130-133
169-178: Verify presence of required secrets or switch to OIDC.If you stay with Twine, ensure
TEST_PYPI_API_TOKENexists, else job will fail. OIDC removes this need.Also applies to: 202-209
112-118: Add dependency vulnerability scan (supply chain).Complement Bandit (code) with dependency audit via
pip-auditorosv-scanner.- name: Run Bandit security scan run: | # Gate on HIGH severity & MEDIUM confidence; produce JSON artifact uvx bandit -r . -x tests,docs -c pyproject.toml -f json -o bandit-report.json -q --severity-level high --confidence-level medium # Human-readable output without affecting job status uvx bandit -r flixopt/ -c pyproject.toml -q --exit-zero + + - name: Run dependency vulnerability audit + run: | + uvx pip-audit -r requirements.txt || true + uvx pip-audit -s --generate SBOM:cyclonedx-json > sbom.json || true + + - name: Upload SBOM + uses: actions/upload-artifact@v4 + if: always() + with: + name: sbom + path: sbom.json + retention-days: 30
33-43: Pin GitHub Actions to commit SHAs for supply-chain hardening.Using floating major tags (e.g.,
@v5,@v6) can be replaced by digest-pinned SHAs to reduce risk.Also applies to: 75-83, 101-111, 136-144, 141-146, 170-178, 183-191, 206-209, 265-273, 269-274, 355-363
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/python-app.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (1)
.github/workflows/python-app.yaml (1)
112-118: No change required — flixopt/ exists at repo rootpyproject and the repo contain flixopt/, so
uvx bandit -r flixopt/will scan the package.
* Fix typo * Prefer robust scalar extraction for timestep sizes in aggregation * Improve docs and error messages * Update examples * Use validated timesteps * Remove unnessesary import * Use FlowSystem.model instead of FlowSystem.submodel * Fix Error message * Improve CHANGELOG.md * Use self.standard_effect instead of provate self._standard_effect and update docstring * in calculate_all_conversion_paths, use `collections.deque` for efficiency on large graphs * Make aggregation_parameters.hours_per_period more robust by using rounding * Improve import and typos * Improve docstring * Use validated timesteps * Improve error * Improve warning * Improve type hint * Improve CHANGELOG.md: typos, wording and duplicate entries
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
🧹 Nitpick comments (13)
CHANGELOG.md (1)
55-69: Fix MD007 list indentation (2 spaces for nested bullets).markdownlint flags inconsistent indentation in nested lists. Normalize nested bullets to two spaces.
Apply:
- * NetCDF export/import capabilities for all Interface objects and FlowSystem - * JSON export for documentation purposes - * Recursive handling of nested Interface objects + * NetCDF export/import capabilities for all Interface objects and FlowSystem + * JSON export for documentation purposes + * Recursive handling of nested Interface objects - * `sel()` and `isel()` methods for temporal data selection - * `resample()` method for temporal resampling - * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data - * `__eq__()` method for FlowSystem comparison + * `sel()` and `isel()` methods for temporal data selection + * `resample()` method for temporal resampling + * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data + * `__eq__()` method for FlowSystem comparison - * `get_dataarray_stats()` function for statistical summaries - * Enhanced `DataConverter` class with better TimeSeriesData support + * `get_dataarray_stats()` function for statistical summaries + * Enhanced `DataConverter` class with better TimeSeriesData support - * `fit_to_model_coords()` method for data alignment - * `fit_effects_to_model_coords()` method for effect data processing - * `connect_and_transform()` method replacing several operations + * `fit_to_model_coords()` method for data alignment + * `fit_effects_to_model_coords()` method for effect data processing + * `connect_and_transform()` method replacing several operationsAlso applies to: 119-121
flixopt/elements.py (1)
600-606: Preferself.investmentover_investmentfor clarity.Avoid private alias; use the public property consistently.
Apply:
- scaling_variable=self._investment.size, + scaling_variable=self.investment.size,flixopt/results.py (1)
390-399: Guard empty flows when assembling flow_rates.Older results may lack flows; avoid xr.concat([]) crash and provide a clear error.
Apply:
- if self._flow_rates is None: + if self._flow_rates is None: + if not self.flows: + raise ValueError('No flow metadata available in results (pre-v2.2.0).') self._flow_rates = self._assign_flow_coords(flixopt/calculation.py (4)
75-85: Deprecation path OK; adjust active_timesteps typing/behavioractive_timesteps can be None; property type hints and return should reflect Optional to avoid misleading annotations.
- def active_timesteps(self) -> pd.DatetimeIndex: + def active_timesteps(self) -> pd.DatetimeIndex | None:Also applies to: 160-168
99-100: Remove unused _modeled flagThe modeled property derives from self.model; _modeled is dead state and can be removed.
- self._modeled = False
105-146: Guard main_results access when model/solutions missingmain_results assumes a solved model. Consider a quick guard to raise a clear error if self.model is None or unsolved to prevent AttributeError in logging paths.
559-564: Optional: Python version dependency (zip(..., strict=True))strict=True requires Python 3.10+. If older versions are supported, drop strict or guard by version.
What Python versions do we support in this project?
flixopt/components.py (2)
642-647: Type hint: on_off_parameters should be OptionalSignature shows default None; expand annotation for clarity.
- on_off_parameters: OnOffParameters = None, + on_off_parameters: OnOffParameters | None = None,
680-692: Message consistency for balanced TransmissionYou compare *_or_fixed_size; align error message fields accordingly.
- f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and ' - f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.' + f'Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and ' + f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.'flixopt/effects.py (2)
236-264: Dims type vs value: pass list to ShareAllocationModelShareAllocationModel expects list-like dims; you pass tuples here. Convert to list for consistency.
- ShareAllocationModel( - model=self._model, - dims=('year', 'scenario'), + ShareAllocationModel( + model=self._model, + dims=['year', 'scenario'], @@ - ShareAllocationModel( - model=self._model, - dims=('time', 'year', 'scenario'), + ShareAllocationModel( + model=self._model, + dims=['time', 'year', 'scenario'],
541-610: Conversion-path calculation: watch path explosionBFS accumulates multiple path factors; for dense graphs this can blow up. Consider limiting path length or deduplicating equivalent paths later if this becomes hot.
If performance becomes an issue, profile calculate_all_conversion_paths on representative graphs.
flixopt/flow_system.py (2)
521-543: Minor: use connect_and_transform for network app tooFor consistency, prefer connect_and_transform over private _connect_network here unless avoiding data transforms is intentional.
762-799: Enforce keyword-only args in FlowSystem.sel
- Add to the sel docstring: “Positional arguments aren’t supported—use
time=,year=orscenario=.”- In flixopt/calculation.py:550 change
self.flow_system.sel(timesteps_of_segment)to
self.flow_system.sel(time=timesteps_of_segment)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md(2 hunks)examples/03_Calculation_types/example_calculation_types.py(5 hunks)examples/05_Two-stage-optimization/two_stage_optimization.py(1 hunks)flixopt/calculation.py(12 hunks)flixopt/components.py(14 hunks)flixopt/effects.py(9 hunks)flixopt/elements.py(8 hunks)flixopt/flow_system.py(11 hunks)flixopt/interface.py(10 hunks)flixopt/modeling.py(1 hunks)flixopt/results.py(30 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/05_Two-stage-optimization/two_stage_optimization.py
🧰 Additional context used
🧬 Code graph analysis (8)
examples/03_Calculation_types/example_calculation_types.py (3)
flixopt/core.py (3)
TimeSeriesData(41-149)aggregation_weight(108-109)aggregation_group(104-105)flixopt/solvers.py (1)
HighsSolver(61-81)flixopt/plotting.py (1)
with_plotly(328-466)
flixopt/modeling.py (4)
flixopt/config.py (1)
CONFIG(98-153)flixopt/structure.py (12)
Submodel(762-938)values(996-997)hours_per_step(132-133)hours_per_step(933-934)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_constraints(797-806)constraints(890-896)variables(899-905)get(838-843)get(1003-1005)flixopt/flow_system.py (1)
coords(750-756)flixopt/elements.py (2)
previous_states(720-733)previous_states(848-865)
flixopt/elements.py (7)
flixopt/core.py (1)
PlausibilityError(29-32)flixopt/features.py (9)
InvestmentModel(24-130)OnOffModel(133-306)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/modeling.py (9)
ModelingPrimitives(179-363)BoundingPatterns(366-735)ModelingUtilitiesAbstract(14-98)expression_tracking_variable(183-219)bounds_with_state(404-447)scaled_bounds(450-490)scaled_bounds_with_state(493-547)to_binary(18-52)mutual_exclusivity_constraint(321-363)flixopt/interface.py (10)
InvestParameters(663-914)OnOffParameters(918-1179)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(874-906)transform_data(1123-1150)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)flixopt/structure.py (19)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)create_model(733-734)_plausibility_checks(728-731)transform_data(225-235)label_full(737-738)label_full(876-877)_do_modeling(936-938)add_variables(786-795)get_coords(139-168)get_coords(845-850)hours_per_step(132-133)hours_per_step(933-934)add_submodels(71-79)results_structure(1024-1029)add_constraints(797-806)values(996-997)register_variable(808-816)flixopt/effects.py (8)
effects(419-420)create_model(218-221)create_model(304-307)_plausibility_checks(223-225)_plausibility_checks(366-379)transform_data(177-216)_do_modeling(234-273)_do_modeling(508-521)flixopt/flow_system.py (5)
create_model(474-480)FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)coords(750-756)
flixopt/results.py (4)
flixopt/flow_system.py (8)
FlowSystem(46-890)to_dataset(246-258)flows(741-743)calculate_hours_per_timestep(177-182)from_dataset(261-315)_connect_network(627-659)coords(750-756)sel(762-797)flixopt/io.py (2)
load_dataset_from_netcdf(249-275)save_dataset_to_netcdf(198-246)flixopt/structure.py (9)
to_dataset(499-522)solution(106-129)items(990-991)get(838-843)get(1003-1005)from_dataset(543-576)values(996-997)variables(899-905)copy(692-704)flixopt/effects.py (2)
effects(419-420)calculate_effect_share_factors(446-468)
flixopt/interface.py (5)
flixopt/effects.py (2)
effects(419-420)transform_data(177-216)flixopt/components.py (3)
transform_data(208-215)transform_data(427-468)transform_data(699-703)flixopt/elements.py (3)
transform_data(100-106)transform_data(193-197)transform_data(422-450)flixopt/flow_system.py (3)
FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)tests/conftest.py (1)
piecewise(203-220)
flixopt/effects.py (3)
flixopt/features.py (8)
ShareAllocationModel(491-592)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)add_share(549-592)flixopt/structure.py (19)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)Submodel(762-938)label_full(737-738)label_full(876-877)create_model(733-734)_plausibility_checks(728-731)_do_modeling(936-938)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_constraints(797-806)items(990-991)values(996-997)add(999-1001)get(838-843)get(1003-1005)flixopt/flow_system.py (5)
FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)create_model(474-480)coords(750-756)
flixopt/components.py (5)
flixopt/elements.py (17)
Component(31-116)ComponentModel(783-865)Flow(226-503)create_model(95-98)create_model(188-191)create_model(417-420)_plausibility_checks(115-116)_plausibility_checks(199-205)_plausibility_checks(452-489)label_full(492-493)transform_data(100-106)transform_data(193-197)transform_data(422-450)_do_modeling(512-541)_do_modeling(744-766)_do_modeling(790-837)on_off(701-705)flixopt/features.py (7)
PiecewiseModel(352-432)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/modeling.py (2)
BoundingPatterns(366-735)scaled_bounds(450-490)flixopt/structure.py (14)
FlowSystemModel(82-204)create_model(733-734)_plausibility_checks(728-731)label_full(737-738)label_full(876-877)transform_data(225-235)values(996-997)items(990-991)_do_modeling(936-938)add_constraints(797-806)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)flixopt/flow_system.py (5)
create_model(474-480)FlowSystem(46-890)fit_to_model_coords(364-402)flows(741-743)coords(750-756)
flixopt/flow_system.py (4)
flixopt/core.py (5)
ConversionError(35-38)DataConverter(161-427)TimeSeriesData(41-149)fit_to_coords(86-101)to_dataarray(329-392)flixopt/effects.py (5)
effects(419-420)Effect(32-225)EffectCollection(291-468)create_effect_values_dict(320-364)transform_data(177-216)flixopt/elements.py (7)
Bus(120-209)Component(31-116)Flow(226-503)transform_data(100-106)transform_data(193-197)transform_data(422-450)label_full(492-493)flixopt/structure.py (16)
Element(715-759)FlowSystemModel(82-204)Interface(207-712)weights(171-182)_create_reference_structure(237-286)items(990-991)to_dataset(499-522)from_dataset(543-576)get(838-843)get(1003-1005)_resolve_dataarray_reference(371-402)_resolve_reference_structure(405-461)values(996-997)transform_data(225-235)label_full(737-738)label_full(876-877)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
58-58: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
61-61: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
63-63: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
67-67: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
119-119: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
120-120: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
121-121: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
🔇 Additional comments (19)
flixopt/interface.py (1)
197-206: Good: coherent time-dim propagation in Piecewise.The
has_time_dimsetter correctly cascades to pieces;transform_datarespects dims based on it.examples/03_Calculation_types/example_calculation_types.py (1)
49-50: Wrong series used for sell price.Use electricity_price, not electricity_demand.
Apply:
-TS_electricity_price_sell = fx.TimeSeriesData(-(electricity_demand - 0.5), aggregation_group='p_el') +TS_electricity_price_sell = fx.TimeSeriesData(-(electricity_price - 0.5), aggregation_group='p_el')flixopt/elements.py (1)
472-477: Fix.any()on scalar boolean (usenp.any).Calling
.any()on a scalar raises. Usenp.any(...)so both scalars and arrays work.Apply:
- if (self.relative_minimum > 0).any() and self.on_off_parameters is None: + if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:flixopt/modeling.py (5)
34-53: Harden to_binary for scalars/empty arrays and robust dim reduction.Current code errors on scalars and zero-length arrays; dim reduction can over-reduce.
Apply:
- if not isinstance(values, xr.DataArray): - values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))}) + if not isinstance(values, xr.DataArray): + if np.isscalar(values): + values = xr.DataArray(values) + else: + values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))}) @@ - if values.size == 0: - return xr.DataArray(0) if values.item() < epsilon else xr.DataArray(1) + if values.size == 0: + return xr.zeros_like(values, dtype=int) @@ - if dims is not None: - dims = [dims] if isinstance(dims, str) else dims - - binary_states = binary_states.any(dim=[d for d in binary_states.dims if d not in dims]) + if dims is not None: + dims = [dims] if isinstance(dims, str) else dims + reduce_dims = [d for d in binary_states.dims if d not in dims] + if reduce_dims: + binary_states = binary_states.any(dim=reduce_dims)
74-99: Fix count_consecutive_states scalar/nd handling and boolean misuse.Avoid
.all()on scalar and mix-ups between xarray and numpy arrays.Apply:
- binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim]) + if binary_values.ndim > 1: + binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim]) @@ - if np.isclose(binary_values.isel({dim: -1}).item(), 0, atol=epsilon).all(): + if np.isclose(binary_values.isel({dim: -1}).item(), 0.0, atol=epsilon): return 0.0 @@ - is_zero = np.isclose(binary_values, 0, atol=epsilon) + arr = binary_values.values + is_zero = np.isclose(arr, 0.0, atol=epsilon) @@ - zero_indices = np.where(is_zero)[0] - if len(zero_indices) == 0: + zero_indices = np.where(is_zero)[0] + if zero_indices.size == 0: # All 'on' - sum everything start_idx = 0 else: # Start after last zero start_idx = zero_indices[-1] + 1 @@ - consecutive_values = binary_values.isel({dim: slice(start_idx, None)}) - - return float(consecutive_values.sum().item()) # TODO: Som only over one dim? + return float(arr[start_idx:].sum())
311-314: Avoid ambiguous truth-value with xr objects in initial min-duration check.Coerce to scalars before comparison.
Apply:
- if previous_duration > 0 and previous_duration < minimum_duration.isel({duration_dim: 0}).max(): + prev = float(previous_duration) if not isinstance(previous_duration, xr.DataArray) else float(previous_duration.max().item()) + min0 = float(minimum_duration.isel({duration_dim: 0}).max().item()) + if prev > 0 and prev < min0:
438-446: Correct equality detection in bounds_with_state.Current boolean-vs-float comparison is wrong; use isclose across arrays.
Apply:
- if np.all(lower_bound - upper_bound) < 1e-10: + if np.all(np.isclose(lower_bound, upper_bound, atol=1e-10)): fix_constraint = model.add_constraints(variable == variable_state * upper_bound, name=f'{name}|fix') return [fix_constraint]
484-486: Correct equality detection in scaled_bounds.Use elementwise closeness.
Apply:
- if np.abs(rel_lower - rel_upper).all() < 10e-10: + if np.all(np.isclose(rel_lower, rel_upper, atol=1e-10)): return [model.add_constraints(variable == scaling_variable * rel_lower, name=f'{name}|fixed')]flixopt/results.py (1)
195-203: Backward-compat: convert oldflow_systemobjects to dataset.Assigning FlowSystem directly breaks later. Convert to xr.Dataset when provided.
Apply:
- if 'flow_system' in kwargs and flow_system_data is None: - flow_system_data = kwargs.pop('flow_system') + if 'flow_system' in kwargs and flow_system_data is None: + old = kwargs.pop('flow_system') + try: + from .flow_system import FlowSystem as _FS # local import to avoid cycles + except Exception: + _FS = None + if _FS is not None and isinstance(old, _FS): + flow_system_data = old.to_dataset() + elif isinstance(old, xr.Dataset): + flow_system_data = old + else: + raise TypeError( + "Deprecated 'flow_system' parameter must be a FlowSystem or an xr.Dataset. " + f'Got {type(old)}' + )flixopt/calculation.py (4)
68-74: Resolved: safe reuse guard now uses FlowSystem.used_in_calculationThe property exists on FlowSystem; this no longer risks AttributeError. LGTM.
192-218: Fix-sizes helper is useful; minor: skip non-existent vars earlyLGTM overall. The existence check and rounding are appropriate.
248-254: Nice: rounded main_results in log outputThe use of utils.round_nested_floats before yaml.dump improves readability. LGTM.
546-554: Bug: FlowSystem.sel requires keyword indexers (time=...)Passing the indexer positionally will fail. Use time=.
- calc = FullCalculation(f'{self.name}-{segment_name}', self.flow_system.sel(timesteps_of_segment)) + calc = FullCalculation( + f'{self.name}-{segment_name}', + self.flow_system.sel(time=timesteps_of_segment), + )flixopt/components.py (3)
758-793: LinearConverter piecewise modeling wiring looks correctSubmodel registration and zero_point usage are coherent. LGTM.
833-839: Storage balance applies discharge efficiency in wrong directionPer docstring, discharge losses should divide by eta_discharge. Current code multiplies.
- + charge_rate * eff_charge * hours_per_step - - discharge_rate * eff_discharge * hours_per_step, + + charge_rate * eff_charge * hours_per_step + - discharge_rate * hours_per_step / eff_discharge,
739-746: Guard relative_losses=None and simplify transmission equationWhen relative_losses is None, current expression fails. Use default 0 and clearer form.
- con_transmission = self.add_constraints( - out_flow.submodel.flow_rate == -in_flow.submodel.flow_rate * (self.element.relative_losses - 1), - short_name=name, - ) + rel_losses = 0 if self.element.relative_losses is None else self.element.relative_losses + con_transmission = self.add_constraints( + out_flow.submodel.flow_rate == in_flow.submodel.flow_rate * (1 - rel_losses), + short_name=name, + )flixopt/effects.py (1)
503-507: Objective construction is soundPenalty is scalar; objective_effect is weighted over (year, scenario). Summation is consistent. LGTM.
Also applies to: 519-521
flixopt/flow_system.py (2)
758-761: Public flag for calculation reuseused_in_calculation accessor backed by private flag is a good fit for Calculation’s copy-on-use. LGTM.
333-347: Serialization guard is helpfulAuto connect_and_transform before get_structure reduces surprises. LGTM.
| if i > 0 and self.nr_of_previous_values > 0: | ||
| self._transfer_start_values(i) | ||
|
|
||
| calculation.do_modeling() | ||
| invest_elements = [ | ||
| model.label_full | ||
| for component in self.flow_system.components.values() | ||
| for model in component.model.all_sub_models | ||
| if isinstance(model, InvestmentModel) | ||
| ] | ||
| if invest_elements: | ||
| logger.critical( | ||
| f'Investments are not supported in Segmented Calculation! ' | ||
| f'Following InvestmentModels were found: {invest_elements}' | ||
| ) | ||
|
|
||
| # Warn about Investments, but only in fist run | ||
| if i == 0: | ||
| invest_elements = [ |
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.
Validate previous-values transfer vs overlap
If nr_of_previous_values > overlap_timesteps, the slice will be out of range. Fail fast.
def _transfer_start_values(self, i: int):
"""
This function gets the last values of the previous solved segment and
inserts them as start values for the next segment
"""
+ if self.nr_of_previous_values > self.overlap_timesteps:
+ raise ValueError(
+ f'nr_of_previous_values ({self.nr_of_previous_values}) must be <= overlap_timesteps ({self.overlap_timesteps})'
+ )Also applies to: 605-618
🤖 Prompt for AI Agents
In flixopt/calculation.py around lines 572-579 (and similarly 605-618), the
transfer of previous values uses a slice that will go out of range when
self.nr_of_previous_values > overlap_timesteps; add a guard that validates the
relationship before slicing and fail fast: check if self.nr_of_previous_values
<= overlap_timesteps (or adjust slicing indices) and if not raise a clear
ValueError (or log and exit) indicating invalid configuration, or clamp/adjust
the slice boundaries to valid indices; apply the same validation and corrective
behavior to the second region (605-618).
Separate example testing from other tests by marking them. By default, purest doesn't run the example tests
* Update DataConverter * Update tests of error messages * Update tests of error messages * Update Dataconverter to allow bool values * fix tests
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: 4
🧹 Nitpick comments (6)
flixopt/core.py (3)
570-575: Time dimension validation: consider allowing PeriodIndex as well.Many time-indexed workflows use pd.PeriodIndex. Broadening acceptance improves ergonomics without weakening guarantees.
Apply this diff:
- if dim_name == 'time' and not isinstance(coord_index, pd.DatetimeIndex): + if dim_name == 'time' and not isinstance(coord_index, (pd.DatetimeIndex, pd.PeriodIndex)): raise ConversionError( f'Dimension named "time" should use DatetimeIndex for proper ' - f'time-series functionality, got {type(coord_index).__name__}' + f'time-series functionality, got {type(coord_index).__name__}' )
445-459: Fix doc examples to reference DataConverter correctly.Examples use an undefined variable “converter”. Use the class method directly for clarity.
Apply this diff:
- >>> coords = {'x': pd.Index([1, 2, 3]), 'y': pd.Index(['a', 'b'])} - >>> converter.to_dataarray(42, coords) + >>> coords = {'x': pd.Index([1, 2, 3]), 'y': pd.Index(['a', 'b'])} + >>> DataConverter.to_dataarray(42, coords) @@ - >>> series = pd.Series([10, 20, 30], index=[1, 2, 3]) - >>> converter.to_dataarray(series, coords) + >>> series = pd.Series([10, 20, 30], index=[1, 2, 3]) + >>> DataConverter.to_dataarray(series, coords) @@ - >>> array = np.array([[1, 2], [3, 4], [5, 6]]) # Shape (3, 2) - >>> converter.to_dataarray(array, coords) + >>> array = np.array([[1, 2], [3, 4], [5, 6]]) # Shape (3, 2) + >>> DataConverter.to_dataarray(array, coords)
317-339: Permutation search may be costly for many dims.For large target_dims, 𝑂(P(n,k)) grows fast. You can pre-index target coordinate lengths and prune permutations by mapping each axis length to candidate dims first.
I can provide a length-to-dims prefilter implementation if you want to pursue this now.
tests/test_dataconverter.py (3)
6-10: Remove the inline “adjust import” comment.Tests should not include TODO-style comments in import lines.
Apply this diff:
-from flixopt.core import ( # Adjust this import to match your project structure +from flixopt.core import (
519-544: Rename or drop the alias test; it asserts to_dataarray == to_dataarray.This test is redundant. If you intended to cover a legacy alias (as_dataarray), add that explicitly or rename to reflect idempotence.
Apply this diff to make intent clear:
-class TestAsDataArrayAlias: - """Test that to_dataarray works as an alias for to_dataarray.""" +class TestToDataArrayIdempotence: + """to_dataarray should be deterministic and idempotent.""" @@ - def test_to_dataarray_is_alias(self, time_coords, scenario_coords): - """to_dataarray should work identically to to_dataarray.""" + def test_to_dataarray_idempotent(self, time_coords, scenario_coords): + """Repeated calls produce the same result."""If legacy alias support is intended, add tests for as_dataarray calling to_dataarray or mark the alias deprecated.
1261-1263: Add coverage for TimeSeriesData.fit_to_coords and drop_constant_arrays.Current tests don’t exercise fit_to_coords or constant-array dropping.
I can add:
- A test ensuring fit_to_coords returns self when coords match and a new instance when broadcasting to extra dims.
- A test dataset where one variable is constant along time and gets dropped, verifying no ambiguous truth evaluation.
Want me to open a follow-up PR with these tests?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/python-app.yaml(4 hunks)flixopt/core.py(3 hunks)pyproject.toml(1 hunks)tests/test_dataconverter.py(1 hunks)tests/test_examples.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/python-app.yaml
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_dataconverter.py (1)
flixopt/core.py (5)
ConversionError(36-39)DataConverter(162-579)TimeSeriesData(42-150)to_dataarray(406-529)aggregation_group(105-106)
flixopt/core.py (1)
flixopt/flow_system.py (1)
coords(750-756)
🔇 Additional comments (3)
tests/test_examples.py (1)
19-19: Marker rename aligns with new test category.Assigning these example runs to the new
examplesmark matches the pytest configuration update and keeps the suite tidy.flixopt/core.py (2)
609-623: Fix ambiguous truth-value when detecting constant arrays.Comparing DataArrays in an if condition is ambiguous and will raise at runtime.
Apply this diff:
for name, da in ds.data_vars.items(): if dim in da.dims: - if da.max(dim) == da.min(dim): - drop_vars.append(name) + spread = da.max(dim) - da.min(dim) + # Reduce across remaining dims (if any) and convert to Python bool + is_constant = bool(spread.max().values == 0) + if is_constant: + drop_vars.append(name) continue elif drop_arrays_without_dim: drop_vars.append(name)Optional: add a tolerance for float noise (e.g., np.isclose) if “near-constant” is desired.
142-150: Deprecated aliases return undefined private attributes. Return attrs or public props instead.Accessing agg_group/agg_weight currently raises AttributeError.
Apply this diff:
@property def agg_group(self): warnings.warn('agg_group is deprecated, use aggregation_group instead', DeprecationWarning, stacklevel=2) - return self._aggregation_group + return self.attrs.get('aggregation_group') @property def agg_weight(self): warnings.warn('agg_weight is deprecated, use aggregation_weight instead', DeprecationWarning, stacklevel=2) - return self._aggregation_weight + return self.attrs.get('aggregation_weight')
flixopt/core.py
Outdated
| NonTemporalDataUser = int | float | np.integer | np.floating | np.ndarray | pd.Series | pd.DataFrame | xr.DataArray | ||
| """User data which has no time dimension. Internally converted to a Scalar or an xr.DataArray without a time dimension.""" | ||
|
|
||
| NonTemporalData = xr.DataArray | ||
| """Internally used datatypes for non-temporal data. Can be a Scalar or an xr.DataArray.""" | ||
|
|
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.
Docstring/type alias mismatch for NonTemporalData.
Alias is xr.DataArray but docstring says it can be a Scalar. Either update the alias or the docstring. Minimal fix: align the docstring.
Apply this diff:
-NonTemporalData = xr.DataArray
-"""Internally used datatypes for non-temporal data. Can be a Scalar or an xr.DataArray."""
+NonTemporalData = xr.DataArray
+"""Internally used datatype for non-temporal data. Always an xr.DataArray."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NonTemporalDataUser = int | float | np.integer | np.floating | np.ndarray | pd.Series | pd.DataFrame | xr.DataArray | |
| """User data which has no time dimension. Internally converted to a Scalar or an xr.DataArray without a time dimension.""" | |
| NonTemporalData = xr.DataArray | |
| """Internally used datatypes for non-temporal data. Can be a Scalar or an xr.DataArray.""" | |
| NonTemporalDataUser = int | float | np.integer | np.floating | np.ndarray | pd.Series | pd.DataFrame | xr.DataArray | |
| """User data which has no time dimension. Internally converted to a Scalar or an xr.DataArray without a time dimension.""" | |
| NonTemporalData = xr.DataArray | |
| """Internally used datatype for non-temporal data. Always an xr.DataArray.""" |
🤖 Prompt for AI Agents
In flixopt/core.py around lines 20 to 25, the type alias NonTemporalData is
declared as xr.DataArray but the docstring states it can be a Scalar or
xr.DataArray; update the docstring to match the alias (or vice versa). Make the
minimal change by editing the docstring to say "Internally used datatype for
non-temporal data: xr.DataArray." so the comment aligns with the alias.
| if (aggregation_group is not None) and (aggregation_weight is not None): | ||
| raise ValueError('Use either aggregation_group or aggregation_weight, not both') | ||
|
|
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Don't forbid using aggregation_group and aggregation_weight together (likely unintended).
These metadata are orthogonal. The current check prevents setting both and can break valid use-cases.
Apply this diff if both should be allowed:
- if (aggregation_group is not None) and (aggregation_weight is not None):
- raise ValueError('Use either aggregation_group or aggregation_weight, not both')
+ # aggregation_group and aggregation_weight can coexist🏁 Script executed:
#!/usr/bin/env bash
set -eux
# Locate core.py
echo "Possible paths for core.py:"
fd -t f core.py
# Show context around aggregation_group
echo "Context around aggregation_group in flixopt/core.py:"
rg -nC5 "aggregation_group" flixopt/core.py
# Show context around aggregation_weight
echo "Context around aggregation_weight in flixopt/core.py:"
rg -nC5 "aggregation_weight" flixopt/core.pyLength of output: 9262
🏁 Script executed:
#!/usr/bin/env bash
set -eux
echo "Other references to aggregation_group outside core.py:"
rg -n "aggregation_group" --exclude "flixopt/core.py"
echo "Other references to aggregation_weight outside core.py:"
rg -n "aggregation_weight" --exclude "flixopt/core.py"Length of output: 582
Allow aggregation_group and aggregation_weight together Drop the ValueError check in core.py init (lines 72–74) since these metadata are orthogonal.
- if (aggregation_group is not None) and (aggregation_weight is not None):
- raise ValueError('Use either aggregation_group or aggregation_weight, not both')
+ # aggregation_group and aggregation_weight can coexist📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (aggregation_group is not None) and (aggregation_weight is not None): | |
| raise ValueError('Use either aggregation_group or aggregation_weight, not both') | |
| # aggregation_group and aggregation_weight can coexist |
🤖 Prompt for AI Agents
In flixopt/core.py around lines 72–74, the constructor currently raises
ValueError if both aggregation_group and aggregation_weight are provided; remove
that mutual-exclusion check so both metadata can be accepted together. Edit the
__init__ to delete the if-block that raises the ValueError (or invert to no-op)
and ensure any downstream logic that depended on exclusivity treats the two
attributes as orthogonal (e.g., store both on self and update validation/tests
accordingly).
| if self.coords.equals(xr.Coordinates(coords)): | ||
| return self | ||
|
|
||
| da = DataConverter.to_dataarray(self.data, coords=coords) | ||
| return self.__class__( | ||
| da, | ||
| aggregation_group=self.aggregation_group, | ||
| aggregation_weight=self.aggregation_weight, | ||
| name=name if name is not None else self.name, |
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.
Invalid use of xr.Coordinates and brittle coord equality in fit_to_coords.
xr.Coordinates(...) is not a public constructor; this will fail. Also equality should compare per-dimension coordinate values. Suggest validating/normalizing coords and short-circuiting only when dims+coords match.
Apply this diff:
- if self.coords.equals(xr.Coordinates(coords)):
- return self
-
- da = DataConverter.to_dataarray(self.data, coords=coords)
+ target_coords, target_dims = DataConverter._validate_and_prepare_target_coordinates(coords)
+ # Fast path: same dims and coordinate values
+ if set(self.dims) == set(target_dims) and all(
+ np.array_equal(self.coords[d].values, target_coords[d].values) for d in target_dims
+ ):
+ return self
+
+ da = DataConverter.to_dataarray(self.data, coords=target_coords)
return self.__class__(
da,
aggregation_group=self.aggregation_group,
aggregation_weight=self.aggregation_weight,
name=name if name is not None else self.name,
)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In flixopt/core.py around lines 93–101, the code incorrectly constructs
xr.Coordinates(...) (not a public API) and performs a brittle overall equality
check; instead, validate and normalize the incoming coords to a canonical
mapping (e.g. mapping from dim->coordinate values), short‑circuit only when the
incoming dims exactly match self.coords.dims and each dimension's coordinate
array is elementwise equal to the corresponding self.coords[dim]; do not call
xr.Coordinates — just compare dims and per‑dimension coordinate values, and if
they differ, call DataConverter.to_dataarray(self.data, coords=coords) and
return the new instance preserving aggregation_group, aggregation_weight and
name logic.
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: 2
🧹 Nitpick comments (8)
.github/workflows/python-app.yaml (2)
93-121: test-examples only on tags — consider allowing manual/PR runsGood to run examples on releases. Recommend also enabling manual runs and/or PR-gated runs (e.g., when examples/** changes) to catch issues earlier.
Minimal change to allow manual runs too:
- if: startsWith(github.ref, 'refs/tags/v') + if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch'If you want PR runs when examples change, consider paths-filter and a job-level condition; I can draft that if desired.
200-201: Dependency chain for TestPyPI publish — verify intended flowpublish-testpypi depends on test, test-examples, and create-release; all are tag-gated, which is consistent. Note SKIP_TESTPYPI_UPLOAD only skips verification, not upload; name is misleading.
Consider renaming to SKIP_TESTPYPI_VERIFY or actually honoring it to skip upload too.
If keeping name, at least clarify with a comment.
flixopt/elements.py (2)
599-606: Prefer non-deprecated alias for investment.
_investmentis marked deprecated; useself.investment.size.Apply:
- scaling_variable=self._investment.size, + scaling_variable=self.investment.size,
755-761: Use xarray-native multiplication for aligned coords.
np.multiplyworks but*keeps xarray coords/attrs clearer.Apply:
- excess_penalty = np.multiply(self._model.hours_per_step, self.element.excess_penalty_per_flow_hour) + excess_penalty = self._model.hours_per_step * self.element.excess_penalty_per_flow_hourflixopt/components.py (3)
903-905: Tighten charge_state bounds for fixed-size investments.Use
minimum_or_fixed_size/maximum_or_fixed_sizeto improve bounds tightness and solver performance.Apply:
- relative_lower_bound * self.element.capacity_in_flow_hours.minimum_size, - relative_upper_bound * self.element.capacity_in_flow_hours.maximum_size, + relative_lower_bound * self.element.capacity_in_flow_hours.minimum_or_fixed_size, + relative_upper_bound * self.element.capacity_in_flow_hours.maximum_or_fixed_size,
680-692: *Use PlausibilityError and align message with _or_fixed_size.These are plausibility checks; keep exception types consistent and messages accurate.
Apply:
- if self.in2 is None: - raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows') + if self.in2 is None: + raise PlausibilityError('Balanced Transmission needs InvestParameters in both in-Flows') - if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters): - raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows') + if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters): + raise PlausibilityError('Balanced Transmission needs InvestParameters in both in-Flows') if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or ( self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size ).any(): - raise ValueError( - f'Balanced Transmission needs compatible minimum and maximum sizes.' - f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and ' - f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.' - ) + raise PlausibilityError( + 'Balanced Transmission needs compatible minimum and maximum sizes.' + f' Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and ' + f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.' + )
862-867: Avoid deprecated alias in balanced-size constraint.Use
.investment.sizeinstead of._investment.size.Apply:
- self.element.charging.submodel._investment.size * 1 - == self.element.discharging.submodel._investment.size * 1, + self.element.charging.submodel.investment.size + == self.element.discharging.submodel.investment.size,flixopt/effects.py (1)
373-380: Consider PlausibilityError for cycle detection failures.To align with other plausibility checks, raise
PlausibilityErrorrather thanValueError.Apply:
- raise ValueError(f'Error: circular operation-shares detected:\n{cycle_str}') + raise PlausibilityError(f'Error: circular operation-shares detected:\n{cycle_str}') @@ - raise ValueError(f'Error: circular invest-shares detected:\n{cycle_str}') + raise PlausibilityError(f'Error: circular invest-shares detected:\n{cycle_str}')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/python-app.yaml(4 hunks)flixopt/components.py(14 hunks)flixopt/effects.py(9 hunks)flixopt/elements.py(8 hunks)pyproject.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
flixopt/effects.py (3)
flixopt/features.py (8)
ShareAllocationModel(491-592)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)add_share(549-592)flixopt/structure.py (20)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)Submodel(762-938)label_full(737-738)label_full(876-877)create_model(733-734)_plausibility_checks(728-731)_do_modeling(936-938)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_constraints(797-806)items(990-991)values(996-997)weights(171-182)add(999-1001)get(838-843)get(1003-1005)flixopt/flow_system.py (5)
FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)create_model(474-480)coords(750-756)
flixopt/components.py (6)
flixopt/elements.py (17)
Component(31-116)ComponentModel(783-865)Flow(226-503)create_model(95-98)create_model(188-191)create_model(417-420)_plausibility_checks(115-116)_plausibility_checks(199-205)_plausibility_checks(452-489)label_full(492-493)transform_data(100-106)transform_data(193-197)transform_data(422-450)_do_modeling(512-541)_do_modeling(744-766)_do_modeling(790-837)on_off(701-705)flixopt/features.py (9)
InvestmentModel(24-130)PiecewiseModel(352-432)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/interface.py (11)
InvestParameters(663-914)OnOffParameters(918-1179)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(874-906)transform_data(1123-1150)items(435-442)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)flixopt/modeling.py (2)
BoundingPatterns(366-735)scaled_bounds(450-490)flixopt/structure.py (14)
FlowSystemModel(82-204)create_model(733-734)_plausibility_checks(728-731)label_full(737-738)label_full(876-877)transform_data(225-235)values(996-997)items(990-991)_do_modeling(936-938)add_constraints(797-806)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)flixopt/flow_system.py (5)
create_model(474-480)FlowSystem(46-890)fit_to_model_coords(364-402)flows(741-743)coords(750-756)
flixopt/elements.py (6)
flixopt/features.py (9)
InvestmentModel(24-130)OnOffModel(133-306)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/modeling.py (9)
ModelingPrimitives(179-363)BoundingPatterns(366-735)ModelingUtilitiesAbstract(14-98)expression_tracking_variable(183-219)bounds_with_state(404-447)scaled_bounds(450-490)scaled_bounds_with_state(493-547)to_binary(18-52)mutual_exclusivity_constraint(321-363)flixopt/interface.py (10)
InvestParameters(663-914)OnOffParameters(918-1179)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(874-906)transform_data(1123-1150)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)flixopt/structure.py (19)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)create_model(733-734)_plausibility_checks(728-731)transform_data(225-235)label_full(737-738)label_full(876-877)_do_modeling(936-938)add_variables(786-795)get_coords(139-168)get_coords(845-850)hours_per_step(132-133)hours_per_step(933-934)add_submodels(71-79)results_structure(1024-1029)add_constraints(797-806)values(996-997)register_variable(808-816)flixopt/effects.py (10)
effects(419-420)create_model(218-221)create_model(304-307)_plausibility_checks(223-225)_plausibility_checks(366-379)transform_data(177-216)_do_modeling(234-273)_do_modeling(508-521)add_share_to_effects(481-501)add_share_to_penalty(503-506)flixopt/flow_system.py (5)
create_model(474-480)FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)coords(750-756)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.10)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (10)
pyproject.toml (1)
185-185: Fix Windows-incompatible pytest addopts quoting.Pytest uses
shlex.split(..., posix=False)on Windows, so the single quotes survive and turn the marker expression into"\'not"—tests won’t even start. Swap the inner quotes to double quotes so this tokenizes correctly everywhere.-addopts = "-m 'not examples'" # Skip examples by default +addopts = '-m "not examples"' # Skip examples by default.github/workflows/python-app.yaml (3)
8-9: Include feature branch in PR triggers — LGTMExtending PR branches to include feature/v3/main aligns CI with your release work.
88-88: No changes needed for pytest-xdist .[dev] extras include pytest-xdist==3.8.0, so the--numprocesses=autoflag is supported.
161-161: Release gating includes test-examples — LGTMcreate-release waits on lint, test, test-examples, and security. This enforces example coverage before publishing.
scripts/extract_release_notes.py exists with a main() entrypoint and
__main__guard.flixopt/effects.py (1)
519-521: LGTM: Objective formulation (weighted operation + penalty).Objective uses scenario weights and scalar penalty; consistent with submodel design.
flixopt/elements.py (1)
472-477: Use np.any(...) to avoid ambiguous truth value on xarray/bool.
.any()on a scalar bool or xarray DataArray yields ambiguous truth value in anif. Usenp.any(...).Apply:
- if (self.relative_minimum > 0).any() and self.on_off_parameters is None: + if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:flixopt/components.py (4)
833-839: Correct discharge efficiency direction (matches docstring and physics).Discharge losses must divide by η_discharge, not multiply.
Apply:
- + charge_rate * eff_charge * hours_per_step - - discharge_rate * eff_discharge * hours_per_step, + + charge_rate * eff_charge * hours_per_step + - discharge_rate * hours_per_step / eff_discharge,
739-746: Guard relative_losses=None and simplify transmission equation.When
relative_lossesis None you’ll raise; also the current form is harder to read.Apply:
- con_transmission = self.add_constraints( - out_flow.submodel.flow_rate == -in_flow.submodel.flow_rate * (self.element.relative_losses - 1), - short_name=name, - ) + rel_losses = 0 if self.element.relative_losses is None else self.element.relative_losses + con_transmission = self.add_constraints( + out_flow.submodel.flow_rate == in_flow.submodel.flow_rate * (1 - rel_losses), + short_name=name, + )
474-505: *Don’t early-return on 'lastValueOfSim'; use _or_fixed_size and PlausibilityError.Early return skips bounds plausibility; also prefer
minimum_or_fixed_size/maximum_or_fixed_sizeand raisePlausibilityErrorfor out-of-range.Apply:
- if isinstance(self.initial_charge_state, str): - if self.initial_charge_state != 'lastValueOfSim': - raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}') - return + initial_is_last = isinstance(self.initial_charge_state, str) and self.initial_charge_state == 'lastValueOfSim' + if isinstance(self.initial_charge_state, str) and not initial_is_last: + raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}') if isinstance(self.capacity_in_flow_hours, InvestParameters): - if self.capacity_in_flow_hours.fixed_size is None: - maximum_capacity = self.capacity_in_flow_hours.maximum_size - minimum_capacity = self.capacity_in_flow_hours.minimum_size - else: - maximum_capacity = self.capacity_in_flow_hours.fixed_size - minimum_capacity = self.capacity_in_flow_hours.fixed_size + maximum_capacity = self.capacity_in_flow_hours.maximum_or_fixed_size + minimum_capacity = self.capacity_in_flow_hours.minimum_or_fixed_size else: maximum_capacity = self.capacity_in_flow_hours minimum_capacity = self.capacity_in_flow_hours # initial capacity >= allowed min for maximum_size: minimum_initial_capacity = maximum_capacity * self.relative_minimum_charge_state.isel(time=0) # initial capacity <= allowed max for minimum_size: maximum_initial_capacity = minimum_capacity * self.relative_maximum_charge_state.isel(time=0) - if (self.initial_charge_state > maximum_initial_capacity).any(): - raise ValueError( + if (not initial_is_last) and np.any(self.initial_charge_state > maximum_initial_capacity): + raise PlausibilityError( f'{self.label_full}: {self.initial_charge_state=} ' f'is above allowed maximum charge_state {maximum_initial_capacity}' ) - if (self.initial_charge_state < minimum_initial_capacity).any(): - raise ValueError( + if (not initial_is_last) and np.any(self.initial_charge_state < minimum_initial_capacity): + raise PlausibilityError( f'{self.label_full}: {self.initial_charge_state=} ' f'is below allowed minimum charge_state {minimum_initial_capacity}' )
506-521: *Balanced storage: use _or_fixed_size for compatibility and PlausibilityError.Ensure fixed-size investments are handled and error type is consistent.
Apply:
- if self.balanced: + if self.balanced: if not isinstance(self.charging.size, InvestParameters) or not isinstance( self.discharging.size, InvestParameters ): raise PlausibilityError( f'Balancing charging and discharging Flows in {self.label_full} is only possible with Investments.' ) - if (self.charging.size.minimum_size > self.discharging.size.maximum_size).any() or ( - self.charging.size.maximum_size < self.discharging.size.minimum_size + if (self.charging.size.minimum_or_fixed_size > self.discharging.size.maximum_or_fixed_size).any() or ( + self.charging.size.maximum_or_fixed_size < self.discharging.size.minimum_or_fixed_size ).any(): raise PlausibilityError( f'Balancing charging and discharging Flows in {self.label_full} need compatible minimum and maximum sizes.' - f'Got: {self.charging.size.minimum_size=}, {self.charging.size.maximum_size=} and ' - f'{self.discharging.size.minimum_size=}, {self.discharging.size.maximum_size=}.' + f'Got: {self.charging.size.minimum_or_fixed_size=}, {self.charging.size.maximum_or_fixed_size=} and ' + f'{self.discharging.size.minimum_or_fixed_size=}, {self.discharging.size.maximum_or_fixed_size=}.' )
| ModelingPrimitives.mutual_exclusivity_constraint( | ||
| self, | ||
| binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows], | ||
| short_name='prevent_simultaneous_use', | ||
| ) |
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.
Call the correct helper: mutual_exclusivity_constraint is in BoundingPatterns.
The method is defined on BoundingPatterns, not ModelingPrimitives.
Apply:
- ModelingPrimitives.mutual_exclusivity_constraint(
+ BoundingPatterns.mutual_exclusivity_constraint(
self,
binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows],
short_name='prevent_simultaneous_use',
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ModelingPrimitives.mutual_exclusivity_constraint( | |
| self, | |
| binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows], | |
| short_name='prevent_simultaneous_use', | |
| ) | |
| BoundingPatterns.mutual_exclusivity_constraint( | |
| self, | |
| binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows], | |
| short_name='prevent_simultaneous_use', | |
| ) |
🤖 Prompt for AI Agents
In flixopt/elements.py around lines 833 to 837, the call incorrectly references
ModelingPrimitives.mutual_exclusivity_constraint even though the helper is
defined on BoundingPatterns; replace the qualifier so the code calls
BoundingPatterns.mutual_exclusivity_constraint(self,
binary_variables=[flow.submodel.on_off.on for flow in
self.element.prevent_simultaneous_flows],
short_name='prevent_simultaneous_use'), and if BoundingPatterns is not already
in scope import or reference it appropriately so the method resolves.
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
🧹 Nitpick comments (3)
flixopt/elements.py (2)
599-606: Prefer public property over deprecated aliasUse
investmentinstead of_investmentfor clarity.BoundingPatterns.scaled_bounds_with_state( model=self, variable=self.flow_rate, - scaling_variable=self._investment.size, + scaling_variable=self.investment.size, relative_bounds=self.relative_flow_rate_bounds, scaling_bounds=(self.element.size.minimum_or_fixed_size, self.element.size.maximum_or_fixed_size), variable_state=self.on_off.on, )
713-718: Fix docstring: this exposes the Investment submodel, not OnOffdef investment(self) -> InvestmentModel | None: - """OnOff feature""" + """Investment submodel""" if 'investment' not in self.submodels: return None return self.submodels['investment']flixopt/effects.py (1)
359-364: Avoid property access that raises; check backing field insteadAccessing
standard_effecthere raises when unset; check the private field.- if self.standard_effect is None: + if self._standard_effect is None: raise KeyError( 'Scalar effect value provided but no standard effect is configured. ' 'Either set an effect as is_standard=True or provide a mapping {effect_label: value}.' )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/python-app.yaml(4 hunks)flixopt/components.py(14 hunks)flixopt/effects.py(9 hunks)flixopt/elements.py(8 hunks)pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/python-app.yaml
🧰 Additional context used
🧬 Code graph analysis (3)
flixopt/components.py (5)
flixopt/elements.py (17)
Component(31-116)ComponentModel(783-865)Flow(226-503)create_model(95-98)create_model(188-191)create_model(417-420)_plausibility_checks(115-116)_plausibility_checks(199-205)_plausibility_checks(452-489)label_full(492-493)transform_data(100-106)transform_data(193-197)transform_data(422-450)_do_modeling(512-541)_do_modeling(744-766)_do_modeling(790-837)on_off(701-705)flixopt/features.py (9)
InvestmentModel(24-130)PiecewiseModel(352-432)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/modeling.py (2)
BoundingPatterns(366-735)scaled_bounds(450-490)flixopt/structure.py (14)
FlowSystemModel(82-204)create_model(733-734)_plausibility_checks(728-731)label_full(737-738)label_full(876-877)transform_data(225-235)values(996-997)items(990-991)_do_modeling(936-938)add_constraints(797-806)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)flixopt/flow_system.py (4)
create_model(474-480)FlowSystem(46-890)fit_to_model_coords(364-402)coords(750-756)
flixopt/effects.py (3)
flixopt/features.py (8)
ShareAllocationModel(491-592)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)add_share(549-592)flixopt/structure.py (17)
FlowSystemModel(82-204)Submodel(762-938)label_full(737-738)label_full(876-877)create_model(733-734)_do_modeling(936-938)add_submodels(71-79)add_variables(786-795)get_coords(139-168)get_coords(845-850)add_constraints(797-806)items(990-991)values(996-997)weights(171-182)add(999-1001)get(838-843)get(1003-1005)flixopt/flow_system.py (5)
FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)create_model(474-480)coords(750-756)
flixopt/elements.py (7)
flixopt/features.py (9)
InvestmentModel(24-130)OnOffModel(133-306)size(121-123)_do_modeling(50-53)_do_modeling(162-232)_do_modeling(326-349)_do_modeling(383-432)_do_modeling(457-488)_do_modeling(523-547)flixopt/modeling.py (8)
ModelingPrimitives(179-363)BoundingPatterns(366-735)ModelingUtilitiesAbstract(14-98)expression_tracking_variable(183-219)bounds_with_state(404-447)scaled_bounds(450-490)scaled_bounds_with_state(493-547)to_binary(18-52)flixopt/interface.py (10)
InvestParameters(663-914)OnOffParameters(918-1179)transform_data(76-79)transform_data(222-224)transform_data(444-446)transform_data(656-659)transform_data(874-906)transform_data(1123-1150)minimum_or_fixed_size(909-910)maximum_or_fixed_size(913-914)flixopt/structure.py (17)
Element(715-759)ElementModel(1008-1029)FlowSystemModel(82-204)create_model(733-734)transform_data(225-235)label_full(737-738)label_full(876-877)_do_modeling(936-938)add_variables(786-795)get_coords(139-168)get_coords(845-850)hours_per_step(132-133)hours_per_step(933-934)add_submodels(71-79)results_structure(1024-1029)add_constraints(797-806)values(996-997)flixopt/effects.py (8)
effects(419-420)create_model(218-221)create_model(304-307)transform_data(177-216)_do_modeling(234-273)_do_modeling(508-521)add_share_to_effects(481-501)add_share_to_penalty(503-506)flixopt/components.py (9)
create_model(173-176)create_model(422-425)create_model(694-697)transform_data(208-215)transform_data(427-468)transform_data(699-703)_do_modeling(717-734)_do_modeling(757-793)_do_modeling(804-867)flixopt/flow_system.py (5)
create_model(474-480)FlowSystem(46-890)fit_to_model_coords(364-402)fit_effects_to_model_coords(404-426)coords(750-756)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (3.10)
🔇 Additional comments (6)
pyproject.toml (1)
185-185: Fix Windows-incompatible pytest addopts quoting
shlex.split(..., posix=False)on Windows ignores single quotes, so pytest sees the marker as"'not"and test invocation fails. Use double quotes inside the string so tokenization works cross-platform.-addopts = "-m 'not examples'" # Skip examples by default +addopts = '-m "not examples"' # Skip examples by defaultflixopt/effects.py (1)
519-521: LGTM: Objective formulation with weights and penaltyObjective composes weighted objective effect and scalar penalty correctly.
flixopt/elements.py (1)
472-477: Use np.any instead of .any() on potential scalars
.any()on a scalar boolean raises; prefernp.any(...).- if (self.relative_minimum > 0).any() and self.on_off_parameters is None: + if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:flixopt/components.py (3)
833-839: Discharge efficiency applied in wrong directionPer doc and standard storage balance, discharge losses should divide, not multiply.
self.add_constraints( charge_state.isel(time=slice(1, None)) == charge_state.isel(time=slice(None, -1)) * ((1 - rel_loss) ** hours_per_step) + charge_rate * eff_charge * hours_per_step - - discharge_rate * eff_discharge * hours_per_step, + - discharge_rate * hours_per_step / eff_discharge, short_name='charge_state', )
475-505: Don’t early-return on 'lastValueOfSim'; fix capacity handling and error typesEarly return skips critical checks. Use *_or_fixed_size and raise PlausibilityError.
- if isinstance(self.initial_charge_state, str): - if self.initial_charge_state != 'lastValueOfSim': - raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}') - return + initial_is_last = isinstance(self.initial_charge_state, str) and self.initial_charge_state == 'lastValueOfSim' + if isinstance(self.initial_charge_state, str) and not initial_is_last: + raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}') if isinstance(self.capacity_in_flow_hours, InvestParameters): - if self.capacity_in_flow_hours.fixed_size is None: - maximum_capacity = self.capacity_in_flow_hours.maximum_size - minimum_capacity = self.capacity_in_flow_hours.minimum_size - else: - maximum_capacity = self.capacity_in_flow_hours.fixed_size - minimum_capacity = self.capacity_in_flow_hours.fixed_size + maximum_capacity = self.capacity_in_flow_hours.maximum_or_fixed_size + minimum_capacity = self.capacity_in_flow_hours.minimum_or_fixed_size else: maximum_capacity = self.capacity_in_flow_hours minimum_capacity = self.capacity_in_flow_hours @@ - if (self.initial_charge_state > maximum_initial_capacity).any(): - raise ValueError( + if (not initial_is_last) and (self.initial_charge_state > maximum_initial_capacity).any(): + raise PlausibilityError( f'{self.label_full}: {self.initial_charge_state=} ' f'is above allowed maximum charge_state {maximum_initial_capacity}' ) - if (self.initial_charge_state < minimum_initial_capacity).any(): - raise ValueError( + if (not initial_is_last) and (self.initial_charge_state < minimum_initial_capacity).any(): + raise PlausibilityError( f'{self.label_full}: {self.initial_charge_state=} ' f'is below allowed minimum charge_state {minimum_initial_capacity}' )
739-746: Guard relative_losses=None and simplify transmission equationWhen
relative_lossesis None, current expression fails. Also make the relation explicit.- con_transmission = self.add_constraints( - out_flow.submodel.flow_rate == -in_flow.submodel.flow_rate * (self.element.relative_losses - 1), - short_name=name, - ) + rel_losses = 0 if self.element.relative_losses is None else self.element.relative_losses + con_transmission = self.add_constraints( + out_flow.submodel.flow_rate == in_flow.submodel.flow_rate * (1 - rel_losses), + short_name=name, + ) if self.element.absolute_losses is not None: con_transmission.lhs += in_flow.submodel.on_off.on * self.element.absolute_losses
| if self.balanced: | ||
| if self.in2 is None: | ||
| raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows') | ||
| if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters): | ||
| raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows') | ||
| if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or ( | ||
| self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size | ||
| ).any(): | ||
| raise ValueError( | ||
| 'Transmission currently does not support separate InvestParameters for Flows. ' | ||
| 'Please use Flow in1. The size of in2 is equal to in1. THis is handled internally' | ||
| f'Balanced Transmission needs compatible minimum and maximum sizes.' | ||
| f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and ' | ||
| f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.' | ||
| ) |
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.
🛠️ Refactor suggestion | 🟠 Major
Use PlausibilityError and consistent attributes in balanced Transmission checks
Align exception type and messages; use *_or_fixed_size.
- if self.balanced:
- if self.in2 is None:
- raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
- if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
- raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
- if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
- self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
- ).any():
- raise ValueError(
- f'Balanced Transmission needs compatible minimum and maximum sizes.'
- f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and '
- f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.'
- )
+ if self.balanced:
+ if self.in2 is None:
+ raise PlausibilityError('Balanced Transmission requires in2 (two directions).')
+ if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
+ raise PlausibilityError('Balanced Transmission requires InvestParameters on both in-Flows.')
+ if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
+ self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
+ ).any():
+ raise PlausibilityError(
+ 'Balanced Transmission needs compatible minimum and maximum sizes.'
+ f' Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and '
+ f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.'
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if self.balanced: | |
| if self.in2 is None: | |
| raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows') | |
| if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters): | |
| raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows') | |
| if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or ( | |
| self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size | |
| ).any(): | |
| raise ValueError( | |
| 'Transmission currently does not support separate InvestParameters for Flows. ' | |
| 'Please use Flow in1. The size of in2 is equal to in1. THis is handled internally' | |
| f'Balanced Transmission needs compatible minimum and maximum sizes.' | |
| f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and ' | |
| f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.' | |
| ) | |
| if self.balanced: | |
| if self.in2 is None: | |
| raise PlausibilityError('Balanced Transmission requires in2 (two directions).') | |
| if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters): | |
| raise PlausibilityError('Balanced Transmission requires InvestParameters on both in-Flows.') | |
| if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or ( | |
| self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size | |
| ).any(): | |
| raise PlausibilityError( | |
| 'Balanced Transmission needs compatible minimum and maximum sizes.' | |
| f' Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and ' | |
| f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.' | |
| ) |
🤖 Prompt for AI Agents
In flixopt/components.py around lines 680 to 692, the balanced Transmission
checks currently raise ValueError and mix attribute names; change the exceptions
to raise PlausibilityError and update the error messages to reference the
consistent *_or_fixed_size attribute names (e.g., minimum_or_fixed_size,
maximum_or_fixed_size, fixed_or_something if applicable) both in the predicate
comparisons and in the f-string details so the reported fields match the checked
attributes; keep the same logical checks but replace ValueError with
PlausibilityError and ensure the f-string prints the *_or_fixed_size attributes
rather than minimum_size/maximum_size.
| def _plausibility_checks(self) -> None: | ||
| if self.excess_penalty_per_flow_hour is not None: | ||
| zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0)) | ||
| if zero_penalty: | ||
| logger.warning( | ||
| f'In Bus {self.label}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.' | ||
| f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.' | ||
| ) |
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.
Fix ambiguous truth-value check for xarray DataArray in zero-penalty guard
np.all(...) on an xarray returns a 0-D DataArray; using it in if is ambiguous and can raise. Coerce to a Python bool.
- if self.excess_penalty_per_flow_hour is not None:
- zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0))
- if zero_penalty:
+ if self.excess_penalty_per_flow_hour is not None:
+ zero_penalty = bool(np.all(self.excess_penalty_per_flow_hour == 0).item())
+ if zero_penalty:
logger.warning(
f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _plausibility_checks(self) -> None: | |
| if self.excess_penalty_per_flow_hour is not None: | |
| zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0)) | |
| if zero_penalty: | |
| logger.warning( | |
| f'In Bus {self.label}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.' | |
| f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.' | |
| ) | |
| def _plausibility_checks(self) -> None: | |
| if self.excess_penalty_per_flow_hour is not None: | |
| zero_penalty = bool(np.all(self.excess_penalty_per_flow_hour == 0).item()) | |
| if zero_penalty: | |
| logger.warning( | |
| f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.' | |
| ) |
🤖 Prompt for AI Agents
In flixopt/elements.py around lines 199 to 205, the check uses np.all(...) which
returns a 0-D xarray DataArray and its truth value is ambiguous; coerce the
result to a Python bool before using it in the if statement (e.g., evaluate the
DataArray to a scalar via .item() or .values or wrap in bool(...)) so the guard
becomes a proper boolean check and avoids ambiguous truth-value errors.
# Conflicts: # CHANGELOG.md
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: 2
🧹 Nitpick comments (1)
CHANGELOG.md (1)
95-108: Fix nested list indentation for markdownlint
markdownlint(MD007) flagged the nested bullet indentation in this block. The inner bullets currently use 3–4 spaces; please adjust them to two spaces so the doc lint passes.-* IO for all Interfaces and the FlowSystem with round-trip serialization support - * NetCDF export/import capabilities for all Interface objects and FlowSystem - * JSON export for documentation purposes - * Recursive handling of nested Interface objects -* FlowSystem data manipulation methods - * `sel()` and `isel()` methods for temporal data selection - * `resample()` method for temporal resampling - * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data - * `__eq__()` method for FlowSystem comparison - -* Core data handling improvements - * `get_dataarray_stats()` function for statistical summaries - * Enhanced `DataConverter` class with better TimeSeriesData support +* IO for all Interfaces and the FlowSystem with round-trip serialization support + * NetCDF export/import capabilities for all Interface objects and FlowSystem + * JSON export for documentation purposes + * Recursive handling of nested Interface objects +* FlowSystem data manipulation methods + * `sel()` and `isel()` methods for temporal data selection + * `resample()` method for temporal resampling + * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data + * `__eq__()` method for FlowSystem comparison + +* Core data handling improvements + * `get_dataarray_stats()` function for statistical summaries + * Enhanced `DataConverter` class with better TimeSeriesData support
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/python-app.yaml(1 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
96-96: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
97-97: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
98-98: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
101-101: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
102-102: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
103-103: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
106-106: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
107-107: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
158-158: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
159-159: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
160-160: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
Add emojis to CHANGELOG.md
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: 4
♻️ Duplicate comments (5)
CHANGELOG.md (1)
180-181: Remove stray “Until here -->” marker.Leftover marker breaks formatting and tooling.
-Until here --> - +---pyproject.toml (1)
198-211: Reorder filters so the experimental network warning is actually ignored.First-match wins: with error::UserWarning:flixopt before the specific ignore, the warning is promoted to error. Move the message-specific ignore above the blanket error.
Apply this diff:
filterwarnings = [ # === Default behavior: show all warnings === "default", # === Treat flixopt warnings as errors (strict mode for our code) === # This ensures we catch deprecations, future changes, and user warnings in our own code + "ignore:.*network visualization is still experimental.*:UserWarning:flixopt", "error::DeprecationWarning:flixopt", "error::FutureWarning:flixopt", "error::UserWarning:flixopt", @@ - "ignore:.*network visualization is still experimental.*:UserWarning:flixopt", ]examples/03_Calculation_types/example_calculation_types.py (1)
226-230: Stacked vs grouped bars inconsistency (use grouped_bar or drop override)Currently style='stacked_bar' but barmode='group'. Pick one approach.
- fx.plotting.with_plotly( - pd.DataFrame(get_solutions(calculations, 'costs(temporal)|per_timestep').to_dataframe().sum()).T, - style='stacked_bar', - title='Total Cost Comparison', - ylabel='Costs [€]', - ).update_layout(barmode='group').write_html('results/Total Costs.html') + fx.plotting.with_plotly( + pd.DataFrame(get_solutions(calculations, 'costs(temporal)|per_timestep').to_dataframe().sum()).T, + style='grouped_bar', + title='Total Cost Comparison', + ylabel='Costs [€]', + ).write_html('results/Total Costs.html')examples/04_Scenarios/scenario_example.py (1)
22-25: Weights shape mismatch with (periods, scenarios) → likely runtime error.FlowSystem.fit_to_model_coords expects weights with dims ['period','scenario']; a 1D array of length 2 won’t align with periods=3, scenarios=2. Provide a 2D array (or a scenario-indexed object if you truly mean scenario-only weights).
Apply this diff for uniform weights over periods×scenarios:
- flow_system = fx.FlowSystem(timesteps=timesteps, periods=periods, scenarios=scenarios, weights=np.array([0.5, 0.6])) + weights = np.full((len(periods), len(scenarios)), 1.0/(len(periods)*len(scenarios))) + flow_system = fx.FlowSystem(timesteps=timesteps, periods=periods, scenarios=scenarios, weights=weights)Alternatively, for scenario-only weights: pass a pandas Series indexed by
scenariosand named'scenario'.Based on relevant code snippets (connect_and_transform expects dims ['period','scenario']).
examples/05_Two-stage-optimization/two_stage_optimization.py (1)
131-136: Timer reset and 'Dispatch' naming fixed correctlyStart is reset and calculation is named 'Dispatch' as intended.
🧹 Nitpick comments (17)
examples/02_Complex/complex_example.py (1)
17-20: Consider extracting experiment flags and documenting their purpose.The experiment configuration flags (
check_penalty,excess_penalty,use_chp_with_piecewise_conversion,time_indices) control different behaviors, but their interactions and purposes could be clearer. Consider adding brief comments explaining what each experiment tests.Example:
# --- Experiment Options --- -# Configure options for testing various parameters and behaviors -check_penalty = False -excess_penalty = 1e5 -use_chp_with_piecewise_conversion = True -time_indices = None # Define specific time steps for custom calculations, or use the entire series +# Configure options for testing various parameters and behaviors +check_penalty = False # Set to True to test penalty handling with extreme heat demand +excess_penalty = 1e5 # Penalty applied when bus balance cannot be satisfied +use_chp_with_piecewise_conversion = True # Toggle between simple CHP and piecewise CHP models +time_indices = None # Define specific time steps for custom calculations, or None for entire seriesflixopt/__init__.py (1)
54-54: Investigate addressing the tsam minimal value warning in flixopt.The TODO suggests this warning might be fixable in flixopt rather than suppressed. Addressing the root cause would improve code quality and reduce the need for warning filters.
Do you want me to help investigate the root cause of the tsam minimal value warning and generate a solution to address it in flixopt?
docs/index.md (2)
13-13: Align wording with released capability (avoid “active development”).CHANGELOG says periods/scenarios are introduced in this release. The parenthetical contradicts that and undermines confidence. Suggest removing it or clarifying stability.
-- **Multi-dimensional modeling**: Full support for multi-period investments and scenario-based stochastic optimization (periods and scenarios are in active development) +- **Multi-dimensional modeling**: Full support for multi-period investments and scenario-based stochastic optimization
19-21: Add authoritative links for tools/standards.Link “marimo” and “VDI 2067” to improve discoverability.
- - **Interactive tutorials**: Browser-based, reactive tutorials for learning FlixOpt without local installation (marimo) + - **Interactive tutorials**: Browser-based, reactive tutorials for learning FlixOpt without local installation ([marimo](https://marimo.io)) - - **Standardized cost calculations**: Align with industry standards (VDI 2067) for CAPEX/OPEX calculations + - **Standardized cost calculations**: Align with industry standards ([VDI 2067](https://www.vdi.de/richtlinien/details/vdi-2067-1-wirtschaftlichkeitsberechnung-von-energieversorgungsanlagen-allgemeine-bewertungen)) for CAPEX/OPEX calculationsCHANGELOG.md (3)
134-136: Call out effect-share removal as breaking, and add a migration hint.Replacing
specific_share_to_other_effects_*without deprecation is breaking. Duplicate a concise note under “💥 Breaking Changes” and add a one-line mapping example to guide users.### 💥 Breaking Changes ... +- Effect share parameters removed without deprecation. Migrate: + - `specific_share_to_other_effects_operation` → `share_from_temporal` (direction inverted) + - `specific_share_to_other_effects_invest` → `share_from_periodic` (direction inverted) + - Example: `{ 'CO2': 180 }` means costs “share from” CO2 at 180 €/tCO2.Also applies to: 84-92
165-165: Unify spelling: “Modeling” vs “Modelling”.Use one variant consistently (project mostly uses “modeling”).
-- Added new module `.modeling` that contains Modelling primitives and utilities +- Added new module `.modeling` that contains modeling primitives and utilities
95-97: Consider elevating FlowSystem sharing change to Breaking Changes.“FlowSystems cannot be shared across multiple Calculations anymore” can break existing workflows relying on identity/sharing semantics. Duplicating this under “💥 Breaking Changes” improves visibility for users scanning for behavioral breaks.
pyproject.toml (1)
52-54: Tighten numexpr lower bound to ≥2.10.0 for NumPy 2.x support
Update the pyproject.toml marker to"numexpr >= 2.10.0, < 2.14; python_version < '3.11'"since official NumPy 2.x support begins with 2.10.0.examples/03_Calculation_types/example_calculation_types.py (4)
36-38: Parse datetime index at read timeUse parse_dates to avoid the later manual conversion.
- data_import = pd.read_csv( - pathlib.Path(__file__).parent.parent / 'resources' / 'Zeitreihen2020.csv', index_col=0 - ).sort_index() + data_import = pd.read_csv( + pathlib.Path(__file__).parent.parent / 'resources' / 'Zeitreihen2020.csv', + index_col=0, + parse_dates=[0], + ).sort_index()Then you can drop the separate pd.to_datetime on Line 42.
127-129: Consider wrapping gas_price in TimeSeriesDataFor consistency and smoother aggregation handling, wrap gas_price; optionally set an aggregation_group if you want prices grouped in aggregation.
Based on learnings
- fx.Flow('Q_Gas', bus='Gas', size=1000, effects_per_flow_hour={costs.label: gas_price, CO2.label: 0.3}) + fx.Flow( + 'Q_Gas', + bus='Gas', + size=1000, + effects_per_flow_hour={costs.label: fx.TimeSeriesData(gas_price), CO2.label: 0.3}, + )
176-176: DRY solver configDefine the solver once and reuse to avoid duplication and drift.
Example:
solver = fx.solvers.HighsSolver(0.01 / 100, 60) ... calculation.solve(solver) ... calculation.do_modeling_and_solve(solver) ... calculation.solve(solver)Also applies to: 181-181, 190-190
233-235: Prefer plotting API kwargs over update_layout herePass title/xlabel/ylabel directly and keep style named for consistency.
- fx.plotting.with_plotly( - pd.DataFrame([calc.durations for calc in calculations], index=[calc.name for calc in calculations]), - 'stacked_bar', - ).update_layout(title='Duration Comparison', xaxis_title='Calculation type', yaxis_title='Time (s)').write_html( - 'results/Speed Comparison.html' - ) + fx.plotting.with_plotly( + pd.DataFrame([calc.durations for calc in calculations], index=[calc.name for calc in calculations]), + style='stacked_bar', + title='Duration Comparison', + xlabel='Calculation type', + ylabel='Time (s)', + ).write_html('results/Speed Comparison.html')examples/04_Scenarios/scenario_example.py (2)
124-131: Remove duplicate plot_heatmap call.
plot_heatmap('CHP(Q_th)|flow_rate')is called twice; drop one.- calculation.results.plot_heatmap('CHP(Q_th)|flow_rate')
122-122: Solver gap of 0 can stall; use a small positive gap.
mip_gap=0may cause long solves or non-termination. Prefer a small gap (e.g., 1e-4) for examples.- calculation.solve(fx.solvers.HighsSolver(mip_gap=0, time_limit_seconds=30)) + calculation.solve(fx.solvers.HighsSolver(mip_gap=1e-4, time_limit_seconds=30))examples/05_Two-stage-optimization/two_stage_optimization.py (3)
23-27: Guard against missing CSV and make path explicitAdd an exists check for the resource file to fail fast with a clear error.
- data_import = pd.read_csv( - pathlib.Path(__file__).parent.parent / 'resources' / 'Zeitreihen2020.csv', index_col=0 - ).sort_index() + data_path = pathlib.Path(__file__).resolve().parents[1] / 'resources' / 'Zeitreihen2020.csv' + if not data_path.exists(): + raise FileNotFoundError(f'Missing data file: {data_path}') + data_import = pd.read_csv(data_path, index_col=0).sort_index()
126-126: Sizing robustness: consider peak‑preserving resampleDefault 'mean' can under‑size capacities. Prefer an explicit method like 'max' for the sizing run.
Based on FlowSystem.resample API in the repo.
- calculation_sizing = fx.FullCalculation('Sizing', flow_system.resample('4h')) + calculation_sizing = fx.FullCalculation('Sizing', flow_system.resample('4h', method='max'))
68-69: Avoid duplicate flow labels on the same busMultiple flows labeled 'P_el' (CHP, Einspeisung, Stromtarif) can confuse outputs and plots. Optional: disambiguate labels.
- P_el=fx.Flow('P_el', bus='Strom'), + P_el=fx.Flow('P_el_chp', bus='Strom'), @@ - fx.Flow( - 'P_el', bus='Strom', size=1000, effects_per_flow_hour={'costs': electricity_price + 0.5, 'CO2': 0.3} - ) + fx.Flow( + 'P_el_grid_sell', bus='Strom', size=1000, + effects_per_flow_hour={'costs': electricity_price + 0.5, 'CO2': 0.3} + ) @@ - fx.Flow('P_el', bus='Strom', size=1000, effects_per_flow_hour={'costs': electricity_price, 'CO2': 0.3}) + fx.Flow('P_el_grid_buy', bus='Strom', size=1000, + effects_per_flow_hour={'costs': electricity_price, 'CO2': 0.3})Also applies to: 107-110, 117-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/resources/Zeitreihen2020.csvis excluded by!**/*.csv
📒 Files selected for processing (9)
CHANGELOG.md(2 hunks)docs/index.md(1 hunks)examples/02_Complex/complex_example.py(7 hunks)examples/03_Calculation_types/example_calculation_types.py(7 hunks)examples/04_Scenarios/scenario_example.py(1 hunks)examples/05_Two-stage-optimization/two_stage_optimization.py(1 hunks)flixopt/__init__.py(2 hunks)mkdocs.yml(1 hunks)pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mkdocs.yml
🧰 Additional context used
🧬 Code graph analysis (5)
pyproject.toml (2)
tests/test_config.py (2)
test_change_logging_level_removed(190-200)test_public_api(202-216)flixopt/components.py (1)
__init__(1117-1142)
examples/02_Complex/complex_example.py (4)
flixopt/config.py (3)
CONFIG(61-322)Logging(88-173)apply(208-244)flixopt/effects.py (1)
Effect(32-387)flixopt/components.py (2)
Source(1115-1219)Sink(1223-1342)flixopt/flow_system.py (1)
start_network_app(519-546)
examples/04_Scenarios/scenario_example.py (3)
flixopt/results.py (11)
flow_system(290-306)plot_network(754-774)plot_heatmap(688-752)plot_heatmap(1473-1507)plot_heatmap(1535-1593)plot_node_balance_pie(978-1058)plot_node_balance(917-976)node_balance_with_charge_state(1208-1240)plot_charge_state(1134-1206)to_file(776-823)to_file(1509-1532)flixopt/flow_system.py (2)
FlowSystem(46-980)plot_network(480-517)flixopt/calculation.py (4)
FullCalculation(176-266)do_modeling(191-199)do_modeling(311-325)solve(228-266)
examples/05_Two-stage-optimization/two_stage_optimization.py (3)
flixopt/results.py (3)
flow_system(290-306)size(1283-1291)sizes(433-460)flixopt/flow_system.py (4)
FlowSystem(46-980)add_elements(440-464)resample(926-976)sel(852-887)flixopt/calculation.py (5)
FullCalculation(176-266)do_modeling(191-199)do_modeling(311-325)solve(228-266)fix_sizes(201-226)
examples/03_Calculation_types/example_calculation_types.py (4)
flixopt/config.py (3)
CONFIG(61-322)Logging(88-173)apply(208-244)flixopt/core.py (3)
TimeSeriesData(42-150)aggregation_weight(109-110)aggregation_group(105-106)flixopt/components.py (2)
Sink(1223-1342)Source(1115-1219)flixopt/plotting.py (1)
with_plotly(328-466)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.10)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (21)
examples/02_Complex/complex_example.py (9)
12-14: LGTM! Logging configuration follows new API.The logging setup correctly uses the new
CONFIG.Logging.consoleandCONFIG.apply()pattern.
46-47: LGTM! Effect definitions align with new API.The
share_from_temporal={'CO2': 0.2}parameter correctly maps 20% of CO2 effects into Costs. The CO2 effect is properly defined and added to the flow system at line 189, satisfying the dependency.
63-66: LGTM! InvestParameters updated to new API.Field renames are correct:
fix_effects→effects_of_investmentoptional→mandatory(properly inverted:False→True)specific_effects→effects_of_investment_per_size
78-79: LGTM! Time-varying consecutive hours constraint.The array-based
consecutive_on_hours_minallows finer-grained control, with 9 values matching the 9 timesteps. The addedconsecutive_off_hours_maxparameter enhances the operational constraint modeling.
135-136: LGTM! Storage investment parameters updated consistently.The parameter renames (
piecewise_effects_of_investment,mandatory=True) align with the InvestParameters API updates seen elsewhere in the example.
152-159: LGTM! Sink wiring updated to new API.The change from
sink=fx.Flow(...)toinputs=[fx.Flow(...)]follows the new Sink constructor signature, making the input flows explicit and consistent with the v3 API.
165-172: LGTM! Source wiring updated to new API.The change from
source=fx.Flow(...)tooutputs=[fx.Flow(...)]aligns with the new Source constructor signature, making the output flows explicit.
178-184: LGTM! Sink wiring consistently updated.The Stromverkauf Sink follows the same
inputs=[fx.Flow(...)]pattern as the other Sink definitions in the file.
193-193: LGTM! Typo corrected.Comment grammar fixed from "DOes" to "Does".
flixopt/__init__.py (1)
8-12: LGTM! Version handling follows best practices.The try/except pattern for reading the installed version with a clear development-mode fallback is correct and idiomatic.
docs/index.md (1)
84-87: Image path casing is correct. The filedocs/images/architecture_flixOpt.pngexists with matching case, so the link will not 404.pyproject.toml (1)
189-190: Windows-safe pytest addopts quoting looks good.The switch to outer single quotes with inner double quotes fixes Windows shlex tokenization.
examples/03_Calculation_types/example_calculation_types.py (6)
14-16: Logging init LGTMEarly CONFIG.apply with console=True is fine.
53-55: Good: TimeSeriesData with aggregation metadataUsing aggregation_weight and aggregation_group matches the new API.
Based on learnings
116-122: Verify fixed_relative_profile alignmentEnsure both profiles’ lengths match timesteps and scaling is as intended (profile × size). Mismatches will error or skew loads.
140-141: Sell tariff wiring LGTMNegative sell price via TimeSeriesData is correct for revenues.
145-152: Buy tariff wiring LGTMEffect mapping with TimeSeriesData for costs is consistent with the API.
219-221: Costs series plotting LGTMVariable path and line style align with plotting API.
examples/04_Scenarios/scenario_example.py (1)
80-88: Verify InvestParameters keyword names match current signature. Confirm that effects_of_investment, fixed_size and mandatory are valid parameters on InvestParameters (e.g. via class annotations or runtime introspection); any mismatch will raise at runtime.examples/05_Two-stage-optimization/two_stage_optimization.py (2)
138-142: Size equality check is soundRounding to 5 decimals matches fix_sizes default; boolean reduction to Python bool is correct.
150-177: Verify and correct cost variable names in comparison
Ensure the keys you select fromcomparisonmatch those inresults.solution: if temporal costs are named
'costs(temporal)|per_timestep'(not plain'costs(temporal)'), update that entry; confirm whether a plain'costs'variable exists (and drop or rename it if not).
| # Advanced control when needed - extend with native linopy | ||
| boiler.model.add_constraints(custom_constraint, name="my_constraint") | ||
| ``` |
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.
Snippet likely uses the wrong access path to the underlying linopy model.
In v3, direct .model on a component (e.g., boiler.model) is not part of the public surface. Access the linopy model via Calculation (or FlowSystemModel), e.g., calc = fx.Calculation(fs).do_modeling(); calc.model....
-# Advanced control when needed - extend with native linopy
-boiler.model.add_constraints(custom_constraint, name="my_constraint")
+# Advanced control when needed — extend with native linopy
+calc = fx.Calculation(flow_system).do_modeling()
+calc.model.add_constraints(custom_constraint, name="my_constraint")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Advanced control when needed - extend with native linopy | |
| boiler.model.add_constraints(custom_constraint, name="my_constraint") | |
| ``` | |
| # Advanced control when needed — extend with native linopy | |
| calc = fx.Calculation(flow_system).do_modeling() | |
| calc.model.add_constraints(custom_constraint, name="my_constraint") |
🤖 Prompt for AI Agents
In docs/index.md around lines 61 to 63, the snippet uses a non-public access
path (.model on a component like boiler.model); update the example to obtain the
linopy model via the public Calculation/FlowSystemModel API (e.g., create a
Calculation for the FlowSystemModel and run do_modeling() to get the calculation
object) and then call add_constraints on calc.model or the calculation's model
object instead of using component.model directly so the docs reflect the v3
public surface.
| # --- Plotting for comparison --- | ||
| fx.plotting.with_plotly( | ||
| get_solutions(calculations, 'Speicher|charge_state').to_dataframe(), | ||
| mode='line', | ||
| style='line', | ||
| title='Charge State Comparison', | ||
| ylabel='Charge state', | ||
| ).write_html('results/Charge State.html') | ||
|
|
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.
Create results/ directory before writing HTML
write_html will fail if the folder doesn’t exist.
# --- Plotting for comparison ---
+ pathlib.Path('results').mkdir(parents=True, exist_ok=True)
fx.plotting.with_plotly(
get_solutions(calculations, 'Speicher|charge_state').to_dataframe(),
style='line',
title='Charge State Comparison',
ylabel='Charge state',
).write_html('results/Charge State.html')Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/03_Calculation_types/example_calculation_types.py around lines
203–210, the call to write_html('results/Charge State.html') will fail if the
results/ directory does not exist; ensure the directory is created before
calling write_html by creating the results folder (e.g., using os.makedirs or
pathlib.Path(...).mkdir with parents=True and exist_ok=True) just prior to
writing the file so the HTML can be saved without error.
| power_prices = np.array([0.08, 0.09, 0.10]) | ||
|
|
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.
Make power price effect explicit and dimensioned by 'period'.
Passing a raw ndarray to effects_per_flow_hour is ambiguous and currently not mapped to the objective effect. Use a period-indexed Series and map to costs:
- power_prices = np.array([0.08, 0.09, 0.10])
+ power_prices = pd.Series([0.08, 0.09, 0.10], index=periods, name='period') # €/kWh by period
...
- power_sink = fx.Sink(
- label='Einspeisung', inputs=[fx.Flow(label='P_el', bus='Strom', effects_per_flow_hour=-1 * power_prices)]
- )
+ power_sink = fx.Sink(
+ label='Einspeisung',
+ inputs=[fx.Flow(label='P_el', bus='Strom', effects_per_flow_hour={costs.label: -power_prices})],
+ )This ensures correct effect mapping and coordinate alignment.
Also applies to: 105-107
🤖 Prompt for AI Agents
In examples/04_Scenarios/scenario_example.py around lines 22-23 (and also lines
105-107), passing a raw numpy array for power_prices to effects_per_flow_hour is
ambiguous and not aligned to the model periods; replace the ndarray with a
pandas Series indexed by the scenario/periods (e.g., pd.Series(power_prices,
index=periods)) and then assign that Series to the objective mapping (e.g., map
to 'costs' via effects_per_flow_hour with explicit period-aligned coordinates)
so the effect is explicit and correctly aligned to the model periods.
| # === Treat flixopt warnings as errors (strict mode for our code) === | ||
| # This ensures we catch deprecations, future changes, and user warnings in our own code | ||
| "error::DeprecationWarning:flixopt", | ||
| "error::FutureWarning:flixopt", | ||
| "error::UserWarning:flixopt", | ||
|
|
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.
Allow-list deprecation used by tests to prevent failures.
Tests assert on flixopt deprecations (e.g., change_logging_level) using pytest.warns. Globally promoting flixopt DeprecationWarning to error will break those tests. Add a message-scoped default rule before the blanket errors.
Apply this diff:
filterwarnings = [
# === Default behavior: show all warnings ===
"default",
# === Treat flixopt warnings as errors (strict mode for our code) ===
# This ensures we catch deprecations, future changes, and user warnings in our own code
+ # Allow specific deprecation expected by tests to remain a warning
+ "default:change_logging_level is deprecated:DeprecationWarning:flixopt",
"error::DeprecationWarning:flixopt",
"error::FutureWarning:flixopt",
"error::UserWarning:flixopt",Reference: tests/test_config.py expects a DeprecationWarning from flixopt.change_logging_level.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # === Treat flixopt warnings as errors (strict mode for our code) === | |
| # This ensures we catch deprecations, future changes, and user warnings in our own code | |
| "error::DeprecationWarning:flixopt", | |
| "error::FutureWarning:flixopt", | |
| "error::UserWarning:flixopt", | |
| filterwarnings = [ | |
| # === Default behavior: show all warnings === | |
| "default", | |
| # === Treat flixopt warnings as errors (strict mode for our code) === | |
| # This ensures we catch deprecations, future changes, and user warnings in our own code | |
| # Allow specific deprecation expected by tests to remain a warning | |
| "default:change_logging_level is deprecated:DeprecationWarning:flixopt", | |
| "error::DeprecationWarning:flixopt", | |
| "error::FutureWarning:flixopt", | |
| "error::UserWarning:flixopt", | |
| … | |
| ] |
🤖 Prompt for AI Agents
In pyproject.toml around lines 198-203, the blanket rules escalate all flixopt
DeprecationWarnings to errors which breaks tests that intentionally assert
specific deprecations; insert a message-scoped allow/default rule immediately
before the three blanket "error::...:flixopt" lines that targets the specific
flixopt DeprecationWarning message(s) used by tests (e.g., the
change_logging_level deprecation) so those warnings remain warnings for test
assertions while leaving the remaining flixopt warnings promoted to errors.
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: 2
♻️ Duplicate comments (3)
pyproject.toml (1)
195-211: Reorder filters: specific ignore must come before blanket error.Pytest applies the first matching filter. With
error::UserWarning:flixoptbefore the message-specific ignore, the “network visualization is still experimental” warning becomes an error.Apply:
filterwarnings = [ # === Default behavior: show all warnings === "default", # === Treat flixopt warnings as errors (strict mode for our code) === # This ensures we catch deprecations, future changes, and user warnings in our own code + "ignore:.*network visualization is still experimental.*:UserWarning:flixopt", "error::DeprecationWarning:flixopt", "error::FutureWarning:flixopt", "error::UserWarning:flixopt", # === Third-party warnings (mirrored from __init__.py) === "ignore::FutureWarning:tsam", "ignore:.*minimal value.*exceeds.*:UserWarning:tsam", "ignore:Coordinates across variables not equal:UserWarning:linopy", "ignore:.*join will change from join='outer' to join='exact'.*:FutureWarning:linopy", "ignore:numpy\\.ndarray size changed:RuntimeWarning", - "ignore:.*network visualization is still experimental.*:UserWarning:flixopt", ]Optionally broaden the NumPy ABI warning ignore to cover “ufunc size changed” variants:
- "ignore:numpy\\.ndarray size changed:RuntimeWarning", + "ignore:.*size changed.*:RuntimeWarning",Only if you intentionally want to suppress all such ABI warnings in tests.
examples/03_Calculation_types/example_calculation_types.py (1)
226-230: Clarify plotting intent: stacked vs. grouped bars.Line 227 specifies
style='stacked_bar'but line 230 overrides with.update_layout(barmode='group'), producing grouped bars instead. This inconsistency was previously flagged: if grouped bars are desired, usestyle='grouped_bar'directly; if stacked bars are intended, remove thebarmode='group'override.CHANGELOG.md (1)
180-180: Remove stray “Until here -->” marker.Leftover debug marker breaks formatting and tooling. Delete the line.
-Until here -->
🧹 Nitpick comments (13)
pyproject.toml (1)
52-53: Relax numexpr cap; exclude the bad release only.Current
<2.14blocks future 2.14.x fixes. Prefer excluding 2.14.0 and allowing later 2.x on Py<3.11.Apply:
- "numexpr >= 2.8.4, < 2.14; python_version < '3.11'", # Avoid 2.14.0 on older Python + "numexpr >= 2.8.4, != 2.14.0, < 3; python_version < '3.11'", # Avoid 2.14.0 on older Python; allow future 2.xPlease confirm this still resolves the Py3.10 + NumPy 1.26.4 issue in your CI matrix.
examples/02_Complex/complex_example.py (1)
193-193: LGTM! Network app start includes helpful dependency warning.The comment clearly informs users about the optional dependency requirement.
If you want more graceful handling, consider wrapping the call:
-flow_system.start_network_app() # Start the network app. Does only work with extra dependencies installed +try: + flow_system.start_network_app() # Start the network app +except ImportError as e: + print(f"Network app requires extra dependencies: {e}").github/workflows/python-app.yaml (5)
90-92: Surface warnings during tests (avoid hiding deprecations)Dropping the warnings plugin helps catch deprecations before releasing v3.
- - name: Run tests - run: pytest -v -p no:warnings --numprocesses=auto + - name: Run tests + run: pytest -v --numprocesses=auto
231-237: Migrate TestPyPI upload to OIDC (Trusted Publishing)Removes long‑lived tokens and simplifies secrets management.
- - name: Upload to TestPyPI - run: | - twine upload --repository-url https://test.pypi.org/legacy/ dist/* --verbose --skip-existing - env: - TWINE_USERNAME: __token__ - TWINE_PASSWORD: ${{ secrets.TEST_PYPI_API_TOKEN }} - TWINE_NON_INTERACTIVE: "1" + - name: Upload to TestPyPI (OIDC) + uses: pypa/gh-action-pypi-publish@release/v1 + with: + repository-url: https://test.pypi.org/legacy/ + packages-dir: dist/ + skip-existing: trueNote: Requires enabling Trusted Publishing for your TestPyPI project.
317-324: Migrate PyPI upload to OIDC (Trusted Publishing)Use the official action and remove API tokens.
- - name: Upload to PyPI - run: | - twine upload dist/* --verbose --skip-existing - env: - TWINE_USERNAME: __token__ - TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }} - TWINE_NON_INTERACTIVE: "1" + - name: Upload to PyPI (OIDC) + uses: pypa/gh-action-pypi-publish@release/v1 + with: + packages-dir: dist/ + skip-existing: trueNote: Requires enabling Trusted Publishing for your PyPI project.
187-194: Avoid mixing body_path with generate_release_notesWhen body_path is provided, auto‑generated notes are redundant. Pick one.
- name: Create GitHub Release uses: softprops/action-gh-release@v2 with: body_path: current_release_notes.md draft: false - prerelease: ${{ contains(github.ref, 'alpha') || contains(github.ref, 'beta') || contains(github.ref, 'rc') }} - generate_release_notes: true + prerelease: ${{ contains(github.ref, 'alpha') || contains(github.ref, 'beta') || contains(github.ref, 'rc') }}
33-40: Pin actions to commit SHAs for supply‑chain hardeningReplace version tags (e.g., actions/checkout@v5) with immutable commit SHAs across the workflow.
Also applies to: 41-45, 72-79, 81-85, 100-109, 110-114, 127-135, 136-140, 165-175, 176-180, 209-217, 218-222, 295-303, 304-308, 379-389, 390-394
docs/index.md (2)
13-14: Clarify support status for periods/scenarios.Text says “Full support …” and “in active development” simultaneously. Reword to avoid contradiction and set accurate expectations.
19-21: Add reference link for “marimo”.Link the tool for readers unfamiliar with it, e.g., https://marimo.io
CHANGELOG.md (4)
99-103: Consistent naming/casing.Use consistent casing for “FlowSystem” and avoid snake_case in prose (“flow_system”). Example:
- “variables and constraints for the FlowSystem.”
74-76: Use code formatting consistently for dataset/function names.Surround identifiers like
effects_per_componentwith backticks for consistency.-- `effects_per_component()` dataset +- `effects_per_component` datasetAlso applies to: 147-154
161-168: Standardize “modeling/modelling”.Use a single variant across the file (prefer “modeling” to match other entries).
86-92: Add migration pointer for breaking changes.Consider a short “Migration notes” link/section guiding users to update patterns (Effect sharing, class renames, return type of
Calculation.do_modeling()).I can draft a minimal migration checklist if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/resources/Zeitreihen2020.csvis excluded by!**/*.csv
📒 Files selected for processing (10)
.github/workflows/python-app.yaml(1 hunks)CHANGELOG.md(2 hunks)docs/index.md(1 hunks)examples/02_Complex/complex_example.py(7 hunks)examples/03_Calculation_types/example_calculation_types.py(7 hunks)examples/04_Scenarios/scenario_example.py(1 hunks)examples/05_Two-stage-optimization/two_stage_optimization.py(1 hunks)flixopt/__init__.py(2 hunks)mkdocs.yml(1 hunks)pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- mkdocs.yml
- examples/04_Scenarios/scenario_example.py
🧰 Additional context used
🧬 Code graph analysis (3)
examples/02_Complex/complex_example.py (3)
flixopt/config.py (3)
CONFIG(61-322)Logging(88-173)apply(208-244)flixopt/effects.py (1)
Effect(32-387)flixopt/components.py (2)
Source(1115-1219)Sink(1223-1342)
examples/03_Calculation_types/example_calculation_types.py (5)
flixopt/config.py (3)
CONFIG(61-322)Logging(88-173)apply(208-244)flixopt/core.py (3)
TimeSeriesData(42-150)aggregation_weight(109-110)aggregation_group(105-106)flixopt/components.py (2)
Sink(1223-1342)Source(1115-1219)flixopt/solvers.py (1)
HighsSolver(61-81)flixopt/plotting.py (1)
with_plotly(328-466)
examples/05_Two-stage-optimization/two_stage_optimization.py (3)
flixopt/results.py (3)
flow_system(290-306)size(1283-1291)sizes(433-460)flixopt/flow_system.py (3)
FlowSystem(46-980)resample(926-976)sel(852-887)flixopt/calculation.py (5)
FullCalculation(176-266)do_modeling(191-199)do_modeling(311-325)solve(228-266)fix_sizes(201-226)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
🔇 Additional comments (25)
pyproject.toml (1)
189-190: LGTM: cross‑platform pytest marker quoting fixed.The addopts string now tokenizes on Windows and POSIX.
flixopt/__init__.py (3)
5-6: LGTM! Standard imports for version handling and warning management.The use of
importlib.metadatais the modern Python approach for retrieving package metadata.
8-12: LGTM! Standard version fallback pattern.The try/except pattern with a development mode fallback is a common and appropriate approach for handling package metadata in various installation scenarios.
69-71: Verify manually: install NumPy and test for “numpy.ndarray size changed” warnings
CI environment lacks NumPy. In your local setup, install NumPy (≥2.x) and any compiled‐extension deps (e.g. pandas, xarray), then rerun imports to see if the RuntimeWarning still occurs. If not, remove the filter; if it does, update the comment to specify which dependency triggers it and link the upstream issue.examples/05_Two-stage-optimization/two_stage_optimization.py (3)
1-20: LGTM! Well-structured example demonstrating two-stage optimization.The module docstring clearly explains the downsampling approach and its benefits. The imports and setup are appropriate for this example.
I note that the previous timer and naming issues flagged in earlier reviews have been successfully addressed (start timer reset on line 131, correct 'Dispatch' naming on line 132).
124-136: Two-stage optimization correctly implemented.The sizing phase uses a downsampled system (
resample('4h')) to reduce computational cost, then the dispatch phase uses the full system with sizes fixed from the sizing results. The timer resets and calculation naming are now correct.
150-179: Comparison logic and output are well-structured.The results comparison correctly:
- Concatenates solutions from both approaches along a 'mode' dimension
- Aggregates timing (sizing + dispatch for two-stage)
- Computes percentage differences
- Presents a clear comparison of key metrics (durations, costs, component sizes)
examples/02_Complex/complex_example.py (8)
12-14: LGTM! Logging configuration follows the new CONFIG API correctly.The console logging setup uses the proper v3 API pattern with
CONFIG.Logging.console = Truefollowed byCONFIG.apply()to activate the configuration.
46-47: LGTM! Effect sharing parameter correctly implements carbon pricing.The
share_from_temporal={'CO2': 0.2}parameter on the Costs effect establishes a 0.2 €/kg carbon price, which is a standard approach for modeling CO2 emission costs in energy systems. This aligns with v3's redesigned effect sharing mechanism.
63-66: LGTM! InvestParameters correctly use the renamed v3 API.The parameter renames (
effects_of_investment,mandatory,effects_of_investment_per_size) align with the v3 public API changes and are applied correctly.
78-79: LGTM! Time-varying on-off constraints are correctly specified.The
consecutive_on_hours_minarray correctly has 9 elements matching the 9 timesteps, demonstrating time-varying operational constraints where minimum consecutive on hours increase to 2 after period 5.
135-136: LGTM! Storage investment parameters correctly use the renamed v3 API.The
piecewise_effects_of_investmentparameter name aligns with v3's API changes and properly references the segmented investment effects defined above.
152-159: LGTM! Sink component correctly migrated to v3 list-based API.The migration from the deprecated
sinkkwarg toinputs=[fx.Flow(...)]follows the new v3 API pattern shown in the Sink class definition.
165-172: LGTM! Source component correctly migrated to v3 list-based API.The migration from the deprecated
sourcekwarg tooutputs=[fx.Flow(...)]follows the new v3 API pattern shown in the Source class definition.
178-184: LGTM! Feed-in sink correctly migrated to v3 list-based API.The migration follows the same pattern as the other Sink components, correctly using the negative electricity price for feed-in revenue modeling.
examples/03_Calculation_types/example_calculation_types.py (6)
14-16: LGTM! Logging configuration follows the correct API pattern.The console logging setup correctly uses
fx.CONFIG.Logging.console = Truefollowed byfx.CONFIG.apply()to activate the settings, consistent with the configuration API introduced in v3.
53-55: LGTM! Correct usage of the updated TimeSeriesData aggregation API.The
TimeSeriesDatainstantiations correctly use the newaggregation_weightandaggregation_groupparameters instead of the deprecatedagg_weightandagg_group. Line 54 also correctly useselectricity_priceinstead of the previously incorrectelectricity_demand.
116-153: LGTM! Sink and Source definitions updated to the new API.All
SinkandSourcecomponent instantiations correctly use the updated API withinputs=[...]andoutputs=[...]list parameters instead of the deprecatedsink=andsource=parameters. Flow definitions are properly wrapped in lists as expected by the new signature.
176-176: LGTM! Reasonable MIP gap relaxation for improved performance.The solver configuration has been updated from an exact solution (
mip_gap=0) to a 0.01% relative optimality gap (0.01/100 = 0.0001). This small relaxation can significantly reduce solve time while maintaining solution quality within 0.01% of optimal, applied consistently across Full, Segmented, and Aggregated calculations with a 60-second time limit.Also applies to: 181-181, 190-190
206-206: LGTM! Appropriate plotting style for time-series comparison.The use of
style='line'for visualizing charge state, flow rate, and temporal cost comparisons is appropriate for these time-series data, providing clear stepped line plots that facilitate comparison across calculation types.Also applies to: 213-213, 220-220
233-233: Valid durations attribute:calc.durationsis defined in theCalculationbase class (flixopt/calculation.py:93) and correctly populated for all calculation types..github/workflows/python-app.yaml (2)
8-9: PR trigger patterns: verify intended coverageGlobs look fine and cover dev and feature branches. Do you also want PRs into release/* or hotfix/* to run? If yes, add 'release/' and/or 'hotfix/'.
181-186: Helper scripts and extras verified scripts/extract_release_notes.py and scripts/extract_changelog.py are present; ‘dev’ and ‘docs’ optional-dependencies include pytest, pytest-xdist, mkdocs, and mike.docs/index.md (2)
84-87: Image path is valid../images/architecture_flixOpt.pngexists indocs/imagesand will be included in the docs build.
61-63: Example’s use ofboiler.model.add_constraintsis already correct for v3’s Submodel API –boiler.modelreturns aSubmodel, so itsadd_constraintsmethod properly scopes new constraints. No update needed.
| - **Recipe collection**: Community-driven library of common modeling patterns, data manipulation techniques, and optimization strategies (see [Recipes](user-guide/recipes/index.md) - help wanted!) | ||
|
|
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
Validate internal doc links.
Check that these targets exist:
user-guide/recipes/index.mdgetting-started.md
Also applies to: 95-96
🏁 Script executed:
#!/bin/bash
# From repo root; look under docs/
fd -a -HI 'docs' -t d >/dev/null || echo "No docs/ dir found"
fd -a -HI 'user-guide/recipes/index.md' 'docs' -x echo "{}"
fd -a -HI 'getting-started.md' 'docs' -x echo "{}"Length of output: 42
🏁 Script executed:
#!/bin/bash
# From repo root, search under docs/
fd -HI -t d user-guide docs
fd -HI -t d recipes docs
fd -HI -t f '*recipe*' docsLength of output: 304
🏁 Script executed:
#!/bin/bash
# From repo root; search for any “recipe” entries under docs/user-guide
fd -HI -t d --glob '*recipe*' docs/user-guide
# List all index.md files under docs/user-guide
fd -HI -t f index.md docs/user-guideLength of output: 136
Ensure internal documentation links point to valid targets
user-guide/recipes/index.mdis missing; update touser-guide/mathematical-notation/modeling-patterns/index.mdgetting-started.mdis present atdocs/getting-started.mdand requires no change
🤖 Prompt for AI Agents
In docs/index.md around lines 22 to 23, the internal link points to a missing
target user-guide/recipes/index.md; update that link to
user-guide/mathematical-notation/modeling-patterns/index.md so the markdown
reference resolves correctly (no change needed for getting-started.md).
flixopt/__init__.py
Outdated
| warnings.filterwarnings('ignore', category=FutureWarning, module='tsam') | ||
| # - UserWarning: Informational message about minimal value constraints | ||
| warnings.filterwarnings('ignore', category=UserWarning, message='.*minimal value.*exceeds.*', module='tsam') | ||
| # TODO: Might be able to fix it in flixopt? |
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.
Narrow the tsam FutureWarning suppression and resolve the TODO.
The current filter on line 51 suppresses all FutureWarnings from tsam without a message pattern, which is too broad and may hide important API change warnings that users should be aware of.
The TODO comment suggests uncertainty about whether this should be fixed within flixopt itself. Before suppressing these warnings globally, investigate whether the underlying issue can be addressed in flixopt's usage of tsam.
Consider one of these approaches:
- Narrow the filter to specific known warnings:
-warnings.filterwarnings('ignore', category=FutureWarning, module='tsam')
+# Specify the exact FutureWarning message pattern(s) that should be suppressed
+warnings.filterwarnings('ignore', category=FutureWarning, message='<specific warning pattern>', module='tsam')-
Fix in flixopt if the warning indicates incorrect usage of tsam APIs (as the TODO suggests)
-
Remove the suppression and let users see these warnings if they're actionable
Since this is a major version release (v3.0.0), it's an appropriate time to address this TODO and avoid carrying forward overly broad suppressions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| warnings.filterwarnings('ignore', category=FutureWarning, module='tsam') | |
| # - UserWarning: Informational message about minimal value constraints | |
| warnings.filterwarnings('ignore', category=UserWarning, message='.*minimal value.*exceeds.*', module='tsam') | |
| # TODO: Might be able to fix it in flixopt? | |
| # Narrow the tsam FutureWarning suppression to specific known warnings only | |
| # Specify the exact FutureWarning message pattern(s) that should be suppressed | |
| warnings.filterwarnings( | |
| 'ignore', | |
| category=FutureWarning, | |
| message='<specific warning pattern>', | |
| module='tsam' | |
| ) | |
| # - UserWarning: Informational message about minimal value constraints | |
| warnings.filterwarnings( | |
| 'ignore', | |
| category=UserWarning, | |
| message='.*minimal value.*exceeds.*', | |
| module='tsam' | |
| ) | |
| # TODO: Might be able to fix it in flixopt? |
🤖 Prompt for AI Agents
In flixopt/__init__.py around lines 51-54 the code suppresses all FutureWarning
from the 'tsam' module which is too broad; investigate test runs to reproduce
the specific FutureWarning message(s) emitted by tsam, then either (A) update
the warnings.filterwarnings call to include a narrow message regex matching only
the known harmless deprecation(s) (and keep module='tsam'), or (B) if the
warning indicates incorrect usage, change the flixopt code that triggers it so
the deprecation is not emitted and remove the suppression; finally remove the
TODO comment once you either narrow the filter or fix the usage and add a brief
comment explaining the chosen approach.
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: 2
♻️ Duplicate comments (6)
CHANGELOG.md (1)
69-71: Remove stray “Until here -->” markerThis leftover marker breaks the changelog format; drop it so only the separator remains.
---- -Until here --> ## [3.0.0] - 2025-10-13flixopt/elements.py (4)
586-593: Use non-deprecatedself.investmentproperty.Line 589 references
self._investment.size, but the property at lines 695-697 marks_investmentas deprecated. Use the currentself.investmentproperty for consistency.Apply this diff:
BoundingPatterns.scaled_bounds_with_state( model=self, variable=self.flow_rate, - scaling_variable=self._investment.size, + scaling_variable=self.investment.size, relative_bounds=self.relative_flow_rate_bounds, scaling_bounds=(self.element.size.minimum_or_fixed_size, self.element.size.maximum_or_fixed_size), variable_state=self.on_off.on, )
203-209: Fix ambiguous truth-value check for xarray DataArray.
np.all(np.equal(...))on xarray returns a 0-DDataArray, causing ambiguous truth-value when used inif. Convert to Pythonbool.Apply this diff:
if self.excess_penalty_per_flow_hour is not None: - zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0)) + zero_penalty = bool(np.all(np.asarray(self.excess_penalty_per_flow_hour) == 0)) if zero_penalty: logger.warning( f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.' )
449-456: Fix ambiguous boolean and reorder conditions for efficiency.After
transform_data,self.size(when notInvestParameters) is xarrayDataArray.np.any(self.size == ...)returns 0-DDataArray, causing ambiguous truth-value. Also, reorder conditions to short-circuit.Apply this diff:
if not isinstance(self.size, InvestParameters) and ( - np.any(self.size == CONFIG.Modeling.big) and self.fixed_relative_profile is not None + self.fixed_relative_profile is not None and bool(np.any(np.asarray(self.size) == CONFIG.Modeling.big)) ): # Default Size --> Most likely by accident logger.warning( f'Flow "{self.label_full}" has no size assigned, but a "fixed_relative_profile". ' f'The default size is {CONFIG.Modeling.big}. As "flow_rate = size * fixed_relative_profile", ' f'the resulting flow_rate will be very high. To fix this, assign a size to the Flow {self}.' )
446-447: Fix ambiguous truth-value in relative bounds check.After
transform_data,relative_minimumandrelative_maximumare xarrayDataArrays. The comparison returnsDataArray, and.any()returns 0-DDataArray, causing ambiguous truth-value inif.Apply this diff:
- if (self.relative_minimum > self.relative_maximum).any(): + if bool((self.relative_minimum > self.relative_maximum).any()): raise PlausibilityError(self.label_full + ': Take care, that relative_minimum <= relative_maximum!')tests/conftest.py (1)
671-672: Derive profile lengths fromflow_system.timestepsfor robustness.Hardcoded lengths (
10) will break ifcoords_configis updated with different timestep counts. Derive the length from the constructedflow_systeminstead.Apply this diff:
+ n = len(flow_system.timesteps) - thermal_load = LoadProfiles.random_thermal(10) - p_el = LoadProfiles.random_electrical(10) + thermal_load = LoadProfiles.random_thermal(n) + p_el = LoadProfiles.random_electrical(n)
🧹 Nitpick comments (2)
CHANGELOG.md (2)
81-83: Normalize nested list indentation to 2 spaces (MD007)Several nested bullets are indented with 4 spaces; markdownlint expects 2. Adjust for consistency and tooling.
Representative fixes:
- - Control variable independence across scenarios via `scenario_independent_sizes` and `scenario_independent_flow_rates` parameters - - By default, investment sizes are shared across scenarios while flow rates vary per scenario + - Control variable independence across scenarios via `scenario_independent_sizes` and `scenario_independent_flow_rates` parameters + - By default, investment sizes are shared across scenarios while flow rates vary per scenario- - Model: The main Model (linopy.Model) that is used to create and store the variables and constraints for the flow_system. - - Submodel: The base class for all submodels. Each is a subset of the Model, for simpler access and clearer code. + - Model: The main Model (linopy.Model) that is used to create and store the variables and constraints for the flow_system. + - Submodel: The base class for all submodels. Each is a subset of the Model, for simpler access and clearer code.- - `fix_effects` → `effects_of_investment` - - `specific_effects` → `effects_of_investment_per_size` - - `divest_effects` → `effects_of_retirement` - - `piecewise_effects` → `piecewise_effects_of_investment` + - `fix_effects` → `effects_of_investment` + - `specific_effects` → `effects_of_investment_per_size` + - `divest_effects` → `effects_of_retirement` + - `piecewise_effects` → `piecewise_effects_of_investment`- - `minimum_investment` → `minimum_periodic` - - `maximum_investment` → `maximum_periodic` - - `minimum_operation` → `minimum_temporal` - - `maximum_operation` → `maximum_temporal` - - `minimum_operation_per_hour` → `minimum_per_hour` - - `maximum_operation_per_hour` → `maximum_per_hour` + - `minimum_investment` → `minimum_periodic` + - `maximum_investment` → `maximum_periodic` + - `minimum_operation` → `minimum_temporal` + - `maximum_operation` → `maximum_temporal` + - `minimum_operation_per_hour` → `minimum_per_hour` + - `maximum_operation_per_hour` → `maximum_per_hour`- - `fit_to_model_coords()` method for data alignment - - `fit_effects_to_model_coords()` method for effect data processing - - `connect_and_transform()` method replacing several operations + - `fit_to_model_coords()` method for data alignment + - `fit_effects_to_model_coords()` method for effect data processing + - `connect_and_transform()` method replacing several operations- - Updated deprecated code patterns in tests and examples (e.g., `sink`/`source` → `inputs`/`outputs`, `'H'` → `'h'` frequency) - - Refactored plotting logic to handle test environments explicitly with non-interactive backends - - Added comprehensive warning filters in `__init__.py` and `pyproject.toml` to suppress third-party library warnings - - Improved test fixtures with proper figure cleanup to prevent memory leaks - - Enhanced backend detection and handling in `plotting.py` for both Matplotlib and Plotly - - Always run dependent tests in order + - Updated deprecated code patterns in tests and examples (e.g., `sink`/`source` → `inputs`/`outputs`, `'H'` → `'h'` frequency) + - Refactored plotting logic to handle test environments explicitly with non-interactive backends + - Added comprehensive warning filters in `__init__.py` and `pyproject.toml` to suppress third-party library warnings + - Improved test fixtures with proper figure cleanup to prevent memory leaks + - Enhanced backend detection and handling in `plotting.py` for both Matplotlib and Plotly + - Always run dependent tests in orderFrom static analysis hints (MD007).
Also applies to: 127-129, 141-152, 153-158, 196-198, 200-205
221-226: Move 2.2.0 breaking bullets into a dedicated “💥 Breaking Changes” sectionRelease-note tooling keys on the 💥 header. Keep “Changed” for non-breaking items and list the two breaking bullets under a new section.
## [2.2.0] - 2025-10-11 ### ✨ Added ... ### ♻️ Changed - Logging and Configuration management changed -- **Breaking**: Console logging is now disabled by default (`CONFIG.Logging.console = False`). Enable it explicitly in your scripts with `CONFIG.Logging.console = True` and `CONFIG.apply()` -- **Breaking**: File logging is now disabled by default (`CONFIG.Logging.file = None`). Set a file path to enable file logging - Improved default logging colors: DEBUG is now gray (`\033[90m`) for de-emphasized messages, INFO uses terminal default color (`\033[0m`) for clean output + +### 💥 Breaking Changes +- Console logging is now disabled by default (`CONFIG.Logging.console = False`). Enable it explicitly with `CONFIG.Logging.console = True` and `CONFIG.apply()` +- File logging is now disabled by default (`CONFIG.Logging.file = None`). Set a file path to enable file loggingAs per prior tooling guidance in earlier reviews.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(3 hunks)flixopt/elements.py(11 hunks)tests/conftest.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
flixopt/elements.py (7)
flixopt/config.py (2)
CONFIG(61-322)Modeling(175-186)flixopt/features.py (9)
InvestmentModel(24-144)OnOffModel(147-320)size(135-137)_do_modeling(50-53)_do_modeling(176-246)_do_modeling(340-362)_do_modeling(396-450)_do_modeling(479-510)_do_modeling(545-568)flixopt/interface.py (10)
InvestParameters(672-1057)OnOffParameters(1061-1326)transform_data(81-84)transform_data(227-229)transform_data(453-455)transform_data(665-668)transform_data(930-984)transform_data(1270-1297)minimum_or_fixed_size(1041-1042)maximum_or_fixed_size(1045-1046)flixopt/modeling.py (9)
BoundingPatterns(390-759)ModelingPrimitives(197-387)ModelingUtilitiesAbstract(14-116)expression_tracking_variable(201-237)bounds_with_state(428-471)scaled_bounds(474-514)scaled_bounds_with_state(517-571)to_binary(18-52)mutual_exclusivity_constraint(345-387)flixopt/structure.py (16)
Element(856-900)ElementModel(1149-1170)FlowSystemModel(82-253)create_model(874-875)_plausibility_checks(869-872)transform_data(274-284)label_full(878-879)label_full(1017-1018)_do_modeling(1077-1079)get_coords(189-218)get_coords(986-991)hours_per_step(182-183)hours_per_step(1074-1075)add_submodels(71-79)results_structure(1165-1170)values(1137-1138)flixopt/effects.py (10)
effects(579-580)create_model(380-383)create_model(464-467)_plausibility_checks(385-387)_plausibility_checks(521-539)transform_data(340-378)_do_modeling(396-433)_do_modeling(673-686)add_share_to_effects(646-666)add_share_to_penalty(668-671)flixopt/flow_system.py (5)
create_model(466-478)FlowSystem(46-980)fit_to_model_coords(362-400)fit_effects_to_model_coords(402-425)coords(769-775)
tests/conftest.py (2)
flixopt/structure.py (3)
FlowSystemModel(82-253)values(1137-1138)weights(221-231)flixopt/core.py (3)
TimeSeriesData(42-150)aggregation_weight(109-110)aggregation_group(105-106)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
81-81: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
82-82: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
127-127: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
128-128: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
141-141: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
142-142: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
143-143: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
144-144: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
146-146: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
147-147: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
148-148: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
149-149: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
150-150: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
151-151: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
153-153: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
154-154: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
155-155: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
156-156: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
157-157: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
162-162: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
163-163: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
196-196: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
197-197: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
198-198: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
200-200: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
201-201: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
202-202: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
203-203: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
204-204: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
205-205: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (8)
tests/conftest.py (4)
80-391: LGTM! Well-structured hierarchical test library.The new hierarchical library (Buses, Effects, Converters, Storage, LoadProfiles, Sinks, Sources) centralizes reusable test components and reduces duplication across fixtures. The design properly parameterizes lengths and provides clear, discoverable test building blocks.
398-639: LGTM! Fixtures consistently leverage hierarchical library.The updated fixtures properly use the new hierarchical library classes and derive lengths from timesteps where needed. The integration maintains backward compatibility while reducing code duplication.
792-811: LGTM! Clear and helpful set comparison utility.The
assert_sets_equalfunction provides informative error messages distinguishing extra vs. missing elements, improving test debugging.
818-850: LGTM! Proper test environment configuration.The new plotting fixtures correctly configure non-interactive backends (matplotlib 'Agg', plotly 'json') and clean up figures, preventing GUI windows and memory leaks in test/CI environments.
flixopt/elements.py (4)
15-19: LGTM! Imports correctly updated.The imports properly reference:
ModelingPrimitivesfrom.modeling(line 18)InvestmentModel,OnOffModelfrom.features(line 16)FlowSystemModelfrom.structure(line 19)This aligns with the new submodel architecture.
493-720: LGTM! Well-structured FlowModel refactoring.The FlowModel refactoring properly implements the submodel architecture with:
- Clean separation of concerns (_do_modeling, _create_on_off_model, _create_investment_model, _constraint_flow_rate)
- Proper variable registration via add_variables with short_name
- Feature composition via add_submodels
- Clear property access patterns for flow_rate, total_flow_hours, on_off, investment
- Correct use of BoundingPatterns and ModelingPrimitives helpers
The design maintains maintainability and extensibility.
723-767: LGTM! BusModel properly integrates with submodel architecture.The BusModel refactoring correctly:
- Accesses flow rates via
flow.submodel.flow_rate(lines 736-737)- Creates bus balance constraints using summed flows
- Handles excess variables and penalties when configured
- Provides comprehensive results_structure with inputs, outputs, flows
The integration with the FlowModel submodels is clean and correct.
770-852: LGTM! ComponentModel orchestration well-implemented.The ComponentModel refactoring properly:
- Creates flow submodels via
flow.create_model(self._model)(line 792)- Links component-level
onwith flow-levelons using appropriate constraints (lines 795-804)- Creates component-level
OnOffModelwith correct parameters (lines 806-816)- Handles mutual exclusivity constraints (lines 820-824)
- Aggregates
previous_statesfrom flow submodels with proper padding (lines 835-852)The orchestration maintains clear separation between component and flow concerns.
| if np.any(self.relative_minimum > 0) and self.on_off_parameters is None: | ||
| logger.warning( | ||
| f'Flow {self.label} has a relative_minimum of {self.relative_minimum.active_data} and no on_off_parameters. ' | ||
| f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. ' | ||
| f'This prevents the flow_rate from switching off (flow_rate = 0). ' | ||
| f'Consider using on_off_parameters to allow the flow to be switched on and off.' | ||
| ) |
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.
Fix ambiguous boolean in relative_minimum check.
After transform_data, relative_minimum is xarray DataArray. np.any(self.relative_minimum > 0) returns 0-D DataArray, causing ambiguous truth-value in if.
Apply this diff:
- if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
+ if bool(np.any(self.relative_minimum > 0)) and self.on_off_parameters is None:
logger.warning(
f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. '
f'This prevents the flow_rate from switching off (flow_rate = 0). '
f'Consider using on_off_parameters to allow the flow to be switched on and off.'
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if np.any(self.relative_minimum > 0) and self.on_off_parameters is None: | |
| logger.warning( | |
| f'Flow {self.label} has a relative_minimum of {self.relative_minimum.active_data} and no on_off_parameters. ' | |
| f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. ' | |
| f'This prevents the flow_rate from switching off (flow_rate = 0). ' | |
| f'Consider using on_off_parameters to allow the flow to be switched on and off.' | |
| ) | |
| if bool(np.any(self.relative_minimum > 0)) and self.on_off_parameters is None: | |
| logger.warning( | |
| f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. ' | |
| f'This prevents the flow_rate from switching off (flow_rate = 0). ' | |
| f'Consider using on_off_parameters to allow the flow to be switched on and off.' | |
| ) |
🤖 Prompt for AI Agents
In flixopt/elements.py around lines 464-469, the condition uses
np.any(self.relative_minimum > 0) which can be a 0-D xarray DataArray and cause
an ambiguous truth-value error; replace that np.any(...) call with an explicit
boolean check that works for xarray/numpy, e.g. use (self.relative_minimum >
0).any().item() or self.relative_minimum.values.any() so the if becomes: if
(self.relative_minimum > 0).any().item() and self.on_off_parameters is None: and
keep the existing warning block unchanged.
| import os | ||
| from collections.abc import Iterable | ||
|
|
||
| import linopy.testing |
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.
Add missing import linopy to resolve NameError in type annotations.
Functions assert_conequal (line 736) and assert_var_equal (line 755) use linopy.Constraint and linopy.Variable in their signatures. Without import linopy or from __future__ import annotations, these annotations are evaluated at import time and will raise NameError.
Apply this diff:
import linopy.testing
+import linopy
import numpy as np📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import linopy.testing | |
| import linopy.testing | |
| import linopy | |
| import numpy as np |
🤖 Prompt for AI Agents
In tests/conftest.py around line 10, add an import for the linopy module (e.g.,
import linopy) near the top of the file so that type annotations like
linopy.Constraint and linopy.Variable used later (lines ~736 and ~755) are
available at import time; ensure the import is placed before any code that
declares or evaluates those annotations (alternatively, you could add from
__future__ import annotations at the top, but the simplest fix is to add import
linopy).
* Typos oin CHANGELOG.md * Add error handling in exmaple * Surface warnings during tests (avoid hiding deprecations) * Add missing docs file * Imrpve Releasnotes of v2.2.0 * Improve docs * Remove some filterwarnings from tsam
Description
This PR collects all changes for the v3.0.0 release.
The main changes are the addition of multi period and stochastic modeling support
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
New Features
Breaking Changes
Documentation
Chores