refac: store per-contributor effect expressions during build, eliminate contributions.py duplication

[FBumann]

Background

compute_effect_contributions() in src/fluxopt/contributions.py re-implements the same multiplication logic that _create_effects() in src/fluxopt/model.py uses to build the lump and temporal expressions. The two paths must stay in lockstep — every time we add a new cost type to the model, we have to remember to mirror it in contributions, or the validation check fails (this is what #84 was about; PR #135 fixed Investment, but the duplication remains).

This issue proposes eliminating the duplication by storing per-contributor expressions during model build and evaluating them after solve.

Current architecture

In _create_effects():

# Each term is built per-flow then summed before becoming a constraint
lump_direct = lump_direct + (eps * self.flow_size.sel(flow=sizing_ids)).sum('flow')
lump_direct = lump_direct + (ef * self.flow_size_indicator).sum('flow')
# ... 6 more similar blocks for sizing fixed, storage, investment (4 types)

self.m.add_constraints(self.effect_lump == lump_rhs, name='effect_lump_eq')

The .sum('flow') collapses contributor dim before the expression becomes a variable. The solver only sees effect_lump[effect, period].

In compute_effect_contributions(), the same multiplications are redone on solved values without the .sum('flow'):

flow_lump = _compute_sizing_lump(...)         # sizing per-size and fixed
flow_lump = flow_lump + _compute_investment_lump(...)  # 4 investment cost types

This duplication is brittle.

Proposed approach

Store the un-summed expressions during model build, evaluate .solution after solve.

Linopy LinearExpression has a .solution property that returns the numeric value of the expression with current solver values. This means we can:

1. In _create_effects(), build per-contributor expressions WITHOUT collapsing flow:

   per_flow_lump = eps * self.flow_size.sel(flow=sizing_ids)  # has 'flow' dim
   self._lump_contrib_terms.append(per_flow_lump)  # store, don't collapse
   lump_direct = lump_direct + per_flow_lump.sum('flow')  # constraint as before

2. After solve, in contributions.py:

   per_flow_contributions = sum(term.solution for term in fs._lump_contrib_terms)
   # Apply Leontief inverse for cross-effects (existing logic, ~5 lines)

3. Delete _compute_sizing_lump(), _compute_investment_lump(), and most of _compute_periodic(). Keep only _leontief() and _apply_leontief().

Pre-work: verify linopy supports this

Before refactoring, confirm:

• LinearExpression.solution works for products of variables and DataArray coefficients (it should — linopy uses this internally for Variable.solution)
• It correctly preserves dims on the result
• It's not too slow

A quick spike:

from linopy import Model
import xarray as xr

m = Model()
x = m.add_variables(coords={'flow': xr.DataArray(['a', 'b'], dims='flow')}, name='x', lower=0)
m.add_constraints(x.sum('flow') >= 5)
m.add_objective(x.sum())
m.solve()

coeff = xr.DataArray([2, 3], dims='flow', coords={'flow': ['a', 'b']})
expr = coeff * x  # has flow dim
print(expr.solution)  # should give per-flow values

Files to touch

• src/fluxopt/model.py — _create_effects() (~lines 728-905): refactor each term to also store the per-contributor expression
• src/fluxopt/contributions.py — replace most of compute_effect_contributions with .solution lookups + Leontief
• tests/math/test_contributions.py — should pass unchanged if the math is right
• tests/math_port/test_multi_period.py — the validation-pipeline xfail case (fix: effect_contributions Leontief inverse fails for period-varying contribution_from #134) might also be fixed

What this does NOT solve

• Issue fix: effect_contributions Leontief inverse fails for period-varying contribution_from #134 (period-varying contribution_from in Leontief) is independent — still needs a separate fix to _leontief() / _apply_leontief() for the period batch dim. But this refactor might make it easier to fix because contributions.py becomes simpler.

Verification

uv run pytest -x                    # all tests pass
uv run mypy src/                    # types check
uv run ruff check . && uv run ruff format .  # lint/format

Specifically watch for:

• Per-contributor breakdown matches solver totals (existing validation)
• Multi-period contributions return correct period dim
• Cross-effects (Leontief) still work correctly
• No memory regression — storing expressions has overhead, but should be small relative to solver state

Out of scope

• Don't change the public contributions API (Result.stats.effect_contributions keys: temporal, lump, total)
• Don't refactor unrelated parts of _create_effects (just touch the lump/temporal accumulation)
• Don't try to fix fix: effect_contributions Leontief inverse fails for period-varying contribution_from #134 in the same PR — separate concern

Trade-offs

Pros

• Single source of truth for contribution math (model.py)
• New cost types added only in one place
• Smaller contributions.py (rough estimate: ~80 lines deleted)
• Eliminates the recurring "we forgot to add X to contributions" bug class

Cons

• Slightly higher build-time memory (storing expressions on FlowSystem)
• Tighter coupling: compute_effect_contributions now depends on FlowSystem internals (the stored term list), not just ModelData + solution
• If a future change makes the model expression structure non-trivially different from a sum of per-flow terms, this breaks down

Related

• PR fix: include Investment costs in compute_effect_contributions #135: fixed Investment in compute_effect_contributions using the duplicated approach (good interim fix, but motivates this refactor)
• Issue fix: effect_contributions does not support investment costs or period decomposition #84 (closed by fix: include Investment costs in compute_effect_contributions #135): the bug class this refactor would prevent recurrence of
• Issue fix: effect_contributions Leontief inverse fails for period-varying contribution_from #134: separate Leontief bug — independent