Skip to content

Conversation

@FBumann
Copy link
Member

@FBumann FBumann commented Jan 3, 2026

Summary

Adds a new Comparison class for comparing multiple FlowSystems side-by-side with automatic faceting.

New Feature: fx.Comparison

Compare optimized FlowSystems with unified data access and plotting:

Compare two systems

comp = fx.Comparison([fs_base, fs_modified])

Side-by-side plots (auto-facets by 'case')

comp.statistics.plot.balance('Heat')
comp.statistics.plot.effects()

Access combined data with 'case' dimension

comp.solution # xr.Dataset
comp.statistics.flow_rates # xr.Dataset

Compute differences

comp.diff() # Relative to first case
comp.diff('baseline') # Or specify reference

Features

  • Unified data: Combines solutions into xr.Dataset with case dimension
  • All plot methods: balance, carrier_balance, flows, storage, charge_states, duration_curve, sizes, effects, heatmap
  • Automatic faceting: Plots automatically facet by case for side-by-side comparison
  • Difference computation: comp.diff() computes differences relative to a reference case
  • Dimension validation: Ensures FlowSystems have matching dimensions

Additional Improvements

  • Simplified _prepare_for_heatmap() helper in statistics_accessor
  • Streamlined effects() method - works directly with Dataset, returns effects as data variables
  • Fixed heatmap handling for data with extra singleton dimensions

Summary by CodeRabbit

  • New Features

    • Compare multiple flow systems side-by-side with automatic visualization and difference calculations.
    • Time-series clustering with configurable storage modes (intercluster_cyclic, intercluster, cyclic, independent) for advanced optimization workflows.
    • Universal plotting accessor with automatic faceting and duration curve transformations.
    • Name flow systems for clearer comparisons and result interpretation.
  • Documentation

    • Added comprehensive clustering workflow guides, storage mode definitions, and comparison examples across tutorials.

✏️ Tip: You can customize this high-level summary in your review settings.

FBumann added 20 commits January 1, 2026 21:30
…ation, reducing code duplication while maintaining the same functionality (data preparation, color resolution from components, PlotResult wrapping).
… area(), and duration_curve() methods in both DatasetPlotAccessor and DataArrayPlotAccessor

  2. scatter() method - Plots two variables against each other with x and y parameters
  3. pie() method - Creates pie charts from aggregated (scalar) dataset values, e.g. ds.sum('time').fxplot.pie()
  4. duration_curve() method - Sorts values along the time dimension in descending order, with optional normalize parameter for percentage x-axis
  5. CONFIG.Plotting.default_line_shape - New config option (default 'hv') that controls the default line shape for line(), area(), and duration_curve() methods
  1. X-axis is now determined first using CONFIG.Plotting.x_dim_priority
  2. Facets are resolved from remaining dimensions (x-axis excluded)

  x_dim_priority expanded:
  x_dim_priority = ('time', 'duration', 'duration_pct', 'period', 'scenario', 'cluster')
  - Time-like dims first, then common grouping dims as fallback
  - variable stays excluded (it's used for color, not x-axis)

  _get_x_dim() refactored:
  - Now takes dims: list[str] instead of a DataFrame
  - More versatile - works with any list of dimension names
  - Add `x` parameter to bar/stacked_bar/line/area for explicit x-axis control
  - Add CONFIG.Plotting.x_dim_priority for auto x-axis selection order
  - X-axis determined first, facets from remaining dimensions
  - Refactor _get_x_column -> _get_x_dim (takes dim list, not DataFrame)
  - Support scalar data (no dims) by using 'variable' as x-axis
  - Add `x` parameter to bar/stacked_bar/line/area for explicit x-axis control
  - Add CONFIG.Plotting.x_dim_priority for auto x-axis selection
    Default: ('time', 'duration', 'duration_pct', 'period', 'scenario', 'cluster')
  - X-axis determined first, facets resolved from remaining dimensions
  - Refactor _get_x_column -> _get_x_dim (takes dim list, more versatile)
  - Support scalar data (no dims) by using 'variable' as x-axis
  - Skip color='variable' when x='variable' to avoid double encoding
  - Fix _dataset_to_long_df to use dims (not just coords) as id_vars
  - Add `x` parameter to bar/stacked_bar/line/area for explicit x-axis control
  - Add CONFIG.Plotting.x_dim_priority for auto x-axis selection
    Default: ('time', 'duration', 'duration_pct', 'period', 'scenario', 'cluster')
  - X-axis determined first, facets resolved from remaining dimensions
  - Refactor _get_x_column -> _get_x_dim (takes dim list, more versatile)
  - Support scalar data (no dims) by using 'variable' as x-axis
  - Skip color='variable' when x='variable' to avoid double encoding
  - Fix _dataset_to_long_df to use dims (not just coords) as id_vars
  - Ensure px_kwargs properly overrides all defaults (color, facets, etc.)
…wargs} so user can override

  2. scatter unused colors - Removed the unused parameter
  3. to_duration_curve sorting - Changed [::-1] to np.flip(..., axis=time_axis) for correct multi-dimensional handling
  4. DataArrayPlotAccessor.heatmap - Same kwarg merge fix
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 3, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Introduces a new Comparison class enabling side-by-side analysis of multiple FlowSystems through unified statistics aggregation and plotting, complemented by refactored statistics accessor logic and comprehensive documentation updates across multiple notebooks demonstrating the comparison workflow.

