Consolidate data stacking into *Data classes #605
Merged
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
Summary Phase 1: TransmissionsData - Added flow_ids parameter to TransmissionsData.__init__ - Moved 12 cached properties from TransmissionsModel to TransmissionsData: bidirectional_ids, balanced_ids, _build_flow_mask(), in1_mask, out1_mask, in2_mask, out2_mask, relative_losses, absolute_losses, has_absolute_losses_mask, transmissions_with_abs_losses - Updated TransmissionsModel.create_constraints() to use self.data.* - Updated BatchedAccessor.transmissions to pass flow_ids Phase 2: BusesData - Added balance_coefficients cached property to BusesData - Updated BusesModel.create_constraints() to use self.data.balance_coefficients Phase 3: ConvertersData - Added flow_ids and timesteps parameters to ConvertersData.__init__ - Moved 13 cached properties from ConvertersModel to ConvertersData: factor_element_ids, max_equations, equation_mask, signed_coefficients, n_equations_per_converter, piecewise_element_ids, piecewise_segment_counts_dict, piecewise_max_segments, piecewise_segment_mask, piecewise_flow_breakpoints, piecewise_segment_counts_array, piecewise_breakpoints - Updated ConvertersModel methods to use self.data.* - Removed unused defaultdict and stack_along_dim imports from elements.py Phase 4: ComponentsData - Added flows_data, effect_ids, timestep_duration parameters to ComponentsData.__init__ - Moved 6 cached properties from ComponentsModel to ComponentsData: with_prevent_simultaneous, status_params, previous_status_dict, status_data, flow_mask, flow_count - Moved _get_previous_status_for_component() helper to ComponentsData - Updated ComponentsModel to use self.data.* throughout Bug Fix - Discovered a stale cache issue: FlowsData.previous_states could be cached before all previous_flow_rate values were set (e.g., in from_old_results). Fixed by having ComponentsData._get_previous_status_for_component() compute previous status directly from flow attributes instead of going through the potentially-stale FlowsData.previous_states cache.
…n2_mask cached properties to TransmissionsData. TransmissionsModel now uses these instead of calling the private d._build_flow_mask(). 2. EffectsModel/elements: Confirmed not a bug — EffectsModel does not inherit from TypeModel and never accesses .elements on its data, so EffectsData not having elements is fine. 3. FlowsData.dim_name: Changed from @cached_property to @Property to match all other Data classes.
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 Use the checkbox below for a quick retry:
✨ 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 |
FBumann
changed the title
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.
Summary
BusesData,ComponentsData,ConvertersData, andTransmissionsData— aligning them withFlowsData/StoragesData/EffectsDatawhich already followed this patternBusesModel,ComponentsModel,ConvertersModel,TransmissionsModel) are now pure consumers ofself.data.*, no longer responsible for stacking element parameters into arrayselements.py, ~460 lines added tobatched.pyWhat moved where
BusesDatabalance_coefficientsComponentsDatastatus_data,status_params,previous_status_dict,flow_mask,flow_count,with_prevent_simultaneousConvertersDatasigned_coefficients,equation_mask,max_equations,n_equations_per_converter,piecewise_*(8 properties)TransmissionsDatarelative_losses,absolute_losses,in1_mask,out1_mask,in2_mask,out2_mask,has_absolute_losses_mask,transmissions_with_abs_losses,bidirectional_ids,balanced_ids,balanced_in1_mask,balanced_in2_maskMinor cleanups
FlowsData.dim_name:@cached_property→@property(consistency with all other Data classes)TransmissionsModel: No longer calls privated._build_flow_mask()— uses pre-computedbalanced_in1_mask/balanced_in2_maskType of Change
Testing
Checklist