Feature/simplify structure

CHANGELOG.md

@@ -51,31 +51,35 @@ If upgrading from v2.x, see the [v3.0.0 release notes](https://github.com/flixOp
 
 ## [Unreleased] - ????-??-??
 
-**Summary**:
+**Summary**: Internal architecture improvements to simplify FlowSystem-Element coupling and eliminate circular dependencies.
 
 If upgrading from v2.x, see the [v3.0.0 release notes](https://github.com/flixOpt/flixOpt/releases/tag/v3.0.0) and [Migration Guide](https://flixopt.github.io/flixopt/latest/user-guide/migration-guide-v3/).
 
 ### ✨ Added
-
-### 💥 Breaking Changes
+- **Auto-modeling**: `Calculation.solve()` now automatically calls `do_modeling()` if not already done, making the explicit `do_modeling()` call optional for simpler workflows
+- **System validation**: Added `_validate_system_integrity()` to validate cross-element references (e.g., Flow.bus) immediately after transformation, providing clearer error messages
+- **Element registration validation**: Added checks to prevent elements from being assigned to multiple FlowSystems simultaneously
+- **Helper methods in Interface base class**: Added `_fit_coords()` and `_fit_effect_coords()` convenience wrappers for cleaner data transformation code
+- **FlowSystem property in Interface**: Added `flow_system` property to access the linked FlowSystem with clear error messages if not yet linked
 
 ### ♻️ Changed
-
-### 🗑️ Deprecated
-
-### 🔥 Removed
+- **Refactored FlowSystem-Element coupling**:
+    - Introduced `_set_flow_system()` method in Interface base class to propagate FlowSystem reference to nested Interface objects
+    - Each Interface subclass now explicitly propagates the reference to its nested interfaces (e.g., Component → OnOffParameters, Flow → InvestParameters)
+    - Elements can now access FlowSystem via `self.flow_system` property instead of passing it through every method call
+- **Simplified transform_data() signature**: Removed `flow_system` parameter from `transform_data()` methods - FlowSystem reference is now accessed via `self.flow_system` property
+- **Two-phase modeling pattern within _do_modeling()**: Clarified the pattern where `_do_modeling()` creates nested submodels first (so their variables exist), then creates constraints that reference those variables - eliminates circular dependencies in Submodel architecture
+- **Improved cache invalidation**: Cache invalidation in `add_elements()` now happens once after all additions rather than per element
+- **Better logging**: Centralized element registration logging to show element type and full label
 
 ### 🐛 Fixed
-
-### 🔒 Security
-
-### 📦 Dependencies
-
-### 📝 Docs
+- Fixed inconsistent argument passing in `_fit_effect_coords()` - standardized all calls to use named arguments (`prefix=`, `effect_values=`, `suffix=`) instead of mix of positional and named arguments
 
 ### 👷 Development
-
-### 🚧 Known Issues
+- **Eliminated circular dependencies**: Implemented two-phase modeling pattern within `_do_modeling()` where nested submodels are created first (creating their variables), then constraints are created that can safely reference those submodel variables
+- Added comprehensive docstrings to `_do_modeling()` methods explaining the pattern: "Create variables, constraints, and nested submodels"
+- Added missing type hints throughout the codebase
+- Improved code organization by making FlowSystem reference propagation explicit and traceable
 
 ---
 

flixopt/calculation.py

@@ -98,8 +98,6 @@ def __init__(
             raise NotADirectoryError(f'Path {self.folder} exists and is not a directory.')
         self.folder.mkdir(parents=False, exist_ok=True)
 
-        self._modeled = False
-
     @property
     def main_results(self) -> dict[str, int | float | dict]:
         from flixopt.features import InvestmentModel
@@ -228,6 +226,11 @@ def fix_sizes(self, ds: xr.Dataset, decimal_rounding: int | None = 5) -> FullCal
     def solve(
         self, solver: _Solver, log_file: pathlib.Path | None = None, log_main_results: bool | None = None
     ) -> FullCalculation:
+        # Auto-call do_modeling() if not already done
+        if not self.modeled:
+            logger.info('Model not yet created. Calling do_modeling() automatically.')
+            self.do_modeling()
+
         t_start = timeit.default_timer()
 
         self.model.solve(

flixopt/components.py

@@ -181,6 +181,12 @@ def create_model(self, model: FlowSystemModel) -> LinearConverterModel:
         self.submodel = LinearConverterModel(model, self)
         return self.submodel
 
+    def _set_flow_system(self, flow_system) -> None:
+        """Propagate flow_system reference to parent Component and piecewise_conversion."""
+        super()._set_flow_system(flow_system)
+        if self.piecewise_conversion is not None:
+            self.piecewise_conversion._set_flow_system(flow_system)
+
     def _plausibility_checks(self) -> None:
         super()._plausibility_checks()
         if not self.conversion_factors and not self.piecewise_conversion:
@@ -211,23 +217,23 @@ def _plausibility_checks(self) -> None:
                         f'({flow.label_full}).'
                     )
 
-    def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None:
+    def transform_data(self, name_prefix: str = '') -> None:
         prefix = '|'.join(filter(None, [name_prefix, self.label_full]))
-        super().transform_data(flow_system, prefix)
+        super().transform_data(prefix)
         if self.conversion_factors:
-            self.conversion_factors = self._transform_conversion_factors(flow_system)
+            self.conversion_factors = self._transform_conversion_factors()
         if self.piecewise_conversion:
             self.piecewise_conversion.has_time_dim = True
-            self.piecewise_conversion.transform_data(flow_system, f'{prefix}|PiecewiseConversion')
+            self.piecewise_conversion.transform_data(f'{prefix}|PiecewiseConversion')
 
-    def _transform_conversion_factors(self, flow_system: FlowSystem) -> list[dict[str, xr.DataArray]]:
+    def _transform_conversion_factors(self) -> list[dict[str, xr.DataArray]]:
         """Converts all conversion factors to internal datatypes"""
         list_of_conversion_factors = []
         for idx, conversion_factor in enumerate(self.conversion_factors):
             transformed_dict = {}
             for flow, values in conversion_factor.items():
                 # TODO: Might be better to use the label of the component instead of the flow
-                ts = flow_system.fit_to_model_coords(f'{self.flows[flow].label_full}|conversion_factor{idx}', values)
+                ts = self._fit_coords(f'{self.flows[flow].label_full}|conversion_factor{idx}', values)
                 if ts is None:
                     raise PlausibilityError(f'{self.label_full}: conversion factor for flow "{flow}" must not be None')
                 transformed_dict[flow] = ts
@@ -433,46 +439,48 @@ def create_model(self, model: FlowSystemModel) -> StorageModel:
         self.submodel = StorageModel(model, self)
         return self.submodel
 
-    def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None:
+    def _set_flow_system(self, flow_system) -> None:
+        """Propagate flow_system reference to parent Component and capacity_in_flow_hours if it's InvestParameters."""
+        super()._set_flow_system(flow_system)
+        if isinstance(self.capacity_in_flow_hours, InvestParameters):
+            self.capacity_in_flow_hours._set_flow_system(flow_system)
+
+    def transform_data(self, name_prefix: str = '') -> None:
         prefix = '|'.join(filter(None, [name_prefix, self.label_full]))
-        super().transform_data(flow_system, prefix)
-        self.relative_minimum_charge_state = flow_system.fit_to_model_coords(
-            f'{prefix}|relative_minimum_charge_state',
-            self.relative_minimum_charge_state,
+        super().transform_data(prefix)
+        self.relative_minimum_charge_state = self._fit_coords(
+            f'{prefix}|relative_minimum_charge_state', self.relative_minimum_charge_state
         )
-        self.relative_maximum_charge_state = flow_system.fit_to_model_coords(
-            f'{prefix}|relative_maximum_charge_state',
-            self.relative_maximum_charge_state,
-        )
-        self.eta_charge = flow_system.fit_to_model_coords(f'{prefix}|eta_charge', self.eta_charge)
-        self.eta_discharge = flow_system.fit_to_model_coords(f'{prefix}|eta_discharge', self.eta_discharge)
-        self.relative_loss_per_hour = flow_system.fit_to_model_coords(
-            f'{prefix}|relative_loss_per_hour', self.relative_loss_per_hour
+        self.relative_maximum_charge_state = self._fit_coords(
+            f'{prefix}|relative_maximum_charge_state', self.relative_maximum_charge_state
         )
+        self.eta_charge = self._fit_coords(f'{prefix}|eta_charge', self.eta_charge)
+        self.eta_discharge = self._fit_coords(f'{prefix}|eta_discharge', self.eta_discharge)
+        self.relative_loss_per_hour = self._fit_coords(f'{prefix}|relative_loss_per_hour', self.relative_loss_per_hour)
         if not isinstance(self.initial_charge_state, str):
-            self.initial_charge_state = flow_system.fit_to_model_coords(
+            self.initial_charge_state = self._fit_coords(
                 f'{prefix}|initial_charge_state', self.initial_charge_state, dims=['period', 'scenario']
             )
-        self.minimal_final_charge_state = flow_system.fit_to_model_coords(
+        self.minimal_final_charge_state = self._fit_coords(
             f'{prefix}|minimal_final_charge_state', self.minimal_final_charge_state, dims=['period', 'scenario']
         )
-        self.maximal_final_charge_state = flow_system.fit_to_model_coords(
+        self.maximal_final_charge_state = self._fit_coords(
             f'{prefix}|maximal_final_charge_state', self.maximal_final_charge_state, dims=['period', 'scenario']
         )
-        self.relative_minimum_final_charge_state = flow_system.fit_to_model_coords(
+        self.relative_minimum_final_charge_state = self._fit_coords(
             f'{prefix}|relative_minimum_final_charge_state',
             self.relative_minimum_final_charge_state,
             dims=['period', 'scenario'],
         )
-        self.relative_maximum_final_charge_state = flow_system.fit_to_model_coords(
+        self.relative_maximum_final_charge_state = self._fit_coords(
             f'{prefix}|relative_maximum_final_charge_state',
             self.relative_maximum_final_charge_state,
             dims=['period', 'scenario'],
         )
         if isinstance(self.capacity_in_flow_hours, InvestParameters):
-            self.capacity_in_flow_hours.transform_data(flow_system, f'{prefix}|InvestParameters')
+            self.capacity_in_flow_hours.transform_data(f'{prefix}|InvestParameters')
         else:
-            self.capacity_in_flow_hours = flow_system.fit_to_model_coords(
+            self.capacity_in_flow_hours = self._fit_coords(
                 f'{prefix}|capacity_in_flow_hours', self.capacity_in_flow_hours, dims=['period', 'scenario']
             )
 
@@ -719,11 +727,11 @@ def create_model(self, model) -> TransmissionModel:
         self.submodel = TransmissionModel(model, self)
         return self.submodel
 
-    def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None:
+    def transform_data(self, name_prefix: str = '') -> None:
         prefix = '|'.join(filter(None, [name_prefix, self.label_full]))
-        super().transform_data(flow_system, prefix)
-        self.relative_losses = flow_system.fit_to_model_coords(f'{prefix}|relative_losses', self.relative_losses)
-        self.absolute_losses = flow_system.fit_to_model_coords(f'{prefix}|absolute_losses', self.absolute_losses)
+        super().transform_data(prefix)
+        self.relative_losses = self._fit_coords(f'{prefix}|relative_losses', self.relative_losses)
+        self.absolute_losses = self._fit_coords(f'{prefix}|absolute_losses', self.absolute_losses)
 
 
 class TransmissionModel(ComponentModel):
@@ -738,7 +746,7 @@ def __init__(self, model: FlowSystemModel, element: Transmission):
         super().__init__(model, element)
 
     def _do_modeling(self):
-        """Initiates all FlowModels"""
+        """Create transmission efficiency equations and optional absolute loss constraints for both flow directions"""
         super()._do_modeling()
 
         # first direction
@@ -779,8 +787,10 @@ def __init__(self, model: FlowSystemModel, element: LinearConverter):
         super().__init__(model, element)
 
     def _do_modeling(self):
+        """Create linear conversion equations or piecewise conversion constraints between input and output flows"""
         super()._do_modeling()
-        # conversion_factors:
+
+        # Create conversion factor constraints if specified
         if self.element.conversion_factors:
             all_input_flows = set(self.element.inputs)
             all_output_flows = set(self.element.outputs)
@@ -826,8 +836,10 @@ def __init__(self, model: FlowSystemModel, element: Storage):
         super().__init__(model, element)
 
     def _do_modeling(self):
+        """Create charge state variables, energy balance equations, and optional investment submodels"""
         super()._do_modeling()
 
+        # Create variables
         lb, ub = self._absolute_charge_state_bounds
         self.add_variables(
             lower=lb,
@@ -838,6 +850,7 @@ def _do_modeling(self):
 
         self.add_variables(coords=self._model.get_coords(), short_name='netto_discharge')
 
+        # Create constraints (can now access flow.submodel.flow_rate)
         # netto_discharge:
         # eq: nettoFlow(t) - discharging(t) + charging(t) = 0
         self.add_constraints(
@@ -862,6 +875,7 @@ def _do_modeling(self):
             short_name='charge_state',
         )
 
+        # Create InvestmentModel and bounding constraints for investment
         if isinstance(self.element.capacity_in_flow_hours, InvestParameters):
             self.add_submodels(
                 InvestmentModel(
@@ -883,6 +897,7 @@ def _do_modeling(self):
         # Initial charge state
         self._initial_and_final_charge_state()
 
+        # Balanced sizes
         if self.element.balanced:
             self.add_constraints(
                 self.element.charging.submodel._investment.size * 1

flixopt/effects.py

@@ -339,43 +339,40 @@ def maximum_operation_per_hour(self, value):
         )
         self.maximum_per_hour = value
 
-    def transform_data(self, flow_system: FlowSystem, name_prefix: str = '') -> None:
+    def transform_data(self, name_prefix: str = '') -> None:
         prefix = '|'.join(filter(None, [name_prefix, self.label_full]))
-        self.minimum_per_hour = flow_system.fit_to_model_coords(f'{prefix}|minimum_per_hour', self.minimum_per_hour)
+        self.minimum_per_hour = self._fit_coords(f'{prefix}|minimum_per_hour', self.minimum_per_hour)
+        self.maximum_per_hour = self._fit_coords(f'{prefix}|maximum_per_hour', self.maximum_per_hour)
 
-        self.maximum_per_hour = flow_system.fit_to_model_coords(f'{prefix}|maximum_per_hour', self.maximum_per_hour)
-
-        self.share_from_temporal = flow_system.fit_effects_to_model_coords(
-            label_prefix=None,
+        self.share_from_temporal = self._fit_effect_coords(
+            prefix=None,
             effect_values=self.share_from_temporal,
-            label_suffix=f'(temporal)->{prefix}(temporal)',
+            suffix=f'(temporal)->{prefix}(temporal)',
             dims=['time', 'period', 'scenario'],
         )
-        self.share_from_periodic = flow_system.fit_effects_to_model_coords(
-            label_prefix=None,
+        self.share_from_periodic = self._fit_effect_coords(
+            prefix=None,
             effect_values=self.share_from_periodic,
-            label_suffix=f'(periodic)->{prefix}(periodic)',
+            suffix=f'(periodic)->{prefix}(periodic)',
             dims=['period', 'scenario'],
         )
 
-        self.minimum_temporal = flow_system.fit_to_model_coords(
+        self.minimum_temporal = self._fit_coords(
             f'{prefix}|minimum_temporal', self.minimum_temporal, dims=['period', 'scenario']
         )
-        self.maximum_temporal = flow_system.fit_to_model_coords(
+        self.maximum_temporal = self._fit_coords(
             f'{prefix}|maximum_temporal', self.maximum_temporal, dims=['period', 'scenario']
         )
-        self.minimum_periodic = flow_system.fit_to_model_coords(
+        self.minimum_periodic = self._fit_coords(
             f'{prefix}|minimum_periodic', self.minimum_periodic, dims=['period', 'scenario']
         )
-        self.maximum_periodic = flow_system.fit_to_model_coords(
+        self.maximum_periodic = self._fit_coords(
             f'{prefix}|maximum_periodic', self.maximum_periodic, dims=['period', 'scenario']
         )
-        self.minimum_total = flow_system.fit_to_model_coords(
-            f'{prefix}|minimum_total',
-            self.minimum_total,
-            dims=['period', 'scenario'],
+        self.minimum_total = self._fit_coords(
+            f'{prefix}|minimum_total', self.minimum_total, dims=['period', 'scenario']
         )
-        self.maximum_total = flow_system.fit_to_model_coords(
+        self.maximum_total = self._fit_coords(
             f'{prefix}|maximum_total', self.maximum_total, dims=['period', 'scenario']
         )
 
@@ -396,6 +393,9 @@ def __init__(self, model: FlowSystemModel, element: Effect):
         super().__init__(model, element)
 
     def _do_modeling(self):
+        """Create variables, constraints, and nested submodels"""
+        super()._do_modeling()
+
         self.total: linopy.Variable | None = None
         self.periodic: ShareAllocationModel = self.add_submodels(
             ShareAllocationModel(
@@ -661,16 +661,26 @@ def add_share_to_penalty(self, name: str, expression: linopy.LinearExpression) -
         self.penalty.add_share(name, expression, dims=())
 
     def _do_modeling(self):
+        """Create variables, constraints, and nested submodels"""
         super()._do_modeling()
+
+        # Create EffectModel for each effect
         for effect in self.effects.values():
             effect.create_model(self._model)
+
+        # Create penalty allocation model
         self.penalty = self.add_submodels(
             ShareAllocationModel(self._model, dims=(), label_of_element='Penalty'),
             short_name='penalty',
         )
 
+        # Add cross-effect shares
         self._add_share_between_effects()
 
+        # Set objective
+        # Note: penalty.total is used here, but penalty shares from buses/components
+        # are added later via add_share_to_penalty(). The ShareAllocationModel supports
+        # this pattern - shares can be added after the objective is defined.
         self._model.add_objective(
             (self.effects.objective_effect.submodel.total * self._model.weights).sum() + self.penalty.total.sum()
         )