Changes

Cohort / File(s) Summary
Core Comparison Implementation
flixopt/comparison.py, flixopt/__init__.py
New Comparison class validates input FlowSystems, aggregates statistics across cases, provides diff() computation relative to reference case, and delegates plotting via ComparisonStatistics. Exports new Comparison class from package.
Statistics Accessor Refactoring
flixopt/statistics_accessor.py
Refactored effects() method to use dictionary mapping for aspect selection, consolidates grouping logic, unifies dataset operations, and switches plotting to fxplot-based pipeline. Returns PlotResult wrapper.
Documentation & User Guide
CHANGELOG.md, docs/user-guide/results/index.md
Added comprehensive changelog entries describing Comparison features, time-series clustering, storage modes, FXPlot accessor, and detailed examples. New user-guide section documenting Comparison workflow, properties, plotting, and diff computation.
Notebook Updates: Heat System & Investment
docs/notebooks/02-heat-system.ipynb, docs/notebooks/03-investment-optimization.ipynb
Replaced textual print outputs with pandas DataFrame-based summaries. Added pandas imports. Removed kernel/metadata blocks.
Notebook Updates: Operational Constraints & Multi-Carrier
docs/notebooks/04-operational-constraints.ipynb, docs/notebooks/05-multi-carrier-system.ipynb
Added FlowSystem name parameter usage. Introduced fx.Comparison side-by-side comparisons with plotting. Replaced print-based outputs with DataFrame structures. Updated metadata.
Notebook Updates: Time-varying Parameters, Scenarios, & Aggregation
docs/notebooks/06a-time-varying-parameters.ipynb, docs/notebooks/07-scenarios-and-periods.ipynb, docs/notebooks/08a-aggregation.ipynb
Replaced Plotly-based visualization with xarray FXPlot. Added FlowSystem naming and Comparison-based side-by-side comparisons. Converted reporting to DataFrame outputs. Updated cell IDs and metadata.
Notebook Updates: Rolling Horizon, Clustering & Multi-period
docs/notebooks/08b-rolling-horizon.ipynb, docs/notebooks/08c-clustering.ipynb, docs/notebooks/08c2-clustering-storage-modes.ipynb, docs/notebooks/08d-clustering-multiperiod.ipynb
Added FlowSystem.name assignments for workflow labeling. Introduced fx.Comparison for side-by-side comparisons. Replaced print-based metrics with DataFrame-based assessments. Added solution expansion steps with naming. Updated cell structure and IDs.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/Notebook
    participant Comp as Comparison
    participant FSList as FlowSystem List
    participant Stats as ComparisonStatistics
    participant Plotter as ComparisonStatisticsPlot
    participant Plot as Plotting Output

    User->>Comp: __init__(flow_systems, names)
    Comp->>Comp: Validate dimensions & duplicates
    Comp->>FSList: Aggregate solutions & combine
    Comp->>Comp: Create 'case' dimension
    User->>Comp: .solution
    Comp-->>User: Combined xarray Dataset
    User->>Comp: .statistics
    Comp->>Stats: Initialize with Comparison
    Stats->>FSList: Extract & cache per-system stats
    Stats->>Stats: Concatenate across cases
    Stats-->>Comp: ComparisonStatistics instance
    User->>Stats: .flow_rates / .sizes / .charge_states
    Stats-->>User: Aggregated Dataset with 'case' dim
    User->>Stats: .plot.balance(...)
    Stats->>Plotter: Initialize plotter
    Plotter->>FSList: Delegate to per-system plotters
    Plotter->>Plotter: Concatenate results by 'case'
    Plotter->>Plot: Combine into faceted figure
    Plot-->>User: Multi-case visualization
    User->>Comp: .diff(reference='case_name')
    Comp->>Comp: Compute differences vs reference
    Comp-->>User: Difference Dataset
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

