-
Notifications
You must be signed in to change notification settings - Fork 75
Introducing multi-unit components #3944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Adding a ComponentCluster class that mimics the interface of the Component class (same attributes and method signatures). The class is non-functional at the moment.
Added logic to the operate and calculate_cost methods: operation of the component cluster distributes energy demand across units using a cascading (water-filling) approach. Costs (investment and O&M costs) are simply aggregated across all units in the cluster.
The ComponentCluster class would have made inheritance for the Active and PassiveComponent classes extremely complicated, so instead I opt for the introduction a "units" attribute directly in the component class, that will allow the creation of multiple units of the same component without the need for new complex code structures.
Implement the new potential multi-unit structure of the component class as part of the initialisation method of each of the component subclasses. This is implementation is a bit long an repetitive but allows to keep access to the class the same as it previously was.
Make sure the cost of multi-unit components is calculated correctly: aggregate of all units' cost values.
Implement a first mode of multi-unit component operation my copying the "cascading" operation used in the wider supply system.
WalkthroughIntroduced multi-unit support infrastructure to the Component class, enabling components to aggregate multiple unit instances when single-unit capacity proves insufficient. Added cascading operation flows for distributed load handling, cost aggregation logic, and capacity detection utilities across various HVAC and energy components. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Component
participant CascadeOp as _cascade_operation_<br/>(active/passive)
participant Unit1 as Unit 1<br/>operate()
participant Unit2 as Unit 2<br/>operate()
participant Aggregator as Aggregation
Caller->>Component: operate(main_energy_flow, ...)
alt n_units > 1
Component->>Component: Check multi-unit flag
Component->>CascadeOp: Call _cascade_operation_*()
rect rgb(200, 220, 240)
note over CascadeOp: Water-filling distribution
CascadeOp->>Unit1: operate(allocated_flow_1)
Unit1-->>CascadeOp: result_1
CascadeOp->>Unit2: operate(remaining_flow)
Unit2-->>CascadeOp: result_2
end
CascadeOp->>Aggregator: Aggregate outputs
Aggregator-->>CascadeOp: aggregated_result
CascadeOp-->>Component: aggregated_result
else n_units == 1
Component->>Component: Standard single-unit operate()
end
Component-->>Caller: result
sequenceDiagram
participant Init as Component.__init__()
participant Checker as _check_multi_unit_<br/>needed()
participant Database as Components DB
participant Setup as _setup_multi_<br/>unit_aggregate()
Init->>Checker: Check if multi-unit needed<br/>(model_code, capacity)
Checker->>Database: Query max_capacity_for_model()
Database-->>Checker: model_max_capacity
rect rgb(240, 220, 200)
note over Checker: Determine unit count
Checker-->>Init: (needs_multi_unit, n_units, unit_capacity)
end
alt needs_multi_unit == True
Init->>Init: Create n_units instances
Init->>Setup: Aggregate and propagate properties
Setup->>Setup: Recompute aggregated costs
Setup-->>Init: Multi-unit component ready
else
Init->>Init: Single-unit initialization
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cea/optimization_new/component.py (2)
363-369: Optimize sample flow creation in passive cascade.The passive cascade creates a sample energy flow (lines 364-369) purely to extract output characteristics (
input_category,output_category,energy_carrier.code) for the final aggregated flow (lines 372-377). This requires an extra invocation ofunit_operate_methodwith a zero-demand profile.Consider one of these alternatives:
- Extract characteristics from the first unit's actual operation result instead of creating a separate sample flow
- Define output characteristics as class attributes or derive them from input characteristics and unit properties
Apply this diff to use the first unit's operation result:
- # Create the aggregated energy flow with the same characteristics as the first unit's output - # Get a sample unit output to extract characteristics - sample_unit = self.units[0] - sample_flow = unit_operate_method(sample_unit, EnergyFlow( - energy_flow.input_category, - energy_flow.output_category, - energy_flow.energy_carrier.code, - pd.Series(0.0, index=total_demand_profile.index) - ), *args) - - # Create final aggregated flow - aggregated_flow = EnergyFlow( - sample_flow.input_category, - sample_flow.output_category, - sample_flow.energy_carrier.code, - aggregated_profile - ) + # Create final aggregated flow with characteristics from first processed unit + # Extract from first unit (guaranteed to exist since we have units) + first_unit_sample = unit_operate_method(self.units[0], EnergyFlow( + energy_flow.input_category, + energy_flow.output_category, + energy_flow.energy_carrier.code, + pd.Series(1.0, index=[total_demand_profile.index[0]]) + ), *args) + + aggregated_flow = EnergyFlow( + first_unit_sample.input_category, + first_unit_sample.output_category, + first_unit_sample.energy_carrier.code, + aggregated_profile + )Note: A minimal sample (1.0 at first timestep) reduces overhead while still extracting characteristics.
446-463: Consider refactoring duplicated multi-unit initialization pattern.The multi-unit initialization logic (checking for multi-unit need, creating units, setting up aggregate) is duplicated across all component classes with identical structure:
needs_multi_unit, n_units, unit_capacity = Component._check_multi_unit_needed(...) if needs_multi_unit: self.n_units = n_units self.units = [] for _ in range(n_units): unit = ComponentClass(..., unit_capacity) self.units.append(unit) self._setup_multi_unit_aggregate(capacity) self.placement = self.units[0].placement returnConsider extracting this pattern into a base class helper method that accepts a factory function:
# In Component class def _create_multi_unit_if_needed(self, component_class, factory_fn, capacity): """ Check if multi-unit is needed and create units if so. :param component_class: The component class (e.g., AbsorptionChiller) :param factory_fn: Function that creates a single unit instance :param capacity: Total requested capacity :return: True if multi-unit was created, False if single unit should proceed """ needs_multi_unit, n_units, unit_capacity = Component._check_multi_unit_needed( component_class, self.code, capacity ) if needs_multi_unit: self.n_units = n_units self.units = [factory_fn(unit_capacity) for _ in range(n_units)] self._setup_multi_unit_aggregate(capacity) self.placement = self.units[0].placement return True return FalseThen each component class could use:
def __init__(self, ach_model_code, placement, capacity): if self._create_multi_unit_if_needed( AbsorptionChiller, lambda cap: AbsorptionChiller(ach_model_code, placement, cap), capacity ): return # Single unit initialization super().__init__(...)This would reduce duplication while maintaining type safety and readability.
Also applies to: 542-559, 632-649, 722-739, 812-829, 911-928, 1016-1033, 1110-1127, 1267-1284
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cea/optimization_new/component.py(22 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cea/optimization_new/component.py (1)
cea/optimization_new/containerclasses/energyFlow.py (9)
profile(91-92)profile(95-114)EnergyFlow(27-221)input_category(49-50)input_category(53-59)output_category(62-63)output_category(66-72)energy_carrier(75-76)energy_carrier(79-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests (windows-latest)
- GitHub Check: tests (macos-latest)
- GitHub Check: tests (ubuntu-latest)
🔇 Additional comments (5)
cea/optimization_new/component.py (5)
93-128: Verify that unit_capacity calculation handles database capacity range gaps correctly.The calculation
unit_capacity = capacity / n_units(line 120) divides capacity evenly across units, but the resulting unit_capacity might not fall within valid database capacity ranges. For example, if a model has capacity ranges [100-500W, 600-1000W] and you request 1100W total capacity, the calculation yields n_units=2 and unit_capacity=550W, which falls in the gap and will cause the recursive initialization to fail with a potentially misleading error message.Consider adding a check or adjusting the error message at lines 124-127 to clarify that multi-unit installation was attempted but failed because the calculated unit capacity doesn't match available capacity ranges in the database.
Additionally, verify that the database capacity ranges for all component models don't have gaps that would make multi-unit installations fail unexpectedly.
72-91: Document or complete the aggregate instance initialization.The
_setup_multi_unit_aggregatemethod copies core properties from the first unit but doesn't replicate subclass-specific operational attributes (e.g.,minimum_COP,aux_power_share,thermal_eff) because multi-unit initialization returns early (line 463, 559, 649, etc.) before these attributes are set.This design treats the aggregate as a routing shell that delegates operations to individual units. While this approach works for the
operate()methods that immediately cascade to units, it could causeAttributeErrorif external code attempts to access operational attributes on the aggregate instance.Consider either:
- Copying all subclass-specific attributes from the first unit in
_setup_multi_unit_aggregate, or- Adding documentation explaining that aggregate instances only support
operate()calls and should not be used to access operational attributes directly.
256-313: Consider adding capacity validation for multi-unit installations.The cascade operation implements a water-filling approach that gracefully handles demand up to total capacity by clipping each unit's allocation (line 279). However, unlike single-unit operations which explicitly validate that demand doesn't exceed capacity via
_check_operational_requirements()(line 246-248), multi-unit operations skip this check (e.g., line 494 returns immediately).If demand exceeds the total capacity of all units, the cascade will satisfy as much as possible and silently leave remaining demand unmet. While this might be intentional for resilience, it differs from the single-unit behavior which raises a
ValueError.Verify whether the graceful degradation behavior (satisfying partial demand without error) is intentional for multi-unit installations, or whether an explicit capacity check should be added before cascading, similar to single-unit validation.
381-408: LGTM: Cost calculation correctly handles both single and multi-unit installations.The multi-unit path (lines 390-394) aggregates pre-calculated costs from individual units, while the single-unit path (lines 396-408) computes costs from database parameters. The guard at line 398 prevents issues with
math.log(capacity_W)when capacity is zero or negative.
480-517: LGTM: Multi-unit operation routing is implemented consistently.All
operate()methods correctly check for multi-unit installations and route to the appropriate cascade method while preserving single-unit operation logic. The pattern of passing unbound methods to cascade operations enables proper delegation while maintaining each component's specific operation signatures.Also applies to: 574-609, 664-699, 754-789, 846-886, 946-981, 1048-1083, 1142-1185, 1297-1346
The purpose of this PR is to allow for multiple units of the same component class.
So far, component models would be entirely disregarded in supply system construction if no model were available that would offer a capacity large enough to cover all of the demand of a building or thermal network. This branch introduces a "multi-unit" attribute to components, implicitly allowing multiple smaller units of a specific component model to be installed to meet the building's or district's demand jointly.
This is particularly relevant for technologies that typically offer only smaller capacities. E.g., individual AC units (direct expansion units) installed in a multi-story residential building.
Summary by CodeRabbit