Skip to content

fix: allow expand() on clustered FlowSystem without a solution#646

Merged
FBumann merged 3 commits intomainfrom
fix/expand-without-solution
Mar 25, 2026
Merged

fix: allow expand() on clustered FlowSystem without a solution#646
FBumann merged 3 commits intomainfrom
fix/expand-without-solution

Conversation

@FBumann
Copy link
Copy Markdown
Member

@FBumann FBumann commented Mar 24, 2026

Summary

  • Removed the requirement for a solution in _validate_for_expansion(), so expand() can be called on unsolved clustered FlowSystems
  • Made solution expansion in expand_flow_system() conditional (if reduced_solution is not None)
  • Fixes: flow_system.transform.expand().transform.clustering_data() raising ValueError when no solution exists

Test plan

  • Existing clustering/expansion tests still pass
  • Verify expand() works on an unsolved clustered FlowSystem
  • Verify expand() still correctly expands solutions when they exist

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Expansion of flow systems no longer fails when no optimization solution exists; expansion proceeds and preserves original timesteps without raising an error.
  • Tests

    • Tests updated to reflect the new behavior: expanding without a solution is supported and validated.

Previously, expand() required a solved FlowSystem, which prevented
using it to inspect expanded data (e.g. clustering_data()) without
solving first. Now the solution expansion is skipped when no solution
is present.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a04f20f2-6bae-4d01-811e-ad5179dca6ef

📥 Commits

Reviewing files that changed from the base of the PR and between 10db018 and b0b7ec4.

📒 Files selected for processing (1)
  • flixopt/transform_accessor.py

📝 Walkthrough

Walkthrough

expand_flow_system no longer assumes a solution exists: solution-variable expansion is executed only when a reduced solution is present. Validation for expansion no longer requires a solution, only clustering. Tests updated to reflect successful expansion when no solution is available.

Changes

Cohort / File(s) Summary
Solution Expansion Logic
flixopt/transform_accessor.py
Guarded solution expansion in expand_flow_system on reduced_solution is not None; removed the explicit ValueError for missing self._fs.solution in _validate_for_expansion, leaving only clustering requirement.
Tests
tests/test_clustering/test_cluster_reduce_expand.py
Renamed test_expand_without_solution_raisestest_expand_without_solution; test now calls expand() without expecting an exception and asserts expanded solution is None and timesteps match original.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰
I hopped through clusters, soft and spry,
Found no solution—so I hopped on by.
Expansion blooms where timesteps mend,
Clusters guide the hop, not a missing end.
Carrots cheer: the flows extend! 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: allowing the expand() method to work on clustered FlowSystems without requiring a solution.
Description check ✅ Passed The description covers the key changes (removing solution requirement, making expansion conditional, fixing the error), includes a test plan, and provides context. Most template sections are adequately addressed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/expand-without-solution

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.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
flixopt/transform_accessor.py (1)

1896-1911: ⚠️ Potential issue | 🟡 Minor

Stale docstring in expand() method.

The _validate_for_expansion() changes look correct. However, the docstring for expand() (lines 1932-1934) still lists:

Raises:
    ValueError: If the FlowSystem was not created with ``cluster()``.
    ValueError: If the FlowSystem has no solution.

The second ValueError is no longer raised and should be removed to keep documentation accurate.

📝 Suggested docstring fix (outside selected range)

In the expand() method docstring around line 1932-1934, remove the stale error documentation:

 Raises:
     ValueError: If the FlowSystem was not created with ``cluster()``.
-    ValueError: If the FlowSystem has no solution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@flixopt/transform_accessor.py` around lines 1896 - 1911, The docstring for
the expand() method still documents two ValueError cases but only one is
actually raised; update the expand() docstring to remove the stale "ValueError:
If the FlowSystem has no solution." entry so it matches the current
implementation that uses _validate_for_expansion() (which raises the
cluster-related ValueError). Locate the expand() method and remove the second
ValueError line from its Raises: section, leaving only the cluster-related
ValueError documented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@flixopt/transform_accessor.py`:
- Around line 1896-1911: The docstring for the expand() method still documents
two ValueError cases but only one is actually raised; update the expand()
docstring to remove the stale "ValueError: If the FlowSystem has no solution."
entry so it matches the current implementation that uses
_validate_for_expansion() (which raises the cluster-related ValueError). Locate
the expand() method and remove the second ValueError line from its Raises:
section, leaving only the cluster-related ValueError documented.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 31e7e5f9-a124-4be2-90ef-5f3e6f905fe5

📥 Commits

Reviewing files that changed from the base of the PR and between 15aca5d and 10db018.

📒 Files selected for processing (2)
  • flixopt/transform_accessor.py
  • tests/test_clustering/test_cluster_reduce_expand.py

@FBumann FBumann enabled auto-merge (squash) March 25, 2026 09:15
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@FBumann FBumann merged commit 2d4a507 into main Mar 25, 2026
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.

1 participant