v3.0.0

Suggested reviewers

  • baumbude
  • PStange

Poem

🐰 A Comparison hops into view,
Side-by-side, we showcase what's new,
Statistics dance, plots align,
Multiple systems by design—
Faceted beauty, diff in tow,
Watch your FlowSystems steal the show! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides a clear summary of the feature and examples, but does not follow the provided template structure with required sections. Restructure description to match template: add 'Type of Change' checkboxes, 'Related Issues' section, 'Testing' section, and complete the checklist items.
Title check ❓ Inconclusive The title 'Feature/comparison' is vague and uses generic formatting that lacks specificity about the main change. Use a more descriptive title such as 'Add Comparison class for side-by-side FlowSystem analysis' to clearly convey the primary feature.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

  1. __getattr__ - dynamically delegates any method to the underlying accessor
  2. _wrap_plot_method - single method that handles all the data collection and concatenation
  3. _recreate_figure - infers plot type from the original figure and recreates with combined data

  Tradeoffs:
  - Less explicit type hints on method signatures (but still works the same)
  - Infers plot type from original figure rather than hardcoding per method
  - Automatically supports any new methods added to StatisticsPlotAccessor in the future
  1. __getattr__ - dynamically delegates any method to the underlying accessor
  2. _wrap_plot_method - single method that handles all the data collection and concatenation
  3. _recreate_figure - infers plot type from the original figure and recreates with combined data

  Tradeoffs:
  - Less explicit type hints on method signatures (but still works the same)
  - Infers plot type from original figure rather than hardcoding per method
  - Automatically supports any new methods added to StatisticsPlotAccessor in the future
…igure creation. The _DATA_KWARGS mapping defines which kwargs affect data processing - everything else passes through to plotly.
…igure creation. The _DATA_KWARGS mapping defines which kwargs affect data processing - everything else passes through to plotly.
…igure creation. The _DATA_KWARGS mapping defines which kwargs affect data processing - everything else passes through to plotly.
FBumann added 19 commits January 4, 2026 22:16
# Conflicts:
#	.github/workflows/docs.yaml
#	mkdocs.yml
…mmary of the changes made:

  Changes Made

  1. flixopt/config.py

  - Replaced three separate config attributes (extra_dim_priority, dim_slot_priority, x_dim_priority) with a single unified dim_priority tuple
  - Updated CONFIG.Plotting class docstring and attribute definitions
  - Updated to_dict() method to use the new attribute
  - The new priority order: ('time', 'duration', 'duration_pct', 'variable', 'cluster', 'period', 'scenario')

  2. flixopt/dataset_plot_accessor.py

  - Created new assign_slots() function that centralizes all dimension-to-slot assignment logic
  - Fixed slot fill order: x → color → facet_col → facet_row → animation_frame
  - Updated all plot methods (bar, stacked_bar, line, area, heatmap, scatter, pie) to use assign_slots()
  - Removed old _get_x_dim() and _resolve_auto_facets() functions
  - Updated docstrings to reference dim_priority instead of x_dim_priority

  3. flixopt/statistics_accessor.py

  - Updated _resolve_auto_facets() to use the new assign_slots() function internally
  - Added import for assign_slots from dataset_plot_accessor

  Key Design Decisions

  - Single priority list controls all auto-assignment
  - Slots are filled in fixed order based on availability
  - None means a slot is not available for that plot type
  - 'auto' triggers auto-assignment from priority list
  - Explicit string values override auto-assignment
…e/comparison

