Feature/v3/invest #351
Closed
Feature/v3/invest #351
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # flixopt/flow_system.py # flixopt/interface.py # flixopt/structure.py
# Conflicts: # .github/ISSUE_TEMPLATE/bug_report.yml # .github/ISSUE_TEMPLATE/config.yml # .github/ISSUE_TEMPLATE/feature_request.yml # .github/pull_request_template.md # CHANGELOG.md # README.md # docs/SUMMARY.md # docs/contribute.md # flixopt/calculation.py # flixopt/effects.py # flixopt/elements.py # flixopt/features.py # flixopt/flow_system.py # flixopt/interface.py # flixopt/modeling.py # flixopt/structure.py # pyproject.toml
…th years. Further, remove investment_scenarios parameter
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds year-aware investment modeling and utilities, extends FlowSystem with years_per_year and related API, generalizes duration tracking primitives, propagates an optional name_prefix through transform_data across interfaces/components/effects/elements, widens effects non-temporal input types, updates weights behavior, and introduces comprehensive tests. CHANGELOG wording adjusted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test/Caller
participant FS as FlowSystem
participant IM as InvestmentModel
participant MP as ModelingPrimitives
participant S as Solver
participant R as Results
T->>FS: __init__(timesteps, years, years_of_last_year, weights)
FS-->>FS: calculate_years_per_year()
FS-->>T: FlowSystem with years_per_year, years_of_investment
T->>IM: build(model, flow, InvestParameters)
IM->>IM: transform_data(..., name_prefix)
IM->>IM: create variables (year-aware if years present)
IM->>MP: consecutive_duration_tracking(duration_per_step=FS.years_per_year, duration_dim='year')
IM->>MP: link_changes_to_level_with_binaries(...)
IM->>MP: continuous_transition_bounds(...)
IM->>S: optimize()
S-->>IM: solution
IM-->>R: assemble results
R-->>T: FlowResults
sequenceDiagram
autonumber
participant C as Component/Effect
participant IF as Interface
participant FS as FlowSystem
C->>IF: transform_data(flow_system, name_prefix='')
Note over C,IF: name_prefix propagated uniformly
IF->>FS: fit_to_model_coords(dims/coords)
FS-->>IF: aligned data
IF-->>C: transformed attributes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
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. Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
flixopt/components.py (1)
833-839: Discharge efficiency applied incorrectly in storage balance equation.Docstring states discharge should divide by eta_discharge. Current code multiplies, which overstates stored energy and breaks energy conservation.
Apply this diff:
- - discharge_rate * eff_discharge * hours_per_step, + - discharge_rate * hours_per_step / eff_discharge,Consider validating eta_discharge > 0 during transform/plausibility checks to avoid division-by-zero.
flixopt/features.py (1)
320-335: Bug: self.hours_per_step attribute does not exist on OnOffModel.Should reference the FlowSystemModel’s hours_per_step.
Apply this diff:
- duration_per_step=self.hours_per_step, + duration_per_step=self._model.hours_per_step, duration_dim='time',Repeat the same change for consecutive_off_hours.
flixopt/modeling.py (2)
14-53: to_binary: handle scalars and avoid invalid len() usage.Passing a scalar (e.g., 0) currently raises. Also the size==0 branch uses .item() unsafely.
Apply this diff:
@staticmethod def to_binary( values: xr.DataArray, epsilon: float | None = None, dims: str | list[str] | None = None, ) -> xr.DataArray: @@ - if not isinstance(values, xr.DataArray): - values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))}) + if not isinstance(values, xr.DataArray): + arr = np.asarray(values) + if arr.ndim == 0: + values = xr.DataArray(arr) # 0-D scalar + else: + values = xr.DataArray(arr, dims=['time'], coords={'time': np.arange(arr.shape[0])}) @@ - if values.size == 0: - return xr.DataArray(0) if values.item() < epsilon else xr.DataArray(1) + if values.size == 0: + return xr.DataArray(0)
81-89: count_consecutive_states: boolean check and numpy where usage.
- Using .all() on a scalar bool is invalid.
- Use underlying .values for np.where.
Apply this diff:
- if np.isclose(binary_values.isel({dim: -1}).item(), 0, atol=epsilon).all(): + if bool(np.isclose(binary_values.isel({dim: -1}).item(), 0, atol=epsilon)): return 0.0 @@ - zero_indices = np.where(is_zero)[0] + zero_indices = np.where(is_zero.values)[0]flixopt/interface.py (1)
879-916: Fix_plausibility_checkscall signature andprevious_sizedims
- Call
self._plausibility_checks()(drop theflow_systemargument)- Use
dims=['scenario']forprevious_sizeto avoid year conflicts- self._plausibility_checks(flow_system) + self._plausibility_checks() … - self.previous_size = flow_system.fit_to_model_coords( - f'{name_prefix}|previous_size', self.previous_size, dims=['year', 'scenario'] - ) + self.previous_size = flow_system.fit_to_model_coords( + f'{name_prefix}|previous_size', self.previous_size, dims=['scenario'] + )
🧹 Nitpick comments (13)
CHANGELOG.md (2)
38-55: Fix Markdown heading level (MD001) under Unreleased section.Headings jump from H2 to H4. Use H3 for section headers to satisfy markdownlint and keep structure consistent.
Apply this diff:
-#### Multi-year-investments +### Multi-year-investments @@ -#### Stochastic modeling +### Stochastic modeling @@ -#### Improved Data handling: IO, resampling and more through xarray +### Improved Data handling: IO, resampling and more through xarray
72-80: Tighten wording and fix minor typos in Unreleased text.Purely editorial; improves clarity and polish.
Apply this diff:
-* FlowSystem Restoring: The used FlowSystem is now acessible directly form the results without manual restoring (lazily). All Parameters can be safely accessed anytime after the solve. +* FlowSystem restoring: The used FlowSystem is now accessible directly from the results without manual restoring (lazily). All parameters can be safely accessed anytime after the solve. @@ -* Improved filter methods in `resulty.py` +* Improved filter methods in `results.py` @@ -* IO for single Interfaces/Elemenets to Datasets might not work properly if the Interface/Element is not part of a fully transformed and connected FlowSystem. This arrises from Numeric Data not being stored as xr.DataArray by the user. To avoid this, always use the `to_dataset()` on Elements inside a FlowSystem thats connected and transformed. +* IO for single Interfaces/Elements to Datasets might not work properly if the Interface/Element is not part of a fully transformed and connected FlowSystem. This arises from numeric data not being stored as xr.DataArray by the user. To avoid this, always use `to_dataset()` on Elements inside a FlowSystem that's connected and transformed. @@ -* Added new module `.modeling`that contains Modelling primitives and utilities +* Added new module `.modeling` that contains modeling primitives and utilities @@ -* Improved access to the Submodels and their variables, constraints and submodels +* Improved access to Submodels and their variables, constraints and submodelsAlso applies to: 107-109, 111-117
flixopt/structure.py (1)
174-178: Default weights: guard dims and make broadcast robust.If year or scenario dims are absent, passing dims=['year','scenario'] may misalign or error depending on fit_to_model_coords. Build dims dynamically from available coords.
Apply this diff:
- weights = self.flow_system.fit_to_model_coords( - 'weights', - 1 if self.flow_system.years is None else self.flow_system.years_per_year, - dims=['year', 'scenario'], - ) + base = 1 if self.flow_system.years is None else self.flow_system.years_per_year + dims = [d for d in ('year', 'scenario') if d in self.flow_system.coords] + weights = self.flow_system.fit_to_model_coords('weights', base, dims=dims)Please also confirm the intended normalization semantics (global sum to 1 vs per-year normalization) are correct for multi-year/multi-scenario objectives.
flixopt/components.py (4)
207-214: Propagate name_prefix through transform_data.Currently ignored. Forward it to parent and compose prefixes for nested transforms for consistent naming.
Apply this diff:
- def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: - super().transform_data(flow_system) + def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: + super().transform_data(flow_system, name_prefix) if self.conversion_factors: self.conversion_factors = self._transform_conversion_factors(flow_system) if self.piecewise_conversion: self.piecewise_conversion.has_time_dim = True - self.piecewise_conversion.transform_data(flow_system, f'{self.label_full}|PiecewiseConversion') + prefix = f'{name_prefix}|{self.label_full}' if name_prefix else self.label_full + self.piecewise_conversion.transform_data(flow_system, f'{prefix}|PiecewiseConversion')
215-227: Conversion factors: explicitly align to model dims.To avoid accidental broadcasting with new year/scenario dims, pass dims when fitting.
Apply this diff:
- ts = flow_system.fit_to_model_coords(f'{self.flows[flow].label_full}|conversion_factor{idx}', values) + ts = flow_system.fit_to_model_coords( + f'{self.flows[flow].label_full}|conversion_factor{idx}', + values, + dims=['time', 'year', 'scenario'], + )
425-466: Storage.transform_data should forward name_prefix and use it when delegating.Keeps naming consistent across nested structures.
Apply this diff:
- def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: - super().transform_data(flow_system) + def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: + super().transform_data(flow_system, name_prefix) @@ - if isinstance(self.capacity_in_flow_hours, InvestParameters): - self.capacity_in_flow_hours.transform_data(flow_system, f'{self.label_full}|InvestParameters') + if isinstance(self.capacity_in_flow_hours, InvestParameters): + prefix = f'{name_prefix}|{self.label_full}' if name_prefix else self.label_full + self.capacity_in_flow_hours.transform_data(flow_system, f'{prefix}|InvestParameters')
696-703: Transmission.transform_data should propagate name_prefix.Forward the prefix and use it for nested names.
Apply this diff:
- def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: - super().transform_data(flow_system) + def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: + super().transform_data(flow_system, name_prefix) self.relative_losses = flow_system.fit_to_model_coords( f'{self.label_full}|relative_losses', self.relative_losses ) self.absolute_losses = flow_system.fit_to_model_coords( f'{self.label_full}|absolute_losses', self.absolute_losses )flixopt/effects.py (1)
177-216: Propagate name_prefix for effect variable naming consistency.Currently ignored; compose it with label_full so nested calls can prefix names coherently.
Apply this diff:
- def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: - self.minimum_operation_per_hour = flow_system.fit_to_model_coords( - f'{self.label_full}|minimum_operation_per_hour', self.minimum_operation_per_hour + def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: + prefix = f'{name_prefix}|{self.label_full}' if name_prefix else self.label_full + self.minimum_operation_per_hour = flow_system.fit_to_model_coords( + f'{prefix}|minimum_operation_per_hour', self.minimum_operation_per_hour ) @@ - self.maximum_operation_per_hour = flow_system.fit_to_model_coords( - f'{self.label_full}|maximum_operation_per_hour', self.maximum_operation_per_hour + self.maximum_operation_per_hour = flow_system.fit_to_model_coords( + f'{prefix}|maximum_operation_per_hour', self.maximum_operation_per_hour ) @@ - self.specific_share_to_other_effects_operation = flow_system.fit_effects_to_model_coords( - f'{self.label_full}|operation->', self.specific_share_to_other_effects_operation, 'operation' + self.specific_share_to_other_effects_operation = flow_system.fit_effects_to_model_coords( + f'{prefix}|operation->', self.specific_share_to_other_effects_operation, 'operation' ) @@ - self.minimum_operation = flow_system.fit_to_model_coords( - f'{self.label_full}|minimum_operation', self.minimum_operation, dims=['year', 'scenario'] + self.minimum_operation = flow_system.fit_to_model_coords( + f'{prefix}|minimum_operation', self.minimum_operation, dims=['year', 'scenario'] ) - self.maximum_operation = flow_system.fit_to_model_coords( - f'{self.label_full}|maximum_operation', self.maximum_operation, dims=['year', 'scenario'] + self.maximum_operation = flow_system.fit_to_model_coords( + f'{prefix}|maximum_operation', self.maximum_operation, dims=['year', 'scenario'] ) - self.minimum_invest = flow_system.fit_to_model_coords( - f'{self.label_full}|minimum_invest', self.minimum_invest, dims=['year', 'scenario'] + self.minimum_invest = flow_system.fit_to_model_coords( + f'{prefix}|minimum_invest', self.minimum_invest, dims=['year', 'scenario'] ) - self.maximum_invest = flow_system.fit_to_model_coords( - f'{self.label_full}|maximum_invest', self.maximum_invest, dims=['year', 'scenario'] + self.maximum_invest = flow_system.fit_to_model_coords( + f'{prefix}|maximum_invest', self.maximum_invest, dims=['year', 'scenario'] ) - self.minimum_total = flow_system.fit_to_model_coords( - f'{self.label_full}|minimum_total', + self.minimum_total = flow_system.fit_to_model_coords( + f'{prefix}|minimum_total', self.minimum_total, dims=['year', 'scenario'], ) - self.maximum_total = flow_system.fit_to_model_coords( - f'{self.label_full}|maximum_total', self.maximum_total, dims=['year', 'scenario'] + self.maximum_total = flow_system.fit_to_model_coords( + f'{prefix}|maximum_total', self.maximum_total, dims=['year', 'scenario'] ) - self.specific_share_to_other_effects_invest = flow_system.fit_effects_to_model_coords( - f'{self.label_full}|invest->', + self.specific_share_to_other_effects_invest = flow_system.fit_effects_to_model_coords( + f'{prefix}|invest->', self.specific_share_to_other_effects_invest, 'invest', dims=['year', 'scenario'], )flixopt/elements.py (3)
100-106: Component.transform_data should pass name_prefix onward.Forward prefix to parent and flows for consistent nested naming.
Apply this diff:
- def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: - if self.on_off_parameters is not None: - self.on_off_parameters.transform_data(flow_system, self.label_full) - - for flow in self.inputs + self.outputs: - flow.transform_data(flow_system) + def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: + if self.on_off_parameters is not None: + prefix = f'{name_prefix}|{self.label_full}' if name_prefix else self.label_full + self.on_off_parameters.transform_data(flow_system, prefix) + + for flow in self.inputs + self.outputs: + flow.transform_data(flow_system, name_prefix)
420-452: Flow.transform_data: incorporate name_prefix in delegated transforms.Use the prefix when passing to OnOffParameters/InvestParameters and keep forwarding to fit_* names.
Apply this diff:
- def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: + def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None: self.relative_minimum = flow_system.fit_to_model_coords( f'{self.label_full}|relative_minimum', self.relative_minimum ) @@ - if self.on_off_parameters is not None: - self.on_off_parameters.transform_data(flow_system, self.label_full) + if self.on_off_parameters is not None: + prefix = f'{name_prefix}|{self.label_full}' if name_prefix else self.label_full + self.on_off_parameters.transform_data(flow_system, prefix) if isinstance(self.size, InvestParameters): - self.size.transform_data(flow_system, self.label_full) + prefix = f'{name_prefix}|{self.label_full}' if name_prefix else self.label_full + self.size.transform_data(flow_system, prefix)
203-205: Minor typos in warning.“FLow” and capitalization.
Apply this diff:
- f'Using a FLow with a fixed size ({flow.label_full}) AND a piecewise_conversion ' - f'(in {self.label_full}) and variable size is uncommon. Please check if this is intended!' + f'Using a Flow with a fixed size ({flow.label_full}) AND a piecewise_conversion ' + f'(in {self.label_full}) and variable size is uncommon. Please check if this is intended.'tests/test_models.py (2)
45-91: Annualized effects generator: OK.Logic is clear; could be vectorized later for performance on large year sets.
186-232: Solver dependency in test: consider graceful skip if Gurobi unavailable.This integration test hard-depends on Gurobi. Mark or guard to avoid CI breaks.
Apply this diff:
@pytest.fixture def flow_system() -> fx.FlowSystem: @@ class TestYearAwareInvestmentModelDirect: @@ - def test_flow_invest_new(self, flow_system): + def test_flow_invest_new(self, flow_system): + if not hasattr(fx.solvers, "GurobiSolver"): + pytest.skip("Gurobi not available in this environment")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CHANGELOG.md(2 hunks)flixopt/components.py(4 hunks)flixopt/effects.py(3 hunks)flixopt/elements.py(4 hunks)flixopt/features.py(8 hunks)flixopt/flow_system.py(8 hunks)flixopt/interface.py(10 hunks)flixopt/modeling.py(4 hunks)flixopt/structure.py(2 hunks)tests/test_models.py(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
38-38: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (26)
flixopt/effects.py (2)
19-19: Type widening is appropriate.Switching NonTemporalEffectsUser to accept NonTemporalDataUser aligns invest-side shares with year/scenario-aware modeling.
278-279: Alias update acknowledged.The new NonTemporalEffectsUser alias remains backward-compatible for scalar inputs and supports year/scenario-aware values.
flixopt/elements.py (2)
192-195: LGTM on Bus.transform_data.The signature is standardized and coordinates fit uses label_full. No issues.
690-693: Lower bound logic for non-optional investment looks correct.The InvestParameters guard avoids unintended LB=0 with required size; matches year-aware modeling.
flixopt/structure.py (1)
225-231: All transform_data implementations include the new name_prefix parameter No subclasses missing the updated signature.flixopt/features.py (5)
14-16: Public imports look good.Exposing CONFIG and ModelingUtilitiesAbstract here is appropriate for downstream usage.
26-38: Docstring + public attribute annotation: OK.Clearer API surface for InvestmentModel.parameters.
59-81: Verify get_coords gracefully ignores missing dims.This path is taken when years is None, yet coords request includes 'year'. Ensure Submodel.get_coords filters out unavailable dims to avoid shape/key errors.
194-206: Piecewise effects integration: OK.Clean encapsulation via PiecewiseEffectsModel and sensible zero_point wiring to is_invested.
219-238: Accessor properties: OK.Names match variable short_names introduced above.
flixopt/flow_system.py (5)
52-63: Docstring extension: OK.years_of_last_year is documented clearly.
85-91: Year-aware attributes initialization: OK.Storing years_per_year and a renamed years_of_investment improves clarity.
383-387: Simplified coord fitting looks good.Restricting coords by dims improves consistency; relies on self.coords to include only active axes.
779-795: Minor efficiency improvement: OK.Using a local ds reduces repeated to_dataset() calls.
816-832: Minor efficiency improvement: OK.Same for isel path.
flixopt/interface.py (7)
11-12: xr import is fine.Keeps typing options open; ensure linters tolerate potential unused import.
78-82: Piece.transform_data: OK.dims gating on has_time_dim is correct; name_prefix propagation is consistent.
224-227: Piecewise.transform_data: OK.Indexing/prefixing per piece is clean.
446-449: PiecewiseConversion.transform_data: OK.Per-flow name scoping prevents collisions.
658-662: PiecewiseEffects.transform_data: OK.Origin/effects prefixed consistently; time-dim propagated via has_time_dim.
857-878: InvestParameters: previous_size wiring is appropriate.Public parameter matches multi-period modeling intent.
1132-1160: OnOffParameters.transform_data: OK.Signature alignment and labeling are consistent with fit_effects_to_model_coords changes.
flixopt/modeling.py (1)
259-296: Consecutive duration constraints: OK post‑validation.Indexing with duration_dim is correct; initial condition uses provided previous_duration.
tests/test_models.py (3)
24-43: Annuity helper looks correct.Edge cases handled for remaining_years==1; non-positive years yield zero via loop guard.
93-113: Fixture: minimal system wiring is fine.Randomized profiles + buses/effects setup provides a workable sandbox.
115-184: API names: verify YearAwareInvestParameters symbols exist.Tests reference fx.YearAwareInvestParameters; ensure the project exposes these (vs InvestParameters/InvestTimingParameters). Otherwise imports will fail.
flixopt/features.py
Outdated
Comment on lines
82
to
163
| def _create_variables_and_constraints_with_years(self): | ||
| size_min, size_max = self.parameters.minimum_or_fixed_size, self.parameters.maximum_or_fixed_size | ||
| self.add_variables( | ||
| short_name='size', | ||
| lower=0, | ||
| upper=size_max, | ||
| coords=self._model.get_coords(['year', 'scenario']), | ||
| ) | ||
|
|
||
| self.add_variables( | ||
| binary=True, | ||
| coords=self._model.get_coords(['year', 'scenario']), | ||
| short_name='is_invested', | ||
| ) | ||
|
|
||
| BoundingPatterns.bounds_with_state( | ||
| self, | ||
| variable=self.size, | ||
| variable_state=self.is_invested, | ||
| bounds=(size_min, size_max), | ||
| ) | ||
| ######################################################################## | ||
| previous_size = self.parameters.previous_size if self.parameters.previous_size is not None else 0 | ||
| self.add_variables( | ||
| binary=True, | ||
| coords=self._model.get_coords(['year', 'scenario']), | ||
| short_name='size|investment_occurs', | ||
| ) | ||
| self.add_variables( | ||
| binary=True, | ||
| coords=self._model.get_coords(['year', 'scenario']), | ||
| short_name='size|decommissioning_occurs', | ||
| ) | ||
| BoundingPatterns.state_transition_bounds( | ||
| self, | ||
| state_variable=self.is_invested, | ||
| switch_on=self.investment_occurs, | ||
| switch_off=self.decommissioning_occurs, | ||
| name=self.is_invested.name, | ||
| previous_state=ModelingUtilitiesAbstract.to_binary(values=previous_size, epsilon=CONFIG.modeling.EPSILON), | ||
| coord='year', | ||
| ) | ||
| if self.parameters.optional: | ||
| self.add_constraints( | ||
| self.investment_occurs.sum('year') <= 1, | ||
| short_name='investment_occurs|once', | ||
| ) | ||
| else: | ||
| self.add_constraints( | ||
| self.investment_occurs.sum('year') == 1, | ||
| short_name='investment_occurs|once', | ||
| ) | ||
|
|
||
| self.add_constraints( | ||
| self.decommissioning_occurs.sum('year') <= 1, | ||
| short_name='decommissioning_occurs|once', | ||
| ) | ||
| ######################################################################## | ||
| self.add_variables( | ||
| coords=self._model.get_coords(['year', 'scenario']), | ||
| short_name='size|increase', | ||
| lower=0, | ||
| upper=size_max, | ||
| ) | ||
| self.add_variables( | ||
| coords=self._model.get_coords(['year', 'scenario']), | ||
| short_name='size|decrease', | ||
| lower=0, | ||
| upper=size_max, | ||
| ) | ||
| BoundingPatterns.link_changes_to_level_with_binaries( | ||
| self, | ||
| level_variable=self.size, | ||
| increase_variable=self.size_increase, | ||
| decrease_variable=self.size_decrease, | ||
| increase_binary=self.investment_occurs, | ||
| decrease_binary=self.decommissioning_occurs, | ||
| name=f'{self.label_of_element}|size|changes', | ||
| max_change=size_max, | ||
| initial_level=previous_size, | ||
| coord='year', | ||
| ) |
Contributor
There was a problem hiding this comment.
Fix previous_state dimensionality and scalar handling in state transition logic.
Two issues:
- previous_state passed to state_transition_bounds currently keeps a 'year' dim, causing a misalignment at the initial condition (LHS is indexed at year=0; RHS keeps full 'year' dim).
- ModelingUtilitiesAbstract.to_binary doesn’t robustly handle scalars; passing 0 (int) can raise due to len() usage upstream.
Action:
- Collapse previous_size to scenario-only before binary casting.
- Use the collapsed previous state consistently in both state_transition_bounds and link_changes_to_level_with_binaries initial level.
Apply this diff:
- previous_size = self.parameters.previous_size if self.parameters.previous_size is not None else 0
+ previous_size = self.parameters.previous_size if self.parameters.previous_size is not None else 0
+ previous_invested = ModelingUtilitiesAbstract.to_binary(
+ values=previous_size,
+ epsilon=CONFIG.modeling.EPSILON,
+ dims='scenario', # keep only scenario; drop year to match initial constraint broadcasting
+ )
...
- BoundingPatterns.state_transition_bounds(
+ BoundingPatterns.state_transition_bounds(
self,
state_variable=self.is_invested,
switch_on=self.investment_occurs,
switch_off=self.decommissioning_occurs,
name=self.is_invested.name,
- previous_state=ModelingUtilitiesAbstract.to_binary(values=previous_size, epsilon=CONFIG.modeling.EPSILON),
+ previous_state=previous_invested,
coord='year',
)
- if self.parameters.optional:
- self.add_constraints(
- self.investment_occurs.sum('year') <= 1,
- short_name='investment_occurs|once',
- )
- else:
- self.add_constraints(
- self.investment_occurs.sum('year') == 1,
- short_name='investment_occurs|once',
- )
+ if self.parameters.optional:
+ self.add_constraints(self.investment_occurs.sum('year') <= 1, short_name='investment_occurs|once')
+ else:
+ # If already invested before start, no new investment is required; otherwise exactly one invest is required.
+ required_invests = 1 - previous_invested
+ self.add_constraints(self.investment_occurs.sum('year') == required_invests, short_name='investment_occurs|once')
...
- BoundingPatterns.link_changes_to_level_with_binaries(
+ BoundingPatterns.link_changes_to_level_with_binaries(
self,
level_variable=self.size,
increase_variable=self.size_increase,
decrease_variable=self.size_decrease,
increase_binary=self.investment_occurs,
decrease_binary=self.decommissioning_occurs,
name=f'{self.label_of_element}|size|changes',
max_change=size_max,
- initial_level=previous_size,
+ initial_level=previous_size if getattr(previous_size, 'dims', None) != ('year', 'scenario') else previous_size.isel(year=0),
coord='year',
)📝 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
| def _create_variables_and_constraints_with_years(self): | |
| size_min, size_max = self.parameters.minimum_or_fixed_size, self.parameters.maximum_or_fixed_size | |
| self.add_variables( | |
| short_name='size', | |
| lower=0, | |
| upper=size_max, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| ) | |
| self.add_variables( | |
| binary=True, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='is_invested', | |
| ) | |
| BoundingPatterns.bounds_with_state( | |
| self, | |
| variable=self.size, | |
| variable_state=self.is_invested, | |
| bounds=(size_min, size_max), | |
| ) | |
| ######################################################################## | |
| previous_size = self.parameters.previous_size if self.parameters.previous_size is not None else 0 | |
| self.add_variables( | |
| binary=True, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|investment_occurs', | |
| ) | |
| self.add_variables( | |
| binary=True, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|decommissioning_occurs', | |
| ) | |
| BoundingPatterns.state_transition_bounds( | |
| self, | |
| state_variable=self.is_invested, | |
| switch_on=self.investment_occurs, | |
| switch_off=self.decommissioning_occurs, | |
| name=self.is_invested.name, | |
| previous_state=ModelingUtilitiesAbstract.to_binary(values=previous_size, epsilon=CONFIG.modeling.EPSILON), | |
| coord='year', | |
| ) | |
| if self.parameters.optional: | |
| self.add_constraints( | |
| self.investment_occurs.sum('year') <= 1, | |
| short_name='investment_occurs|once', | |
| ) | |
| else: | |
| self.add_constraints( | |
| self.investment_occurs.sum('year') == 1, | |
| short_name='investment_occurs|once', | |
| ) | |
| self.add_constraints( | |
| self.decommissioning_occurs.sum('year') <= 1, | |
| short_name='decommissioning_occurs|once', | |
| ) | |
| ######################################################################## | |
| self.add_variables( | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|increase', | |
| lower=0, | |
| upper=size_max, | |
| ) | |
| self.add_variables( | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|decrease', | |
| lower=0, | |
| upper=size_max, | |
| ) | |
| BoundingPatterns.link_changes_to_level_with_binaries( | |
| self, | |
| level_variable=self.size, | |
| increase_variable=self.size_increase, | |
| decrease_variable=self.size_decrease, | |
| increase_binary=self.investment_occurs, | |
| decrease_binary=self.decommissioning_occurs, | |
| name=f'{self.label_of_element}|size|changes', | |
| max_change=size_max, | |
| initial_level=previous_size, | |
| coord='year', | |
| ) | |
| def _create_variables_and_constraints_with_years(self): | |
| size_min, size_max = self.parameters.minimum_or_fixed_size, self.parameters.maximum_or_fixed_size | |
| self.add_variables( | |
| short_name='size', | |
| lower=0, | |
| upper=size_max, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| ) | |
| self.add_variables( | |
| binary=True, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='is_invested', | |
| ) | |
| BoundingPatterns.bounds_with_state( | |
| self, | |
| variable=self.size, | |
| variable_state=self.is_invested, | |
| bounds=(size_min, size_max), | |
| ) | |
| ######################################################################## | |
| previous_size = self.parameters.previous_size if self.parameters.previous_size is not None else 0 | |
| previous_invested = ModelingUtilitiesAbstract.to_binary( | |
| values=previous_size, | |
| epsilon=CONFIG.modeling.EPSILON, | |
| dims='scenario', # keep only scenario; drop year to match initial constraint broadcasting | |
| ) | |
| self.add_variables( | |
| binary=True, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|investment_occurs', | |
| ) | |
| self.add_variables( | |
| binary=True, | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|decommissioning_occurs', | |
| ) | |
| BoundingPatterns.state_transition_bounds( | |
| self, | |
| state_variable=self.is_invested, | |
| switch_on=self.investment_occurs, | |
| switch_off=self.decommissioning_occurs, | |
| name=self.is_invested.name, | |
| previous_state=previous_invested, | |
| coord='year', | |
| ) | |
| if self.parameters.optional: | |
| self.add_constraints(self.investment_occurs.sum('year') <= 1, short_name='investment_occurs|once') | |
| else: | |
| # If already invested before start, no new investment is required; otherwise exactly one invest is required. | |
| required_invests = 1 - previous_invested | |
| self.add_constraints(self.investment_occurs.sum('year') == required_invests, short_name='investment_occurs|once') | |
| self.add_constraints( | |
| self.decommissioning_occurs.sum('year') <= 1, | |
| short_name='decommissioning_occurs|once', | |
| ) | |
| ######################################################################## | |
| self.add_variables( | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|increase', | |
| lower=0, | |
| upper=size_max, | |
| ) | |
| self.add_variables( | |
| coords=self._model.get_coords(['year', 'scenario']), | |
| short_name='size|decrease', | |
| lower=0, | |
| upper=size_max, | |
| ) | |
| BoundingPatterns.link_changes_to_level_with_binaries( | |
| self, | |
| level_variable=self.size, | |
| increase_variable=self.size_increase, | |
| decrease_variable=self.size_decrease, | |
| increase_binary=self.investment_occurs, | |
| decrease_binary=self.decommissioning_occurs, | |
| name=f'{self.label_of_element}|size|changes', | |
| max_change=size_max, | |
| initial_level=previous_size if getattr(previous_size, 'dims', None) != ('year', 'scenario') else previous_size.isel(year=0), | |
| coord='year', | |
| ) |
🤖 Prompt for AI Agents
In flixopt/features.py around lines 82-163, previous_size retains a 'year'
dimension and raw ints can be passed into to_binary; collapse previous_size to
scenario-only (e.g., pick the initial year or reduce the 'year' axis so the
result has only 'scenario' coords) and normalize scalar ints into the same shape
before calling ModelingUtilitiesAbstract.to_binary; then pass that
collapsed/normalized previous_state to BoundingPatterns.state_transition_bounds
and also use the same collapsed value as initial_level for
link_changes_to_level_with_binaries so both APIs receive a scenario-only
previous state without a lingering 'year' dimension.
Comment on lines
184
to
194
| @staticmethod | ||
| def calculate_years_per_year(years: pd.Index, years_of_last_year: int | None = None) -> xr.DataArray: | ||
| """Calculate duration of each timestep as a 1D DataArray.""" | ||
| years_per_year = np.diff(years) | ||
| return xr.DataArray( | ||
| np.append(years_per_year, years_of_last_year or years_per_year[-1]), | ||
| coords={'year': years}, | ||
| dims='year', | ||
| name='years_per_year', | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
Handle single-year indexes in calculate_years_per_year.
np.diff(years) is empty for a single-year index, making years_per_year[-1] invalid.
Apply this diff:
@staticmethod
def calculate_years_per_year(years: pd.Index, years_of_last_year: int | None = None) -> xr.DataArray:
- """Calculate duration of each timestep as a 1D DataArray."""
- years_per_year = np.diff(years)
- return xr.DataArray(
- np.append(years_per_year, years_of_last_year or years_per_year[-1]),
- coords={'year': years},
- dims='year',
- name='years_per_year',
- )
+ """Calculate the represented duration for each year as a 1D DataArray."""
+ if len(years) == 1:
+ last = years_of_last_year if years_of_last_year is not None else 1
+ data = np.array([last])
+ else:
+ diffs = np.diff(years)
+ data = np.append(diffs, years_of_last_year if years_of_last_year is not None else diffs[-1])
+ return xr.DataArray(data, coords={'year': years}, dims='year', name='years_per_year')📝 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
| @staticmethod | |
| def calculate_years_per_year(years: pd.Index, years_of_last_year: int | None = None) -> xr.DataArray: | |
| """Calculate duration of each timestep as a 1D DataArray.""" | |
| years_per_year = np.diff(years) | |
| return xr.DataArray( | |
| np.append(years_per_year, years_of_last_year or years_per_year[-1]), | |
| coords={'year': years}, | |
| dims='year', | |
| name='years_per_year', | |
| ) | |
| @staticmethod | |
| def calculate_years_per_year(years: pd.Index, years_of_last_year: int | None = None) -> xr.DataArray: | |
| """Calculate the represented duration for each year as a 1D DataArray.""" | |
| if len(years) == 1: | |
| last = years_of_last_year if years_of_last_year is not None else 1 | |
| data = np.array([last]) | |
| else: | |
| diffs = np.diff(years) | |
| data = np.append(diffs, years_of_last_year if years_of_last_year is not None else diffs[-1]) | |
| return xr.DataArray(data, coords={'year': years}, dims='year', name='years_per_year') |
🤖 Prompt for AI Agents
In flixopt/flow_system.py around lines 184 to 194, calculate_years_per_year
fails for a single-year pd.Index because np.diff(years) is empty and
years_per_year[-1] is invalid; update the function to detect when len(years) ==
1 and return a DataArray of length 1 using years_of_last_year if provided or a
sensible default (e.g. 1.0), otherwise compute diffs as before and append the
last interval (use years_of_last_year if given); ensure resulting array has the
same dtype and coords/dims as before.
Comment on lines
229
to
231
| duration_dim: str = 'time', | ||
| duration_per_step: int | float | TemporalData = None, | ||
| previous_duration: TemporalData = 0, |
Contributor
There was a problem hiding this comment.
Make duration_per_step mandatory and validate early.
When None, the Big‑M and constraints fail. Add an explicit check.
Apply this diff:
def consecutive_duration_tracking(
@@
- duration_dim: str = 'time',
- duration_per_step: int | float | TemporalData = None,
+ duration_dim: str = 'time',
+ duration_per_step: int | float | TemporalData = None,
@@
- if not isinstance(model, Submodel):
+ if not isinstance(model, Submodel):
raise ValueError('ModelingPrimitives.sum_up_variable() can only be used with a Submodel')
+ if duration_per_step is None:
+ raise ValueError("duration_per_step must be provided for consecutive_duration_tracking()")📝 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
| duration_dim: str = 'time', | |
| duration_per_step: int | float | TemporalData = None, | |
| previous_duration: TemporalData = 0, | |
| duration_dim: str = 'time', | |
| duration_per_step: int | float | TemporalData = None, | |
| previous_duration: TemporalData = 0, | |
| if not isinstance(model, Submodel): | |
| raise ValueError('ModelingPrimitives.sum_up_variable() can only be used with a Submodel') | |
| if duration_per_step is None: | |
| raise ValueError( | |
| "duration_per_step must be provided for consecutive_duration_tracking()" | |
| ) |
🤖 Prompt for AI Agents
In flixopt/modeling.py around lines 229-231, the duration_per_step parameter is
currently optional (default None) which breaks Big‑M and constraint setup; make
duration_per_step mandatory by removing the "= None" default from the signature
(and update any callers) and add an early validation at the start of the
function that raises a clear ValueError if duration_per_step is None (and
optionally if it's non-positive or an invalid type), so the function fails fast
with an explicit message instead of producing invalid constraints.
Comment on lines
594
to
659
| @staticmethod | ||
| def continuous_transition_bounds( | ||
| model: Submodel, | ||
| continuous_variable: linopy.Variable, | ||
| switch_on: linopy.Variable, | ||
| switch_off: linopy.Variable, | ||
| name: str, | ||
| max_change: float | xr.DataArray, | ||
| previous_value: float | xr.DataArray = 0.0, | ||
| coord: str = 'time', | ||
| ) -> tuple[linopy.Constraint, linopy.Constraint, linopy.Constraint, linopy.Constraint]: | ||
| """ | ||
| Constrains a continuous variable to only change when switch variables are active. | ||
| Mathematical formulation: | ||
| -max_change * (switch_on[t] + switch_off[t]) <= continuous[t] - continuous[t-1] <= max_change * (switch_on[t] + switch_off[t]) ∀t > 0 | ||
| -max_change * (switch_on[0] + switch_off[0]) <= continuous[0] - previous_value <= max_change * (switch_on[0] + switch_off[0]) | ||
| switch_on[t], switch_off[t] ∈ {0, 1} | ||
| This ensures the continuous variable can only change when switch_on or switch_off is 1. | ||
| When both switches are 0, the variable must stay exactly constant. | ||
| Args: | ||
| model: The submodel to add constraints to | ||
| continuous_variable: The continuous variable to constrain | ||
| switch_on: Binary variable indicating when changes are allowed (typically transitions to active state) | ||
| switch_off: Binary variable indicating when changes are allowed (typically transitions to inactive state) | ||
| name: Base name for the constraints | ||
| max_change: Maximum possible change in the continuous variable (Big-M value) | ||
| previous_value: Initial value of the continuous variable before first period | ||
| coord: Coordinate name for time dimension | ||
| Returns: | ||
| Tuple of constraints: (transition_upper, transition_lower, initial_upper, initial_lower) | ||
| """ | ||
| if not isinstance(model, Submodel): | ||
| raise ValueError('BoundingPatterns.continuous_transition_bounds() can only be used with a Submodel') | ||
|
|
||
| # Transition constraints for t > 0: continuous variable can only change when switches are active | ||
| transition_upper = model.add_constraints( | ||
| continuous_variable.isel({coord: slice(1, None)}) - continuous_variable.isel({coord: slice(None, -1)}) | ||
| <= max_change * (switch_on.isel({coord: slice(1, None)}) + switch_off.isel({coord: slice(1, None)})), | ||
| name=f'{name}|transition_ub', | ||
| ) | ||
|
|
||
| transition_lower = model.add_constraints( | ||
| continuous_variable.isel({coord: slice(None, -1)}) - continuous_variable.isel({coord: slice(1, None)}) | ||
| <= max_change * (switch_on.isel({coord: slice(1, None)}) + switch_off.isel({coord: slice(1, None)})), | ||
| name=f'{name}|transition_lb', | ||
| ) | ||
|
|
||
| # Initial constraints for t = 0 | ||
| initial_upper = model.add_constraints( | ||
| continuous_variable.isel({coord: 0}) - previous_value | ||
| <= max_change * (switch_on.isel({coord: 0}) + switch_off.isel({coord: 0})), | ||
| name=f'{name}|initial_ub', | ||
| ) | ||
|
|
||
| initial_lower = model.add_constraints( | ||
| -continuous_variable.isel({coord: 0}) + previous_value | ||
| <= max_change * (switch_on.isel({coord: 0}) + switch_off.isel({coord: 0})), | ||
| name=f'{name}|initial_lb', | ||
| ) | ||
|
|
||
| return transition_upper, transition_lower, initial_upper, initial_lower | ||
|
|
Contributor
There was a problem hiding this comment.
Minor: error message references wrong class.
Message says BoundingPatterns.* but the method belongs to ModelingPrimitives.
Apply this diff:
- raise ValueError('BoundingPatterns.continuous_transition_bounds() can only be used with a Submodel')
+ raise ValueError('ModelingPrimitives.continuous_transition_bounds() can only be used with a Submodel')📝 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
| @staticmethod | |
| def continuous_transition_bounds( | |
| model: Submodel, | |
| continuous_variable: linopy.Variable, | |
| switch_on: linopy.Variable, | |
| switch_off: linopy.Variable, | |
| name: str, | |
| max_change: float | xr.DataArray, | |
| previous_value: float | xr.DataArray = 0.0, | |
| coord: str = 'time', | |
| ) -> tuple[linopy.Constraint, linopy.Constraint, linopy.Constraint, linopy.Constraint]: | |
| """ | |
| Constrains a continuous variable to only change when switch variables are active. | |
| Mathematical formulation: | |
| -max_change * (switch_on[t] + switch_off[t]) <= continuous[t] - continuous[t-1] <= max_change * (switch_on[t] + switch_off[t]) ∀t > 0 | |
| -max_change * (switch_on[0] + switch_off[0]) <= continuous[0] - previous_value <= max_change * (switch_on[0] + switch_off[0]) | |
| switch_on[t], switch_off[t] ∈ {0, 1} | |
| This ensures the continuous variable can only change when switch_on or switch_off is 1. | |
| When both switches are 0, the variable must stay exactly constant. | |
| Args: | |
| model: The submodel to add constraints to | |
| continuous_variable: The continuous variable to constrain | |
| switch_on: Binary variable indicating when changes are allowed (typically transitions to active state) | |
| switch_off: Binary variable indicating when changes are allowed (typically transitions to inactive state) | |
| name: Base name for the constraints | |
| max_change: Maximum possible change in the continuous variable (Big-M value) | |
| previous_value: Initial value of the continuous variable before first period | |
| coord: Coordinate name for time dimension | |
| Returns: | |
| Tuple of constraints: (transition_upper, transition_lower, initial_upper, initial_lower) | |
| """ | |
| if not isinstance(model, Submodel): | |
| raise ValueError('BoundingPatterns.continuous_transition_bounds() can only be used with a Submodel') | |
| # Transition constraints for t > 0: continuous variable can only change when switches are active | |
| transition_upper = model.add_constraints( | |
| continuous_variable.isel({coord: slice(1, None)}) - continuous_variable.isel({coord: slice(None, -1)}) | |
| <= max_change * (switch_on.isel({coord: slice(1, None)}) + switch_off.isel({coord: slice(1, None)})), | |
| name=f'{name}|transition_ub', | |
| ) | |
| transition_lower = model.add_constraints( | |
| continuous_variable.isel({coord: slice(None, -1)}) - continuous_variable.isel({coord: slice(1, None)}) | |
| <= max_change * (switch_on.isel({coord: slice(1, None)}) + switch_off.isel({coord: slice(1, None)})), | |
| name=f'{name}|transition_lb', | |
| ) | |
| # Initial constraints for t = 0 | |
| initial_upper = model.add_constraints( | |
| continuous_variable.isel({coord: 0}) - previous_value | |
| <= max_change * (switch_on.isel({coord: 0}) + switch_off.isel({coord: 0})), | |
| name=f'{name}|initial_ub', | |
| ) | |
| initial_lower = model.add_constraints( | |
| -continuous_variable.isel({coord: 0}) + previous_value | |
| <= max_change * (switch_on.isel({coord: 0}) + switch_off.isel({coord: 0})), | |
| name=f'{name}|initial_lb', | |
| ) | |
| return transition_upper, transition_lower, initial_upper, initial_lower | |
| if not isinstance(model, Submodel): | |
| raise ValueError('ModelingPrimitives.continuous_transition_bounds() can only be used with a Submodel') |
🤖 Prompt for AI Agents
flixopt/modeling.py lines 594-659: The ValueError raised when model is not a
Submodel references BoundingPatterns but this static method belongs to
ModelingPrimitives; update the exception message to reference ModelingPrimitives
(e.g. "ModelingPrimitives.continuous_transition_bounds() can only be used with a
Submodel") so the class name in the error matches the actual owner of the
method.
# Conflicts: # CHANGELOG.md # flixopt/components.py # flixopt/effects.py # flixopt/elements.py # flixopt/features.py # flixopt/flow_system.py # flixopt/interface.py # flixopt/modeling.py # flixopt/structure.py
Member
Author
Renaming InvestParametersCurrent vs. Proposed Naming
Benefits of Proposed ChangesClarity: Each parameter name explicitly states what it represents without requiring domain knowledge of “fix” vs “specific” terminology. Consistency: Uses parallel structure with “effects of [action]” pattern throughout. Accessibility: More intuitive for users who aren’t familiar with optimization modeling conventions. Alignment: Example Usageinvestment = InvestParameters(
effects_of_investment={...}, # one-time effects when investing
effects_of_investment_per_size={...}, # effects that scale with size
effects_of_retirement={...} # effects when retiring the asset
) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Make the InvestmentParameters work properly with multiple years
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
New Features
Changes
Documentation
Tests