Skip to content

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

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.

Sign up for GitHub

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

Merged
FBumann merged 1 commit into from
Jul 7, 2026
+122 −21
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 24 additions & 18 deletions flixopt/transform_accessor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,7 +1365,9 @@ def fix_sizes(
2. Fix sizes and solve dispatch at full resolution

The returned FlowSystem has InvestParameters with fixed_size set,
making those sizes mandatory rather than decision variables.
turning those sizes into constants rather than decision variables. A fixed
size of 0 keeps the investment optional so its fixed effects_of_investment
are not charged, letting the dispatch objective match the sizing run.

Args:
sizes: The sizes to fix. Can be:
Expand Down Expand Up @@ -1435,40 +1437,38 @@ def fix_sizes(
# Fix sizes in the new FlowSystem's InvestParameters
# Note: statistics.sizes returns keys without '|size' suffix (e.g., 'Boiler(Q_fu)')
# but dicts may have either format
modified = False
for size_var in sizes.data_vars:
# Normalize: strip '|size' suffix if present
base_name = size_var.replace('|size', '') if size_var.endswith('|size') else size_var
size_data = sizes[size_var]
if size_data.ndim == 0:
fixed_value = float(size_data.item())
else:
# Per-period/per-scenario sizes: keep the DataArray so each
# coordinate retains its own fixed size
fixed_value = size_data
base_name = size_var[: -len('|size')] if size_var.endswith('|size') else size_var
fixed_value = sizes[size_var]

# Find matching element with InvestParameters
found = False
# Only force the investment where every value is non-zero. A fixed size of
# 0 means "do not invest"; mandatory=True would still charge the flat
# effects_of_investment (no invested binary to gate it), so keep it
# optional whenever any period/scenario is 0.
mandatory = bool((fixed_value != 0).all())

# Check flows
found = False
for flow in new_fs.flows.values():
if flow.label_full == base_name and isinstance(flow.size, InvestParameters):
flow.size.fixed_size = fixed_value
flow.size.mandatory = True
flow.size.mandatory = mandatory
found = True
logger.debug(f'Fixed size of {base_name} to {fixed_value}')
modified = True
logger.debug(f'Fixed size of {base_name} to {fixed_value} (mandatory={mandatory})')
break

# Check storage capacity
if not found:
for component in new_fs.components.values():
if hasattr(component, 'capacity_in_flow_hours'):
if component.label == base_name and isinstance(
component.capacity_in_flow_hours, InvestParameters
):
component.capacity_in_flow_hours.fixed_size = fixed_value
component.capacity_in_flow_hours.mandatory = True
component.capacity_in_flow_hours.mandatory = mandatory
found = True
logger.debug(f'Fixed size of {base_name} to {fixed_value}')
modified = True
logger.debug(f'Fixed size of {base_name} to {fixed_value} (mandatory={mandatory})')
break

if not found:
Expand All @@ -1477,6 +1477,12 @@ def fix_sizes(
f'It may be a fixed-size component or the name may not match.'
)

# from_dataset() restores the stage-1 solution; drop it so the returned system
# is an unsolved dispatch problem (as documented) and re-transforms cleanly
# with the sizes we just assigned on the next optimize().
if modified:
new_fs.reset()

Comment on lines +1480 to +1485

@coderabbitai coderabbitai Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 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).

return new_fs

def clustering_data(
Expand Down
101 changes: 98 additions & 3 deletions tests/test_math/test_multi_period.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import flixopt as fx

from .conftest import make_multi_period_flow_system
from .conftest import _SOLVER, make_flow_system, make_multi_period_flow_system


class TestMultiPeriod:
Expand Down Expand Up @@ -390,8 +390,6 @@ def test_fix_sizes_preserves_per_period_sizes(self, optimize):
scalar' on any multi-period model. If per-period sizes were collapsed
to a single value instead, stage-2 sizes or objective would differ.
"""
from .conftest import _SOLVER

fs = make_multi_period_flow_system(n_timesteps=3, periods=[2020, 2025], weight_of_last_period=5)
demand = xr.DataArray(
np.array([[10, 50, 20], [10, 80, 20]], dtype=float),
Expand Down Expand Up @@ -439,3 +437,100 @@ def test_fix_sizes_preserves_per_period_sizes(self, optimize):
fs_dispatch.optimize(_SOLVER)
assert_allclose(fs_dispatch.solution['Boiler(heat)|size'].values, [50.0, 80.0], rtol=1e-5)
assert_allclose(fs_dispatch.solution['objective'].item(), 1700.0, rtol=1e-5)

def test_fix_sizes_no_invest_reproduces_objective(self, optimize):
"""Proves: transform.fix_sizes() does not charge investment for a size of 0.

Single period, 3 ts. Demand=[10, 50, 20] (sum 80). A DirectHeat source @2€
competes with a Boiler whose investment costs a prohibitive 100000€ fixed.
Optimal is to NOT invest and serve demand directly: objective = 80*2 = 160.

Stage 2 must reproduce size 0 AND objective 160.

Sensitivity: fix_sizes() used to force mandatory=True unconditionally. With
a fixed size of 0 that still charges the flat effects_of_investment (100000),
so the dispatch objective jumped to 100160 instead of 160.
"""
fs = make_flow_system(n_timesteps=3)
demand = xr.DataArray(np.array([10, 50, 20], dtype=float), coords={'time': fs.timesteps}, dims=['time'])
fs.add_elements(
fx.Bus('Heat'),
fx.Bus('Gas'),
fx.Effect('costs', '€', is_standard=True, is_objective=True),
fx.Sink('Demand', inputs=[fx.Flow('heat', bus='Heat', size=1, fixed_relative_profile=demand)]),
fx.Source('DirectHeat', outputs=[fx.Flow('h', bus='Heat', effects_per_flow_hour=2)]),
fx.Source('GasSrc', outputs=[fx.Flow('gas', bus='Gas', effects_per_flow_hour=1)]),
fx.linear_converters.Boiler(
'Boiler',
thermal_efficiency=1.0,
fuel_flow=fx.Flow('fuel', bus='Gas'),
thermal_flow=fx.Flow(
'heat',
bus='Heat',
size=fx.InvestParameters(
maximum_size=200,
effects_of_investment=100000,
effects_of_investment_per_size=1,
),
),
),
)
fs = optimize(fs)
assert_allclose(fs.solution['Boiler(heat)|size'].item(), 0.0, atol=1e-6)
assert_allclose(fs.solution['objective'].item(), 160.0, rtol=1e-5)

fs_dispatch = fs.transform.fix_sizes()
fs_dispatch.optimize(_SOLVER)
assert_allclose(fs_dispatch.solution['Boiler(heat)|size'].item(), 0.0, atol=1e-6)
assert_allclose(fs_dispatch.solution['objective'].item(), 160.0, rtol=1e-5)

def test_fix_sizes_mixed_period_invest_reproduces_objective(self, optimize):
"""Proves: transform.fix_sizes() charges investment per period, not globally.

periods=[2020, 2021], weight_of_last_period=1 -> weights [1, 1]. 3 ts each.
Demand is 0 in 2020 and [10, 90, 10] in 2021. The Boiler (10000€ fixed
invest + 1 per size) is only built in 2021 (size 90); 2020 stays at size 0.
Per-period cost: 2020: 0; 2021: 10000 + 90 + 110 = 10200. Objective = 10200.

Stage 2 must reproduce sizes [0, 90] AND objective 10200.

Sensitivity: with the old unconditional mandatory=True, the 2020 period
(size 0) was still charged the 10000€ fixed investment, inflating the
objective to 20200. A scalar mandatory flag cannot express "invest in 2021
but not 2020"; keeping it optional lets the invested binary gate the cost.
"""
fs = make_multi_period_flow_system(n_timesteps=3, periods=[2020, 2021], weight_of_last_period=1)
demand = xr.DataArray(
np.array([[0, 0, 0], [10, 90, 10]], dtype=float),
coords={'period': [2020, 2021], 'time': fs.timesteps},
dims=['period', 'time'],
)
fs.add_elements(
fx.Bus('Heat'),
fx.Bus('Gas'),
fx.Effect('costs', '€', is_standard=True, is_objective=True),
fx.Sink('Demand', inputs=[fx.Flow('heat', bus='Heat', size=1, fixed_relative_profile=demand)]),
fx.Source('GasSrc', outputs=[fx.Flow('gas', bus='Gas', effects_per_flow_hour=1)]),
fx.linear_converters.Boiler(
'Boiler',
thermal_efficiency=1.0,
fuel_flow=fx.Flow('fuel', bus='Gas'),
thermal_flow=fx.Flow(
'heat',
bus='Heat',
size=fx.InvestParameters(
maximum_size=200,
effects_of_investment=10000,
effects_of_investment_per_size=1,
),
),
),
)
fs = optimize(fs)
assert_allclose(fs.solution['Boiler(heat)|size'].values, [0.0, 90.0], atol=1e-6)
assert_allclose(fs.solution['objective'].item(), 10200.0, rtol=1e-5)

fs_dispatch = fs.transform.fix_sizes()
fs_dispatch.optimize(_SOLVER)
assert_allclose(fs_dispatch.solution['Boiler(heat)|size'].values, [0.0, 90.0], atol=1e-6)
assert_allclose(fs_dispatch.solution['objective'].item(), 10200.0, rtol=1e-5)
Loading