# Conflicts:
#	docs/notebooks/08c-clustering.ipynb
#	flixopt/config.py
…e/comparison

# Conflicts:
#	docs/notebooks/08c-clustering.ipynb
#	flixopt/config.py
  1. Removed _resolve_facets method - fxplot handles 'auto' resolution internally
  2. Updated all methods to pass facet params directly to fxplot
  3. sizes now uses ds.fxplot.bar() instead of px.bar
  4. effects now uses ds.fxplot.bar() with proper column naming

  statistics_accessor.py:
  1. Simplified effects method significantly:
    - Works directly with Dataset (no DataArray concat/conversion)
    - Uses dict.get for aspect selection
    - Cleaner aggregation logic
    - Returns Dataset with effects as data variables
    - Uses fxplot.bar instead of px.bar

  The code is now consistent - all plotting methods in both StatisticsPlotAccessor and ComparisonStatisticsPlot use fxplot for centralized dimension/slot handling.
# Conflicts:
#	docs/notebooks/02-heat-system.ipynb
#	docs/notebooks/03-investment-optimization.ipynb
#	docs/notebooks/04-operational-constraints.ipynb
#	docs/notebooks/05-multi-carrier-system.ipynb
#	docs/notebooks/06a-time-varying-parameters.ipynb
#	docs/notebooks/07-scenarios-and-periods.ipynb
#	docs/notebooks/08c-clustering.ipynb
#	flixopt/config.py
#	flixopt/statistics_accessor.py
@FBumann
Copy link
Member Author

FBumann commented Jan 5, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

✅ Actions performed

Review triggered.

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Fix all issues with AI Agents 🤖
In @docs/notebooks/08a-aggregation.ipynb:
- Line 276: Remove the stray XML tag artifact "<cell_type>markdown</cell_type>"
from the markdown cell so the header renders correctly; replace the entire line
that currently reads "<cell_type>markdown</cell_type>## Visual Comparison: Heat
Balance" with a plain markdown header "## Visual Comparison: Heat Balance"
(ensure no surrounding XML fragments remain in the notebook cell content).

In @flixopt/comparison.py:
- Around line 70-76: In __init__, ensure FlowSystem.name may be None: when names
is None, build self._names by mapping each FlowSystem in flow_systems to a
non-None fallback (e.g., use fs.name if truthy else f"flow_system_{index}" or
another deterministic fallback like fs.__class__.__name__ + f"_{index}"), then
validate uniqueness of the resulting self._names (raise ValueError if
duplicates). Update the code that currently sets self._names = names or [fs.name
for fs in flow_systems] to perform this defaulting and uniqueness check (refer
to the __init__ method and self._names symbol to locate the change).
🧹 Nitpick comments (1)
flixopt/comparison.py (1)

354-365: Silent exception handling may hide legitimate errors.

The except (KeyError, ValueError): continue on lines 359-360 silently skips systems when data extraction fails. While this allows partial data when systems have different components, it could also hide genuine configuration errors or bugs.

Consider logging a warning when a system is skipped so users are aware that not all cases are included in the plot.

🔎 Suggested improvement
+import logging
+logger = logging.getLogger('flixopt')
+
         for fs, case_name in zip(self._comp._systems, self._comp._names, strict=True):
             try:
                 result = getattr(fs.statistics.plot, method_name)(*args, **kwargs)
                 datasets.append(result.data.expand_dims(case=[case_name]))
                 title = result.figure.layout.title.text or title
             except (KeyError, ValueError):
+                logger.debug(f"Skipping case '{case_name}' for {method_name}: data not available")
                 continue
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f1d827 and e4c6510.

📒 Files selected for processing (16)
  • CHANGELOG.md
  • docs/notebooks/02-heat-system.ipynb
  • docs/notebooks/03-investment-optimization.ipynb
  • docs/notebooks/04-operational-constraints.ipynb
  • docs/notebooks/05-multi-carrier-system.ipynb
  • docs/notebooks/06a-time-varying-parameters.ipynb
  • docs/notebooks/07-scenarios-and-periods.ipynb
  • docs/notebooks/08a-aggregation.ipynb
  • docs/notebooks/08b-rolling-horizon.ipynb
  • docs/notebooks/08c-clustering.ipynb
  • docs/notebooks/08c2-clustering-storage-modes.ipynb
  • docs/notebooks/08d-clustering-multiperiod.ipynb
  • docs/user-guide/results/index.md
  • flixopt/__init__.py
  • flixopt/comparison.py
  • flixopt/statistics_accessor.py
