Add FlowSystemStatus enum and restructure validation architecture#598
Merged
FBumann merged 9 commits intofeature/element-data-classesfrom Feb 5, 2026
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR implements a comprehensive validation architecture refactoring by introducing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Classes Updated with validate_config() ┌──────────────────┬───────────────┬────────────────────────────────────────────────────────────────────────────┐ │ Class │ Location │ Config Checks │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ Component │ elements.py │ unique flow labels, status→flows.size │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ Bus │ elements.py │ no flows connected │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ Flow │ elements.py │ status→size, fixed_profile→size, load_factor→size, previous_flow_rate type │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ Effect │ effects.py │ period dimension for over_periods constraints │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ EffectCollection │ effects.py │ circular loops, unknown effect refs │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ LinearConverter │ components.py │ conversion_factors XOR piecewise, degrees_of_freedom, flow refs │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ Storage │ components.py │ initial_charge_state string, balanced→InvestParams, final_charge→capacity │ ├──────────────────┼───────────────┼────────────────────────────────────────────────────────────────────────────┤ │ Transmission │ components.py │ bus consistency, balanced→InvestParams │ └──────────────────┴───────────────┴────────────────────────────────────────────────────────────────────────────┘ *Data Classes with validate() ┌───────────────────┬────────────┬─────────────────────────────────────────────────────────────────────────┐ │ Class │ Location │ DataArray Checks │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ FlowsData │ batched.py │ relative_min ≤ max, size required for bounds │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ StoragesData │ batched.py │ capacity for relative bounds, initial vs capacity, balanced size compat │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ BusesData │ batched.py │ imbalance_penalty == 0 warning │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ TransmissionsData │ batched.py │ balanced size compatibility │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ EffectsData │ batched.py │ delegates to validate_config() │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ ComponentsData │ batched.py │ delegates to validate_config() │ ├───────────────────┼────────────┼─────────────────────────────────────────────────────────────────────────┤ │ ConvertersData │ batched.py │ delegates to validate_config() │ └───────────────────┴────────────┴─────────────────────────────────────────────────────────────────────────┘ Updated FlowSystem _run_plausibility_checks() now creates temporary *Data instances and calls validate() on each, which handles both config and DataArray checks in a centralized way.
What Changed
*1. BatchedAccessor now caches all Data classes:
class BatchedAccessor:
@Property
def flows(self) -> FlowsData: ...
@Property
def storages(self) -> StoragesData: ...
@Property
def intercluster_storages(self) -> StoragesData: ...
@Property
def buses(self) -> BusesData: ...
@Property
def effects(self) -> EffectsData: ...
@Property
def components(self) -> ComponentsData: ...
@Property
def converters(self) -> ConvertersData: ...
@Property
def transmissions(self) -> TransmissionsData: ...
2. FlowSystemModel.build_model() now uses cached instances:
batched = self.flow_system.batched
self.effects = EffectsModel(self, batched.effects) # reuses cached
self._flows_model = FlowsModel(self, batched.flows) # reuses cached
# etc.
3. _run_plausibility_checks() simplified:
batched = self.batched
batched.flows.validate()
batched.buses.validate()
# etc.
Benefits
- No duplicate creation: Same *Data objects used for validation AND model building
- Early validation: Errors caught during connect_and_transform()
- Proper invalidation: _batched = None when status drops below CONNECTED
- Cleaner code: No temporary object creation in validation or build_model
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
FBumann
changed the title
When a Bus has no flows connected, FlowsData.validate() crashed with a cryptic TypeError instead of raising a clear ValueError message. Root Cause In _run_validation(), the validation order was: 1. batched.flows.validate() ← crashed on empty DataArray operations 2. batched.buses.validate() ← would have caught the error
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
…-classes+validation # Conflicts: # flixopt/elements.py
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
This PR introduces explicit lifecycle status tracking for FlowSystem and restructures the validation architecture for better separation of concerns.
Key Changes
FlowSystemStatus Enum (
flow_system_status.py)FlowSystemStatusIntEnum with 5 lifecycle states:INITIALIZED,CONNECTED,MODEL_CREATED,MODEL_BUILT,SOLVEDinvalidate_to_status()that clears appropriate caches when status dropsis_locked,_connected_and_transformed) with derived properties from statusValidation Architecture Split
validate_config()on Element classes (Flow, Storage, Component, Bus, Effect, etc.) - simple checks that don't require DataArrays (None checks, isinstance, basic constraints)validate()on*Dataclasses (FlowsData, StoragesData, etc.) - batched DataArray validation checksBatchedAccessor Caching
*Dataobjects (flows, storages, buses, effects, components, converters, transmissions) are now cached inBatchedAccessor_run_validation()andbuild_model(), avoiding duplicate creation_reset()during status invalidationBug Fix: Validation Order (commit a5157a2)
Buswith no flows caused crypticTypeErrorinstead of clearValueErrorFlowsData.validate()for empty flows edge caseType of Change
Testing
Checklist