fix(transform): don't force investment when fixing a size of 0

[FBumann]

Problem

transform.fix_sizes() unconditionally set mandatory=True on every InvestParameters it fixed. When a fixed size is 0 (the sizing run chose not to invest), mandatory=True still charges the flat effects_of_investment, because a mandatory investment has no invested binary to gate the cost (features.py:101-109).

The two-stage sizing → dispatch workflow is meant to reproduce the sizing objective at full resolution, but the phantom fixed cost silently broke that:

scenario	stage-1 objective	old two-stage	fixed
single period, optimal = don't invest	160	100160	160
multi-period, sizes mix 0 and non-zero [0, 90]	700	1200	700

No error was raised — just a wrong objective.

Fix

• mandatory = bool((fixed_value != 0).all()) — only force the investment when every period/scenario value is non-zero; otherwise keep it optional so the invested binary gates the fixed cost per period. This handles the don't-invest case and per-period sizes that mix 0 and non-zero values (a single allclose(size, 0) check cannot, since it stays mandatory whenever any value is non-zero).
• Keep the raw DataArray for fixed_value (a DataArray always has .dims, avoiding a scalar-float crash in bound construction when mandatory is False).
• reset() the returned FlowSystem so it is an unsolved dispatch problem, as the docstring promises — from_dataset() had been restoring the stage-1 solution.

Tests

Adds two regression tests asserting the two-stage objective reproduces the sizing objective:

• test_fix_sizes_no_invest_reproduces_objective (single period, size 0)
• test_fix_sizes_mixed_period_invest_reproduces_objective (per-period [0, 90])

tests/test_math/test_multi_period.py: 36 passed. Full tests/test_math/: 386 passed (the 9 test_flow_invest failures pre-exist on main and are unrelated).

Known limitation

For genuinely mixed per-period sizes, mandatory=False pins the non-zero periods only because demand forces the invested binary on. A scalar mandatory flag cannot express "invest in 2021 but not 2020" exactly; this is the most faithful option available without per-period invested-fixing, and is strictly better than the previous behavior.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed investment sizing so optional investments with a fixed size of 0 are no longer treated as mandatory, preventing incorrect charges in dispatch results.
  • Improved multi-period sizing behavior so fixed investment costs are applied per period correctly, preserving expected objectives and results.
  • Ensured restored solutions are refreshed after size adjustments, reducing inconsistencies when re-running dispatch calculations.

[coderabbitai]

📝 Walkthrough

Walkthrough

TransformAccessor.fix_sizes() now computes the mandatory flag per size variable based on whether all fixed values are non-zero, rather than always forcing it to True, and calls new_fs.reset() when investments are modified. New regression tests validate zero-investment and mixed-period investment scenarios.

Changes

fix_sizes Mandatory Flag and Reset Behavior

Layer / File(s)	Summary
Mandatory flag computation and reset flixopt/transform_accessor.py	Docstring clarifies that fixed size 0 keeps investments optional; mandatory is now computed per size variable from non-zero fixed values and applied to flow and component InvestParameters; new_fs.reset() is called when modifications occurred.
Multi-period regression tests tests/test_math/test_multi_period.py	Imports consolidated to include _SOLVER and make_flow_system; two new tests verify fix_sizes() avoids charging flat costs for zero-size investments and correctly charges per-period costs in mixed-investment scenarios.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• flixOpt/flixopt#358: Both PRs adjust the “optional vs mandatory” semantics for InvestParameters, with the main PR computing mandatory per fixed-size variable inside fix_sizes().
• flixOpt/flixopt#696: Both PRs modify TransformAccessor.fix_sizes() for how fixed investment sizes are computed/applied across multi-period inputs.

Poem
A rabbit checked the sizes twice,
Zero means optional, that's precise,
Mandatory now earns its name,
Per period, per var, the same,
Reset the stage, and run again! 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and directly matches the main change: fixing size-0 investments from being forced mandatory.
Description check	✅ Passed	The PR covers the problem, fix, tests, and limitation, but it does not use the repository's template sections like Type of Change, Related Issues, or checklist items.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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/fix-sizes-mandatory-investment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@flixopt/transform_accessor.py`:
- Around line 1480-1485: FlowSystem.from_dataset() is only resetting the copied
system when size keys are modified, which leaves the restored stage-1 solution
locked in the no-match case. Update the from_dataset() flow in
transform_accessor.py so the copied FlowSystem is always reset before returning,
regardless of whether modified is true, and keep the existing reset path tied to
the copied system object (new_fs).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d2a3ee52-131e-4a7a-bbac-08666378be1e

📥 Commits

Reviewing files that changed from the base of the PR and between 59d70a8 and 15f294b.

📒 Files selected for processing (2)

• flixopt/transform_accessor.py
• tests/test_math/test_multi_period.py

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm to_dataset() includes the solution by default (making from_dataset restore it)
ast-grep run --pattern 'def to_dataset($$$):
  $$$' --lang python flixopt/flow_system.py
rg -nP 'def to_dataset' flixopt/flow_system.py

Repository: flixOpt/flixopt

Length of output: 264

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect relevant implementations and surrounding logic.
ast-grep outline flixopt/transform_accessor.py --match from_dataset --view expanded || true
ast-grep outline flixopt/flow_system.py --match to_dataset --view expanded || true

# Read the relevant ranges with line numbers.
sed -n '1430,1505p' flixopt/transform_accessor.py | cat -n
printf '\n--- flow_system.py ---\n'
sed -n '680,760p' flixopt/flow_system.py | cat -n

# Look for reset semantics and any solution-lock related flags.
rg -n "def reset|locked|solution|include_solution|from_dataset|to_dataset" flixopt/flow_system.py flixopt/transform_accessor.py

Repository: flixOpt/flixopt

Length of output: 25572

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check whether from_dataset() / reset() manipulate the stage-1 solution and lock state.
rg -n "class FlowSystem|def reset|def from_dataset|stage-1|locked|lock|solution" flixopt -g '*.py'

# Inspect the specific methods if found.
sed -n '1,260p' flixopt/flow_system.py | cat -n

Repository: flixOpt/flixopt

Length of output: 50371

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the dataset round-trip and reset behavior around the reported lines.
sed -n '680,760p' flixopt/flow_system.py | cat -n
printf '\n---\n'
sed -n '1430,1505p' flixopt/transform_accessor.py | cat -n

# Find the relevant method definitions and any lock/solution handling.
rg -n "def to_dataset|def from_dataset|def reset|locked|solution|include_solution|stage-1" flixopt/flow_system.py flixopt/transform_accessor.py

Repository: flixOpt/flixopt

Length of output: 23221

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect dataset restoration and reset semantics directly.
sed -n '1600,1835p' flixopt/io.py | cat -n
printf '\n--- reset ---\n'
sed -n '1600,1640p' flixopt/flow_system.py | cat -n
printf '\n--- optimize/lock ---\n'
sed -n '1410,1495p' flixopt/flow_system.py | cat -n

Repository: flixOpt/flixopt

Length of output: 18126

---

Always reset the copied system The returned FlowSystem should be unlocked even when no size key matches; otherwise FlowSystem.from_dataset() keeps the restored solution and the copy stays locked.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@flixopt/transform_accessor.py` around lines 1480 - 1485,
FlowSystem.from_dataset() is only resetting the copied system when size keys are
modified, which leaves the restored stage-1 solution locked in the no-match
case. Update the from_dataset() flow in transform_accessor.py so the copied
FlowSystem is always reset before returning, regardless of whether modified is
true, and keep the existing reset path tied to the copied system object
(new_fs).