🧰 Additional context used
🧬 Code graph analysis (1)
flixopt/__init__.py (1)
flixopt/comparison.py (1)
  • Comparison (23-165)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

65-65: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)

⏰ 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: Build documentation
🔇 Additional comments (30)
docs/notebooks/06a-time-varying-parameters.ipynb (1)

311-311: Verify notebook metadata stripping intent and implications.

Line 311 shows the notebook metadata reduced to {} (empty). Per the AI summary, this removes kernelspec and language_info sections. While consistent with broader PR-wide notebook refactoring, confirm:

  1. This metadata removal is intentional as part of a documentation consistency effort (not accidental).
  2. Removing kernelspec and language_info does not break notebook execution or CI/documentation pipeline assumptions (e.g., if Jupyter Book, JupyterHub, or doc-build tools rely on these fields).
docs/notebooks/02-heat-system.ipynb (1)

35-35: LGTM! Clean migration to structured data presentation.

The pandas import and DataFrame-based output improve readability and align with the comparison-oriented workflows introduced in this PR.

Also applies to: 287-294

docs/notebooks/08c-clustering.ipynb (3)

104-104: LGTM! Consistent FlowSystem labeling for comparison workflows.

Setting explicit names on FlowSystem instances enables clear identification in the new Comparison API.

Also applies to: 146-146, 306-306, 355-355


254-260: LGTM! Structured metrics comparison.

The DataFrame-based comparison of clustering algorithms is more readable than the previous print-based approach.


311-312: LGTM! Appropriate use of the Comparison class.

Demonstrates comparing two FlowSystem configurations (clustered vs. modified clustered) with the new Comparison API.

flixopt/__init__.py (1)

20-20: LGTM! Properly exposes the new Comparison class.

The import and all export correctly add the Comparison class to the public API.

Also applies to: 43-43

docs/notebooks/08b-rolling-horizon.ipynb (2)

97-97: LGTM! Consistent labeling for workflow variants.

Setting names on the FlowSystem copies clearly distinguishes the "Full Optimization" and "Rolling Horizon" approaches.

Also applies to: 137-137


184-197: LGTM! Effective use of Comparison for side-by-side visualization.

Consolidates two separate balance plots into a single, unified comparison view using the new Comparison class.

docs/notebooks/08d-clustering-multiperiod.ipynb (3)

143-143: LGTM! Consistent FlowSystem labeling.

The names clearly identify the optimization methods being compared.

Also applies to: 349-349


439-448: LGTM! Demonstrates side-by-side comparison in multi-period context.

Shows how the Comparison class can be used to compare full optimization vs. two-stage workflow results with automatic faceting by period and scenario.


467-471: LGTM! Demonstrates solution expansion workflow.

Shows how to map clustered solutions back to full resolution using expand_solution(), which is valuable for understanding the expanded timestep mapping.

docs/notebooks/08a-aggregation.ipynb (2)

175-175: Good practice: naming FlowSystem for comparison clarity.

Setting fs_dispatch.name = 'Two-Stage' ensures the system is clearly labeled in subsequent comparison plots and outputs.


288-290: Effective use of the new Comparison API.

The side-by-side comparison workflow is concise and aligns with the new Comparison class capabilities described in the PR objectives.

docs/notebooks/08c2-clustering-storage-modes.ipynb (2)

145-145: Consistent FlowSystem naming pattern.

The naming workflow (fs_full.name = 'Full Optimization' and fs_expanded.name = f'Mode: {mode}') aligns with the comparison-based approach introduced across the notebook ecosystem.

Also applies to: 281-283


335-337: Clear demonstration of recommended storage mode.

The comparison between full optimization and the intercluster_cyclic mode effectively demonstrates the recommended approach for seasonal storage systems.

docs/user-guide/results/index.md (1)

280-428: Comprehensive documentation for the Comparison API.

The new section is well-structured, covering:

  • Clear basic usage examples
  • Explicit dimension matching requirements
  • Complete property and method listings
  • Practical comparison scenarios

The examples are runnable and align with the notebook usage patterns demonstrated elsewhere in the PR.

