Improve clustering #533
Improve clustering #533
Conversation
…egration # Conflicts: # flixopt/flow_system.py
WalkthroughIntroduces a new clustering API via Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FlowSystem
participant TransformAccessor
participant ClusteringParameters
participant Clustering
participant ClusteringModel
participant Optimizer
User->>FlowSystem: transform.cluster(n_clusters, cluster_duration, ...)
FlowSystem->>TransformAccessor: cluster(keyword args)
TransformAccessor->>ClusteringParameters: Create from keywords
ClusteringParameters-->>TransformAccessor: Instance
alt Multi-dimensional clustering
TransformAccessor->>TransformAccessor: _cluster_multi_dimensional()
loop Per period/scenario
TransformAccessor->>Clustering: Compute per-slice
Clustering-->>TransformAccessor: Clustering results
end
else Single-dimension clustering
TransformAccessor->>TransformAccessor: _cluster_simple()
TransformAccessor->>Clustering: Compute for full dataset
Clustering-->>TransformAccessor: Clustering results
end
TransformAccessor->>ClusteringModel: Apply constraints (multi-dim support)
ClusteringModel->>ClusteringModel: do_modeling() with segments/flexibility
ClusteringModel-->>TransformAccessor: Constraints added
alt aggregate_data enabled
TransformAccessor->>TransformAccessor: Aggregate time-series back
end
TransformAccessor-->>FlowSystem: Clustered FlowSystem
FlowSystem-->>User: Return result
User->>Optimizer: optimize()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/notebooks/data/generate_example_systems.py (1)
246-250: Add error handling for file loading operations.Both
create_district_heating_system()andcreate_operational_system()load CSV data without error handling. If the file is missing or the date range doesn't exist, users will see cryptic pandas errors rather than helpful messages.Consider adding validation:
data_path = DATA_DIR / 'Zeitreihen2020.csv' if not data_path.exists(): raise FileNotFoundError(f"Required data file not found: {data_path}") data = pd.read_csv(data_path, index_col=0, parse_dates=True).sort_index()docs/notebooks/08a-aggregation.ipynb (1)
61-61: Consider handling the import error more gracefully.The inline import
from data.generate_example_systems import create_district_heating_systemwill raise an ImportError with a confusing message if the module doesn't exist. A more helpful error would guide users:if not data_file.exists(): try: from data.generate_example_systems import create_district_heating_system except ImportError: raise ImportError( "Example data file not found and generator module unavailable. " "Please download the example data or ensure data/generate_example_systems.py exists." ) # ... rest of generation codeflixopt/transform_accessor.py (2)
256-269: Consider extracting validation to a shared helper.The validation logic (lines 256-269) is duplicated from
_cluster_simple()(lines 186-198). Extracting this to a private method would reduce duplication and ensure consistency.def _validate_clustering_timesteps(self, params: ClusteringParameters) -> float: """Validate timesteps and return dt value.""" dt_min = float(self._fs.hours_per_timestep.min().item()) dt_max = float(self._fs.hours_per_timestep.max().item()) if dt_min != dt_max: raise ValueError(...) ratio = params.cluster_duration_hours / dt_max if not np.isclose(ratio, round(ratio), atol=1e-9): raise ValueError(...) return dt_min
363-371: Redundant keys in clustering_info dict.Both
'clustering'and'clustering_results'store the sameclustering_resultsdict. If'clustering'is required for backwards compatibility with_add_clustering_constraints, consider documenting this more explicitly or refactoring the consumer to use a single key.clustered_fs._clustering_info = { 'parameters': params, - 'clustering': clustering_results, # Required by _add_clustering_constraints - 'clustering_results': clustering_results, # Dict of Clustering objects per dimension + 'clustering': clustering_results, # Dict of Clustering objects per (period, scenario) 'components_to_clusterize': components_to_clusterize, 'original_fs': self._fs, 'has_periods': self._fs.periods is not None, 'has_scenarios': self._fs.scenarios is not None, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)docs/notebooks/08a-aggregation.ipynb(19 hunks)docs/notebooks/08b-rolling-horizon.ipynb(2 hunks)docs/notebooks/data/generate_example_systems.py(4 hunks)examples/03_Optimization_modes/example_optimization_modes.py(2 hunks)flixopt/clustering.py(5 hunks)flixopt/optimization.py(4 hunks)flixopt/transform_accessor.py(5 hunks)tests/deprecated/test_integration.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
flixopt/transform_accessor.py (2)
flixopt/flow_system.py (4)
FlowSystem(49-2225)to_dataset(581-633)copy(856-881)sel(2014-2040)flixopt/core.py (2)
drop_constant_arrays(564-594)to_dataarray(370-484)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
62-62: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
93-93: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🔇 Additional comments (22)
docs/notebooks/data/generate_example_systems.py (2)
236-348: LGTM! Well-structured helper function.The
create_district_heating_system()function is well-documented and follows consistent patterns with existing helper functions. The docstring clearly explains the purpose and usage.
351-448: LGTM! Operational system helper is well-designed.The
create_operational_system()function provides a clean example for operational planning without investments. Good separation of concerns between investment and operational optimization examples.CHANGELOG.md (1)
54-112: LGTM! Comprehensive migration documentation.The changelog clearly documents the clustering API improvements with:
- Clear before/after migration examples
- Parameter mapping table
- Usage examples for the new API
This will help users migrate smoothly from the old to the new clustering API.
flixopt/optimization.py (4)
396-402: LGTM! Clear deprecation guidance with migration example.The updated deprecation message provides a concrete example of the new API usage, making it easy for users to migrate from
ClusteredOptimizationto the newflow_system.transform.cluster()API.
444-449: LGTM! Consistent parameter naming.The internal parameter name updates (
cluster_duration_hoursand error messages) align with the public API changes documented in the CHANGELOG.
462-463: LGTM! Clustering object initialization updated correctly.The parameter names passed to the
Clusteringconstructor have been updated to match the new API (hours_per_periodandnr_of_periods), maintaining consistency with the parameter renaming.
472-472: LGTM! Simplified parameter naming.The change from
aggregate_data_and_fix_non_binary_varstoaggregate_dataimproves readability while maintaining the same functionality.tests/deprecated/test_integration.py (1)
284-293: LGTM! Test correctly updated for new clustering API.The
ClusteringParametersinitialization has been properly migrated to use the new parameter names:
n_clusters(wasnr_of_periods)cluster_duration(washours_per_period)include_storage(wasfix_storage_flows)aggregate_data(wasaggregate_data_and_fix_non_binary_vars)flexibility_percent(waspercentage_of_period_freedom)flexibility_penalty(waspenalty_of_period_freedom)This matches the migration guide in CHANGELOG.md exactly.
docs/notebooks/08b-rolling-horizon.ipynb (1)
54-60: LGTM! Clear narrative update.The markdown cell effectively communicates the shift to loading a pre-built FlowSystem, aligning with the PR's objective to adopt a FlowSystem-centric workflow.
examples/03_Optimization_modes/example_optimization_modes.py (2)
36-41: LGTM! Clear clustering configuration.The new parameter names (
n_clusters,cluster_duration,include_storage) are more intuitive than the previoushours_per_periodandnr_of_periods.
192-204: Clean migration to the new transform.cluster() API.The new API usage is clear and correctly passes the clustering configuration. The conditional peak time series assignment based on
keep_extreme_periodsis properly implemented.docs/notebooks/08a-aggregation.ipynb (2)
69-83: Visualization code looks appropriate for the notebook.The hardcoded component names ('HeatDemand', 'GridBuy') and timestep slice (672 = 7 days × 96 at 15-min resolution) are reasonable for a demonstration notebook where the example system is controlled.
360-386: Well-organized summary with clear learning outcomes.The summary effectively captures the key takeaways and appropriately defers clustering details to the dedicated notebook (08c-clustering.ipynb).
flixopt/transform_accessor.py (2)
55-172: Well-designed public API with clear documentation.The explicit parameter signature is more user-friendly than requiring a separate
ClusteringParametersobject. The docstring provides comprehensive examples covering basic clustering, segmentation, and multi-period scenarios.
174-243: Solid implementation with proper validation.The timestep consistency check and cluster duration divisibility validation are correctly implemented. The use of
np.isclosewith a tight tolerance for the ratio check is appropriate for floating-point comparison.flixopt/clustering.py (7)
89-121: Clean integration of segmentation support.The logic correctly handles the case when
nr_of_periodsisNone(segmentation-only mode) by falling back tototal_periods. Thetsamconfiguration properly setssegmentationandnoSegmentsbased on then_segmentsparameter.
285-325: Well-implemented segment equation index generation.The early return on line 300-301 when
n_segments is Nonecorrectly guards against accessingsegmentDurationDictwhen segmentation is disabled. The loop logic properly generates index pairs to equate all timesteps within a segment to the first timestep.
328-353: Clean utility function for duration parsing.Using
pd.Timedeltafor parsing duration strings is appropriate and handles standard pandas duration formats correctly.
356-454: Well-documented parameter class with clear API.The redesigned
ClusteringParametersclass provides a cleaner interface with intuitive parameter names. The convenience properties (use_extreme_periods,use_segmentation) improve code readability.
495-562: Efficient constraint generation with proper batching.Batching continuous variables into a single constraint (line 545-548) is a good optimization. Binary variables correctly get individual constraints to support the flexibility correction variables.
653-667: Correct penalty application for flexibility deviations.The method properly sums correction variables over the appropriate dimension (
eq_idxfor equality constraints ortimeas fallback) and adds the penalty to the objective viaPENALTY_EFFECT_LABEL.
623-628: Theself._model.variables.binariesattribute is a valid linopy API feature and requires no changes.The
.binariesproperty is a documented feature of linopy's Variables class, so the membership check on line 625 is correct and aligns with linopy's standard usage patterns.
| } | ||
| }, | ||
| "source": "# Load time series data (15-min resolution)\ndata = pd.read_csv('../../examples/resources/Zeitreihen2020.csv', index_col=0, parse_dates=True).sort_index()\ndata = data['2020-01-01':'2020-01-14 23:45:00'] # Two weeks\ndata.index.name = 'time' # Rename index for consistency\n\ntimesteps = data.index\n\n# Extract profiles\nelectricity_demand = data['P_Netz/MW'].to_numpy()\nheat_demand = data['Q_Netz/MW'].to_numpy()\nelectricity_price = data['Strompr.€/MWh'].to_numpy()\ngas_price = data['Gaspr.€/MWh'].to_numpy()\n\nprint(f'Timesteps: {len(timesteps)} ({len(timesteps) / 96:.0f} days at 15-min resolution)')\nprint(f'Heat demand: {heat_demand.min():.1f} - {heat_demand.max():.1f} MW')\nprint(f'Electricity price: {electricity_price.min():.1f} - {electricity_price.max():.1f} €/MWh')", | ||
| "source": "from pathlib import Path\n\n# Generate example data if not present (for local development)\ndata_file = Path('data/operational_system.nc4')\nif not data_file.exists():\n from data.generate_example_systems import create_operational_system\n\n fs = create_operational_system()\n fs.optimize(fx.solvers.HighsSolver(log_to_console=False))\n fs.to_netcdf(data_file, overwrite=True)\n\n# Load the operational system (real data from Zeitreihen2020.csv, two weeks)\nflow_system = fx.FlowSystem.from_netcdf(data_file)\n\ntimesteps = flow_system.timesteps\nprint(f'Loaded FlowSystem: {len(timesteps)} timesteps ({len(timesteps) / 96:.0f} days at 15-min resolution)')\nprint(f'Components: {list(flow_system.components.keys())}')", |
There was a problem hiding this comment.
Ensure parent directory exists before writing NetCDF.
If the data/ directory doesn't exist, fs.to_netcdf(data_file, overwrite=True) will fail with a file system error. Additionally, there's no error handling for file I/O operations or optimization failures, which could lead to confusing errors for users.
Apply this diff to create the directory and add basic error handling:
from pathlib import Path
# Generate example data if not present (for local development)
data_file = Path('data/operational_system.nc4')
if not data_file.exists():
+ # Ensure parent directory exists
+ data_file.parent.mkdir(parents=True, exist_ok=True)
+
from data.generate_example_systems import create_operational_system
+ print("Generating example data (this may take a moment)...")
fs = create_operational_system()
fs.optimize(fx.solvers.HighsSolver(log_to_console=False))
fs.to_netcdf(data_file, overwrite=True)
+ print(f"Example data saved to {data_file}")
-# Load the operational system (real data from Zeitreihen2020.csv, two weeks)
+# Load the operational system
flow_system = fx.FlowSystem.from_netcdf(data_file)
timesteps = flow_system.timesteps
print(f'Loaded FlowSystem: {len(timesteps)} timesteps ({len(timesteps) / 96:.0f} days at 15-min resolution)')
print(f'Components: {list(flow_system.components.keys())}')Note: The comment mentioning "real data from Zeitreihen2020.csv" is misleading since the code may generate synthetic data if the file doesn't exist.
📝 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.
| "source": "from pathlib import Path\n\n# Generate example data if not present (for local development)\ndata_file = Path('data/operational_system.nc4')\nif not data_file.exists():\n from data.generate_example_systems import create_operational_system\n\n fs = create_operational_system()\n fs.optimize(fx.solvers.HighsSolver(log_to_console=False))\n fs.to_netcdf(data_file, overwrite=True)\n\n# Load the operational system (real data from Zeitreihen2020.csv, two weeks)\nflow_system = fx.FlowSystem.from_netcdf(data_file)\n\ntimesteps = flow_system.timesteps\nprint(f'Loaded FlowSystem: {len(timesteps)} timesteps ({len(timesteps) / 96:.0f} days at 15-min resolution)')\nprint(f'Components: {list(flow_system.components.keys())}')", | |
| from pathlib import Path | |
| # Generate example data if not present (for local development) | |
| data_file = Path('data/operational_system.nc4') | |
| if not data_file.exists(): | |
| # Ensure parent directory exists | |
| data_file.parent.mkdir(parents=True, exist_ok=True) | |
| from data.generate_example_systems import create_operational_system | |
| print("Generating example data (this may take a moment)...") | |
| fs = create_operational_system() | |
| fs.optimize(fx.solvers.HighsSolver(log_to_console=False)) | |
| fs.to_netcdf(data_file, overwrite=True) | |
| print(f"Example data saved to {data_file}") | |
| # Load the operational system | |
| flow_system = fx.FlowSystem.from_netcdf(data_file) | |
| timesteps = flow_system.timesteps | |
| print(f'Loaded FlowSystem: {len(timesteps)} timesteps ({len(timesteps) / 96:.0f} days at 15-min resolution)') | |
| print(f'Components: {list(flow_system.components.keys())}') |
| class ClusteredResult: | ||
| def __init__(self, name, fs): | ||
| self.name = name | ||
| self.flow_system = fs | ||
| self.durations = {'total': 0} # Placeholder | ||
|
|
||
| optimization = ClusteredResult('Clustered', clustered_fs) | ||
| optimizations.append(optimization) |
There was a problem hiding this comment.
The durations placeholder will show misleading 0 seconds in the comparison chart.
Line 250 uses calc.durations for the speed comparison. This placeholder returns {'total': 0}, making the clustered optimization appear to take no time.
Consider capturing the actual duration:
+ import timeit
+ start = timeit.default_timer()
clustered_fs = flow_system.copy().transform.cluster(
n_clusters=n_clusters,
cluster_duration=cluster_duration,
include_storage=include_storage,
time_series_for_high_peaks=time_series_for_high_peaks,
time_series_for_low_peaks=time_series_for_low_peaks,
)
clustered_fs.optimize(fx.solvers.HighsSolver(0.01 / 100, 60))
+ elapsed = timeit.default_timer() - start
# Wrap in a simple object for compatibility with comparison code
class ClusteredResult:
def __init__(self, name, fs):
self.name = name
self.flow_system = fs
- self.durations = {'total': 0} # Placeholder
+ self.durations = {'total': elapsed}
optimization = ClusteredResult('Clustered', clustered_fs)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/03_Optimization_modes/example_optimization_modes.py around lines 207
to 214, the ClusteredResult currently sets durations to a hardcoded {'total':
0}, which misrepresents timing in the comparison chart; replace the placeholder
by measuring the actual runtime of the clustered optimization (e.g., record
start = time.perf_counter() before running the clustered flow system, run the
optimization, record end = time.perf_counter() afterwards) and set
optimization.durations['total'] = end - start (and any other duration keys to
match the shape used elsewhere) so the plotted speed comparison uses real
elapsed seconds.
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
New Features
Breaking Changes
hours_per_period→cluster_duration,nr_of_periods→n_clusters,fix_storage_flows→include_storage, and additional semantic updates.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.