docs/notebooks/03-investment-optimization.ipynb (1)

233-240: Improved output presentation with DataFrames.

Converting print-based outputs to pandas DataFrames provides:

  • Structured, tabular presentation
  • Better readability for single-row results (via .T)
  • Professional appearance consistent with data analysis best practices

These changes enhance the notebook's pedagogical value.

Also applies to: 283-290, 348-362

docs/notebooks/07-scenarios-and-periods.ipynb (2)

103-115: Effective xarray-based visualization of scenario data.

The approach of constructing an xarray Dataset from scenario-specific demand profiles and rendering with fxplot.line aligns with the project's shift toward xarray-based visualization, providing a clean and maintainable pattern for multi-scenario data.


273-280: Consistent DataFrame-based output improvements.

Converting results to pandas DataFrames (optimal sizing, CHP operation summary, sensitivity analysis) provides structured, professional output formatting that enhances the notebook's clarity and aligns with data analysis best practices.

Also applies to: 354-362, 389-395

docs/notebooks/05-multi-carrier-system.ipynb (1)

35-40: LGTM! Well-structured demonstration of the new Comparison API.

The notebook updates effectively showcase:

  • FlowSystem naming with name='With CHP' and name='No CHP'
  • Clean DataFrame-based summary outputs for comparison
  • Side-by-side visualization using fx.Comparison

The progression from individual system analysis to comparison is pedagogically sound.

Also applies to: 130-130, 335-348, 369-369, 413-426, 446-458

docs/notebooks/04-operational-constraints.ipynb (1)

35-41: LGTM! Consistent updates demonstrating the Comparison workflow.

The notebook appropriately demonstrates:

  • Named FlowSystem instances for clearer comparison
  • DataFrame-based cost summaries
  • comp.statistics.plot.effects() for side-by-side effect visualization

Also applies to: 112-112, 276-282, 333-333, 363-372, 392-394

flixopt/statistics_accessor.py (3)

1869-1875: LGTM on the dictionary-based aspect selection.

Using a dictionary with .get() is cleaner than the previous approach. The None check with explicit error message provides clear feedback for invalid aspects.


1891-1916: Dimension handling in by parameter logic looks correct.

The logic correctly:

  • Sums over all groupable dimensions when by=None
  • Preserves the appropriate dimension for by='component', by='contributor', or by='time'
  • Validates that time dimension exists before allowing by='time'

1886-1889: Dimension state after groupby is incorrectly described; selection filtering provides safety.

After ds.groupby('component').sum() at line 1887, 'component' remains as a dimension (it's the grouping key), while 'contributor' is removed (summed). The concern about component becoming unavailable is reversed. However, if select contains a 'contributor' key, it will be silently ignored after groupby removes that dimension—though _apply_selection filters to only valid dimensions, so this is handled gracefully. If intentional (preventing selection on aggregated dimensions), this is fine; otherwise, document the expected behavior for the select parameter with respect to by.

CHANGELOG.md (1)

63-89: LGTM! Comprehensive changelog entry for the new Comparison feature.

The documentation clearly covers:

  • Constructor options (default names vs custom names)
  • Side-by-side plotting with automatic faceting
  • Combined data access via comp.solution and comp.statistics.*
  • Diff computation with reference case selection
  • Complete list of mirrored properties and methods

The static analysis hint about code block style (fenced vs indented) can be safely ignored—fenced code blocks are the preferred style for this changelog format.

flixopt/comparison.py (5)

97-123: LGTM! Well-designed dimension validation.

The validation:

  • Only checks core dimensions (time, period, scenario), correctly ignoring auxiliary dimensions like cluster_boundary
  • Provides clear error messages indicating which dimensions are missing or extra
  • Suggests using .transform.sel() to align dimensions

This is a thoughtful design that allows flexibility while ensuring data compatibility.


148-165: LGTM! Clean diff() implementation.

The method:

  • Accepts either string name or integer index for flexibility
  • Validates that string references exist in names
  • Uses xarray broadcasting correctly to subtract reference from all cases

196-209: LGTM on helper methods for data aggregation.

_concat_property and _merge_dict_property provide clean abstractions for combining data across cases. The documented behavior of later cases overriding earlier ones in _merge_dict_property is reasonable for color/unit merging.


317-346: LGTM on kwargs splitting approach.

The _DATA_KWARGS dictionary and _split_kwargs method provide a clean separation between data-processing arguments and plotly styling arguments. This allows the wrapper to pass data kwargs to the underlying methods while forwarding plotly kwargs to the figure creation.


1-21: LGTM! Clean module setup with appropriate exports.

The module correctly:

  • Uses __all__ to export only Comparison
  • Defines type aliases matching statistics_accessor.py
  • Uses TYPE_CHECKING for the FlowSystem import to avoid circular imports

"metadata": {},
"source": [
"## Visual Comparison: Heat Balance"
"<cell_type>markdown</cell_type>## Visual Comparison: Heat Balance\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix corrupted markdown text.

The line contains an XML tag artifact <cell_type>markdown</cell_type> that will render incorrectly in the notebook. This appears to be a merge/edit error.

🔎 Proposed fix
-    "<cell_type>markdown</cell_type>## Visual Comparison: Heat Balance\n",
+    "## Visual Comparison: Heat Balance\n",
📝 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.

Suggested change
"<cell_type>markdown</cell_type>## Visual Comparison: Heat Balance\n",
"## Visual Comparison: Heat Balance\n",
🤖 Prompt for AI Agents
In @docs/notebooks/08a-aggregation.ipynb around line 276, Remove the stray XML
tag artifact "<cell_type>markdown</cell_type>" from the markdown cell so the
header renders correctly; replace the entire line that currently reads
"<cell_type>markdown</cell_type>## Visual Comparison: Heat Balance" with a plain
markdown header "## Visual Comparison: Heat Balance" (ensure no surrounding XML
fragments remain in the notebook cell content).

Comment on lines 70 to 76
def __init__(self, flow_systems: list[FlowSystem], names: list[str] | None = None) -> None:
if len(flow_systems) < 2:
raise ValueError('Comparison requires at least 2 FlowSystems')

self._systems = flow_systems
self._names = names or [fs.name for fs in flow_systems]

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential issue: FlowSystem.name might be None, leading to None in names list.

If names is not provided and a FlowSystem has name=None, line 75 will include None in the names list. This could cause issues:

  • The uniqueness check (line 82) would pass with multiple None values if only one FS has no name
  • Using None as a coordinate value in xarray can cause unexpected behavior

Consider adding validation or defaulting to a fallback name pattern.

🔎 Suggested fix
-        self._names = names or [fs.name for fs in flow_systems]
+        self._names = names or [fs.name or f'Case {i}' for i, fs in enumerate(flow_systems)]
🤖 Prompt for AI Agents
In @flixopt/comparison.py around lines 70-76, In __init__, ensure
FlowSystem.name may be None: when names is None, build self._names by mapping
each FlowSystem in flow_systems to a non-None fallback (e.g., use fs.name if
truthy else f"flow_system_{index}" or another deterministic fallback like
fs.__class__.__name__ + f"_{index}"), then validate uniqueness of the resulting
self._names (raise ValueError if duplicates). Update the code that currently
sets self._names = names or [fs.name for fs in flow_systems] to perform this
defaulting and uniqueness check (refer to the __init__ method and self._names
symbol to locate the change).

…own</cell_type> tag from markdown source

  2. flixopt/comparison.py line 75: Added fallback for None names:
  # Before
  self._names = names or [fs.name for fs in flow_systems]

  # After
  self._names = names or [fs.name or f'System {i}' for i, fs in enumerate(flow_systems)]
@FBumann FBumann merged commit 5ea8651 into feature/aggregate-rework-v2 Jan 5, 2026
9 checks passed
FBumann added a commit that referenced this pull request Jan 5, 2026
Add comparison functionality for analyzing multiple FlowSystem solutions:
- Compare results across different scenarios
- Visualization support for comparison data

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@FBumann FBumann mentioned this pull request Jan 5, 2026
FBumann added a commit that referenced this pull request Jan 6, 2026
Add comparison functionality for analyzing multiple FlowSystem solutions:
- Compare results across different scenarios
- Visualization support for comparison data

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
FBumann added a commit that referenced this pull request Jan 6, 2026
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
FBumann added a commit that referenced this pull request Jan 6, 2026
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants