Anthropogenic heat feature #3977
Conversation
…-costs-from-optimisation-new
…-costs-from-optimisation-new
Added a check to warn and set base demand to zero for buildings connected to the network without a demand profile. This prevents errors when such buildings are present and informs the user to review the network configuration.
…aseline-costs-from-optimisation-new
|
Your changes to Please run: python -m cea.utilities.config_type_generatorAnd commit the updated |
WalkthroughAdds end-to-end baseline-costs and anthropogenic-heat analyses: new orchestration, supply‑component selection with multi‑level fallbacks, solar costing, heat‑rejection computations (standalone and networked), expanded config/schema/locator surfaces, visualization support, and tests; legacy system_costs marked deprecated. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Configuration
participant Locator as InputLocator
participant Main as baseline_costs_main
participant Supply as supply_costs
participant Solar as solar_costs
participant Format as format_output_simplified
participant FS as Files (CSV)
Config->>Main: run baseline_costs_main(config)
Main->>Locator: create InputLocator(config.scenario)
alt No network selected
Main->>Supply: calculate_all_buildings_as_standalone(locator, config)
Supply-->>FS: write baseline & detailed costs
else Network mode
Main->>Main: validate_network_results_exist(...) per network_type
Main->>Supply: calculate_all_buildings_as_standalone(locator, config)
loop per valid network type
Main->>Supply: calculate_costs_for_network_type(..., standalone_results)
Supply->>Supply: select components (assemblies or category fallbacks)
Supply-->>FS: write network-specific baseline & detailed costs
end
Main->>Supply: merge_network_type_costs(all_results)
end
Main->>Solar: calculate_building_solar_costs(config, locator, merged_results)
Solar-->>FS: append solar costs
Main->>Format: format_output_simplified(merged_results, locator)
Format-->>FS: final summary + detailed CSVs
sequenceDiagram
participant Config as Configuration
participant Locator as InputLocator
participant Heat as anthropogenic_heat_main
participant Supply as supply_costs
participant Network as SupplySystem
participant FS as Files (CSV)
Config->>Heat: run anthropogenic_heat_main(config)
Heat->>Locator: create InputLocator(config.scenario)
alt Standalone mode
Heat->>Supply: calculate_standalone_heat_rejection(locator, config, network_types)
Supply->>Network: create SupplySystem per building
Network-->>Heat: hourly heat_rejection per building
else Network mode
loop per network_type
Heat->>Heat: validate layout & get connected buildings
Heat->>Heat: get_user_component_selection(config, network_type)
Heat->>Supply: create_network_supply_system(locator, config, network_type, network_name, connected_buildings, standalone_results)
Supply->>Network: build/evaluate district SupplySystem
Network-->>Heat: plant & building heat_rejection contributions
end
end
Heat->>Heat: merge_heat_rejection_results(...)
Heat-->>FS: write heat_rejection_buildings.csv, heat_rejection_components.csv, per-building hourly CSVs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
cea/visualisation/b_data_processor.py (1)
192-347: Update docstring forgenerate_dataframe_for_plotlyto list all supported featuresThe docstring lists only
'demand', 'pv', 'pvt', 'sc'as validplot_cea_featurevalues, but the function now also supports'operational-emissions', 'lifecycle-emissions', and'heat-rejection'. Update the docstring to reflect all supported features.Additionally,
_calculate_plant_floor_arearedundantly imports pandas at line 205 even though it is already imported at the top of the file (line 7); remove the local import.All call sites of
generate_dataframe_for_plotlyandcalc_x_y_metricinplot_main.py,emission_timeline.py, andb_data_processor.pycorrectly pass thescenarioargument.cea/schemas.yml (1)
15782-15833: Duplicate schema definitions for get_new_optimization_optimal_network_layout_file with conflicting fieldsThe file defines this schema twice with different field names/cases ('building' vs 'Building', 'pipe_DN' vs 'tipe_DN'). This will confuse consumers and schema validators.
Please keep a single definition and reconcile field names. Suggested approach: retain the first (lines 15782-15833) and remove/merge the second, fixing 'tipe_DN' typo to 'pipe_DN'.
Also applies to: 16096-16151
cea/import_export/result_summary.py (1)
1381-1419:network_nameis undefined insideexec_aggregate_building(Ruff F821) and needs to be passed inWithin
exec_aggregate_building, the new heat_rejection handling branches call:heat_buildings_df = pd.read_csv(locator.get_heat_rejection_buildings(network_name=network_name))but
network_nameis not defined in this function’s scope, which is exactly what Ruff flags (F821). The only placenetwork_nameis available is inprocess_building_summary.You can fix this by threading
network_namethrough the function signature and call site. For example:-def exec_aggregate_building(locator, hour_start, hour_end, summary_folder, list_metrics, bool_use_acronym, list_list_useful_cea_results, list_buildings, list_appendix, list_selected_time_period, date_column='date', plot=False): +def exec_aggregate_building( + locator, + hour_start, + hour_end, + summary_folder, + list_metrics, + bool_use_acronym, + list_list_useful_cea_results, + list_buildings, + list_appendix, + list_selected_time_period, + date_column='date', + plot=False, + network_name='', +): @@ - if appendix == 'heat_rejection': - # Get entity names from heat_rejection_buildings.csv - heat_buildings_df = pd.read_csv(locator.get_heat_rejection_buildings(network_name=network_name)) + if appendix == 'heat_rejection': + # Get entity names from heat_rejection_buildings.csv + heat_buildings_df = pd.read_csv( + locator.get_heat_rejection_buildings(network_name=network_name) + ) @@ - if appendix == 'heat_rejection': - # Get entity names from heat_rejection_buildings.csv - heat_buildings_df = pd.read_csv(locator.get_heat_rejection_buildings(network_name=network_name)) + if appendix == 'heat_rejection': + # Get entity names from heat_rejection_buildings.csv + heat_buildings_df = pd.read_csv( + locator.get_heat_rejection_buildings(network_name=network_name) + ) @@ - if appendix == 'heat_rejection': - # Get entity names from heat_rejection_buildings.csv - heat_buildings_df = pd.read_csv(locator.get_heat_rejection_buildings(network_name=network_name)) + if appendix == 'heat_rejection': + # Get entity names from heat_rejection_buildings.csv + heat_buildings_df = pd.read_csv( + locator.get_heat_rejection_buildings(network_name=network_name) + )and in
process_building_summaryStep 8:- exec_aggregate_building(locator, hour_start, hour_end, summary_folder, list_metrics, bool_use_acronym, list_list_useful_cea_results, list_buildings, list_appendix, list_selected_time_period, plot=plot) + exec_aggregate_building( + locator, + hour_start, + hour_end, + summary_folder, + list_metrics, + bool_use_acronym, + list_list_useful_cea_results, + list_buildings, + list_appendix, + list_selected_time_period, + plot=plot, + network_name=network_name, + )This both fixes the lint error and ensures the heat-rejection building/entity mapping uses the same network scope as the rest of the summary.
Also applies to: 1453-1464
cea/inputlocator.py (1)
1053-1065: Thermal-network helper refactor: unnamed networks produce malformed filenamesThe
_get_thermal_network_results_file_pathhelper centralizes path logic, but has an edge case with unnamed networks:When
network_nameis empty orNone, the helper falls back toget_thermal_network_folder()but still interpolatesnetwork_nameinto the filename, producing malformed names likeDH__metadata_edges.csv(with double underscore) instead of a proper pattern.Call sites confirm this is a real issue:
cea/optimization/optimization_main.py:146explicitly callsget_thermal_network_edge_list_file(network_type, '')with an empty string in a validation checkcea/optimization/master/summarize_network.py:57callsget_thermal_network_edge_list_file(network_type)with a missingnetwork_nameargument entirely (runtime error)Either:
- Enforce non-empty
network_namefor all thermal network paths (simplest approach), or- Add an explicit branch in
_get_thermal_network_results_file_pathto handle unnamed networks with a different naming pattern
🟡 Minor comments (10)
cea/analysis/costs/supply_costs.py-636-636 (1)
636-636: Remove unused import to fix pipeline failure.The Ruff linter reports
F401: Unused import 'os'. This import is redundant asosis already imported at module level (line 9).def calculate_standalone_building_costs(locator, config, network_name): ... - import os - print(" Loading buildings and demands...")cea/analysis/heat/main.py-7-7 (1)
7-7: Remove unused import to fix pipeline failure.The
osimport is unused and causes the Ruff linter to fail (F401). This blocks CI.""" import os +# Remove the unused import above import cea.configApply this fix:
-import os import cea.configcea/analysis/costs/solar_costs.py-451-512 (1)
451-512: Potential KeyError if expected columns don't exist infinal_results.When updating existing rows (lines 481-491), the code assumes columns like
Capex_total_USD,TAC_USD, etc. exist. Iffinal_resultsis missing any of these columns, this will raise aKeyError.Consider adding a guard or ensuring upstream code always provides these columns:
# Ensure required columns exist before updating required_cols = ['Capex_total_USD', 'Capex_a_USD', 'Opex_fixed_a_USD', 'Opex_var_a_USD', 'Opex_a_USD', 'TAC_USD'] missing = [c for c in required_cols if c not in result.columns] if missing: raise ValueError(f"Missing required columns in final_results: {missing}")cea/tests/test_anthropogenic_heat.py-106-107 (1)
106-107: Trivial assertion:len(results) >= 0is always true.# Should have results (buildings may have cooling systems even with DH only) - assert len(results) >= 0, "Results should be present" + # DH-only may still produce results if buildings have standalone cooling + # This is a weak assertion - consider checking for specific expected behaviorcea/tests/test_baseline_costs.py-115-118 (1)
115-118: Manual test runner should also create the locator.If the function signature requires
locator, the manual test runner needs updating too:if __name__ == '__main__': # For manual testing config = cea.config.Configuration() - test_baseline_costs_reference_case(config) + # Note: test_baseline_costs_reference_case expects a config fixture + # For manual testing, you may need to run via pytestCommittable suggestion skipped: line range outside the PR's diff.
cea/tests/test_anthropogenic_heat.py-73-74 (1)
73-74: Trivial assertion:len(detailed) >= 0is always true.This assertion provides no value as a list length can never be negative. Consider asserting a minimum expected count or removing the assertion.
# Check detailed output detailed = pd.read_csv(locator.get_heat_rejection_components()) - assert len(detailed) >= 0, "Components output missing" + # Components output can be empty if no heat-rejection components exist + assert 'name' in detailed.columns, "Missing 'name' column in components output"cea/config.py-1838-1941 (1)
1838-1941: SolarPanelChoiceParameter: docstring vs behaviour and decode I/OThe
SolarPanelChoiceParameterimplementation is generally consistent with the config guidelines (lenientdecode, strictencode, dynamic_choicesvia the potentials/solar folder), but there are two nits:
- The
decodedocstring says “Returns empty string if value is None or empty”, while the implementation returns'No solar technology installed'in that case. The docstring should be updated to match the actual behaviour.decodecalls_get_available_solar_technologies(), which touches the filesystem viaInputLocatorandglob. That is fine in practice, but if these parameters are accessed frequently it may be worth keeping decode as cheap as possible and relying on encode for stricter validation, in line with the config guidelines.Suggested docstring tweak:
- """ - Decode value - lenient parsing for config loading. - Returns empty string if value is None or empty. - """ + """ + Decode value – lenient parsing for config loading. + Returns 'No solar technology installed' if value is None, empty, + or not present in the available solar technologies. + """cea/config.py-1025-1091 (1)
1025-1091: DC/DH network layout subclasses do not actually filter by network type and unnecessarily mutate config stateThe
DCNetworkLayoutChoiceParameterandDHNetworkLayoutChoiceParameterimplementations temporarily overrideconfig.thermal_network.network_typeandconfig.optimization_new.network_type, but this has no effect. The parentNetworkLayoutChoiceParameter._get_available_networks()lists folders in the thermal-network directory that are not named'DH'or'DC'(see line 908:if os.path.isdir(...) and name not in self._network_types). It does not inspect the config parameters being overridden.Consequently:
- The docstrings claiming "Hardcoded to filter for DC/DH networks" are misleading; no actual filtering occurs.
- Mutating config state in a parameter helper is surprising and violates the principle that config helpers should not have side effects.
Remove the temporary config overrides and either delegate directly to the parent implementation or implement true filtering via
InputLocatorif DC/DH-specific results need to be distinguished (e.g. checking for folders at{network_name}/DC/or{network_name}/DH/).cea/visualisation/special/cost_breakdown.py-336-440 (1)
336-440: Refine y-axis range handling clarity and plumb network_name through baseline costs loadingThe y-axis range code at lines ~425–426 is functionally correct—Plotly supports
Nonein range parameters, treating it as autorange for that bound. However, the current logic is redundant: checking(y_min is not None or y_max is not None)then unconditionally passing both values will silently use autorange for whichever isNone. For clarity, simplify to:y_min = plot_config_general.y_min y_max = plot_config_general.y_max if y_min is not None or y_max is not None: fig.update_xaxes(range=[y_min, y_max])More importantly,
load_baseline_costs_data()(line 76) andmain()(line 462) callget_baseline_costs_detailed()without passingnetwork_name, always loading from theno-networkfolder. Since baseline costs are written per-network (as shown incea/analysis/costs/main.py), plumb the network name through the config or via a dedicated parameter—similar to howget_network_buildings_and_area()already handles it in this file—so costs can be loaded for the specified network context.Finally, the module docstring uses American spelling ("visualization"); align it to British spelling ("visualisation") for consistency with project guidelines and the filename itself.
cea/analysis/heat/heat_rejection.py-356-452 (1)
356-452: Network plant splitting vs. component breakdown may double-count per-plant components
calculate_network_heat_rejectioncorrectly:
- Builds a single network-scale
SupplySystem(plant_supply_system) for the DH/DC network.- Computes total annual and peak heat rejection from
plant_supply_system.heat_rejection.- Splits the summary metrics (annual, hourly profile, peak) equally among all plant nodes when there are multiple plants.
However, in
save_heat_rejection_outputs:
- Each plant entry carries the same
plant_supply_systemobject via thesupply_systemfield.extract_component_heat_rejectionis called for each plant, and always usessupply_system.heat_rejectionandinstalled_componentsas-is.This means:
- The
heat_rejection_buildings.csvsummary correctly reports per-plant values (split bynum_plants), butheat_rejection_components.csvwill contain duplicate component rows for each plant, each representing the full network’s components and heat rejection. Aggregating that file naively will over-count district-scale heat rejection by a factor ofnum_plants.If the intent is for the components file to be per-network (not per-plant), you could:
- Only emit component rows once per network (e.g. for the first plant) or
- Add a field like
network_idand document that the rows are network-level, not per-plant.If, instead, you want true per-plant component accounting, you’ll need to either:
- Divide
heat_seriesand any relevant capacity indicators bynum_plantsbefore passing them intoextract_component_heat_rejection, or- Construct one
SupplySystemper plant with appropriately scaled demand.Right now the split logic only applies to summaries, not to component breakdowns.
Also applies to: 820-922
🧹 Nitpick comments (34)
cea/optimization_new/domain.py (1)
120-128: Consider logging the skipped building for transparency.The defensive error handling is appropriate and prevents crashes when loading buildings with incompatible network types. However, silently skipping buildings may confuse users about why certain buildings aren't included in the optimization.
Consider adding a log message:
if has_demand and not is_district: try: building.check_demand_energy_carrier() except ValueError as e: # Skip buildings with incompatible network types (e.g., DH network with cooling-only building) # This can happen when loading all buildings with a specific network_type filter if "Network type mismatch" in str(e): + print(f" Skipping {building_code}: Incompatible with {network_type} network (network type mismatch)") continue else: raiseThis would help users understand why the building count might differ from expectations.
cea/analysis/heat/AGENTS.md (1)
11-11: Consider adding language specifiers to fenced code blocks.For improved syntax highlighting and markdown rendering, consider specifying languages for the fenced code blocks:
-``` +```text ✅ **DO**: Treat plants as "buildings" with serviced GFA for normalisation ❌ **DON'T**: Use plant footprint area - meaningless for district systemsThis is a minor formatting improvement flagged by markdownlint. Also applies to: 243-243 </blockquote></details> <details> <summary>cea/interfaces/dashboard/map_layers/life_cycle_analysis/layers.py (3)</summary><blockquote> `26-27`: **Remove redundant imports inside the function.** `geopandas` and `CRS` are already imported at module level (lines 5-6). These local imports are unnecessary. ```diff def safe_filter_buildings_with_geometry(locator, buildings: list) -> tuple: """ Filter buildings to only include those that exist in zone geometry. Returns tuple of (filtered_buildings, geometry_df, centroids). Gracefully handles missing buildings by excluding them. """ - import geopandas as gpd - from pyproj import CRS - if not buildings: return [], None, []
568-592: Move the empty entity check earlier to avoid unnecessary processing.The empty check at line 586 happens after creating the GeoDataFrame. Move it earlier to avoid unnecessary work when there are no entities.
# Set buildings file as index buildings_df = buildings_df.set_index('name') + # Handle case where there are no entities + if not entity_names: + output['data'] = [] + output['properties']['range'] = { + 'total': {'label': 'Total Range', 'min': 0.0, 'max': 0.0}, + 'period': {'label': 'Period Range', 'min': 0.0, 'max': 0.0} + } + return output + # Get zone geometry for CRS zone_gdf = gpd.read_file(self.locator.get_zone_geometry()) # Create GeoDataFrame with entity coordinates entity_gdf = gpd.GeoDataFrame( buildings_df.loc[entity_names], geometry=gpd.points_from_xy( buildings_df.loc[entity_names]['x_coord'], buildings_df.loc[entity_names]['y_coord'] ), crs=zone_gdf.crs ) entity_centroids = entity_gdf.geometry.to_crs(CRS.from_epsg(4326)) - # Handle case where there are no entities or all files are missing - if not entity_names: - output['data'] = [] - output['properties']['range'] = { - 'total': {'label': 'Total Range', 'min': 0.0, 'max': 0.0}, - 'period': {'label': 'Period Range', 'min': 0.0, 'max': 0.0} - } - return output - values = (get_data(entity_name, centroid)
531-533: Add error handling for reading the buildings file.Other layers in this file wrap CSV reading in try/except blocks (e.g., lines 176-198). Consider adding similar protection here to handle missing or malformed files gracefully.
+ try: # Read buildings file to get entity list and coordinates buildings_df = pd.read_csv(self.locator.get_heat_rejection_buildings()) entity_names = buildings_df['name'].tolist() + except (FileNotFoundError, pd.errors.EmptyDataError, KeyError) as e: + print(f"Warning: Error reading heat rejection buildings: {e}") + output['properties']['range'] = { + 'total': {'label': 'Total Range', 'min': 0.0, 'max': 0.0}, + 'period': {'label': 'Period Range', 'min': 0.0, 'max': 0.0} + } + return outputcea/interfaces/dashboard/map_layers/demand/layers.py (1)
12-36: Consider extracting shared utility function to reduce duplication.This
safe_filter_buildings_with_geometryfunction is duplicated across three files:
cea/interfaces/dashboard/map_layers/demand/layers.pycea/interfaces/dashboard/map_layers/life_cycle_analysis/layers.pycea/interfaces/dashboard/map_layers/renewable_energy_potentials/layers.pyConsider extracting it to a shared module (e.g.,
cea/interfaces/dashboard/map_layers/utils.py) to maintain a single source of truth.cea/analysis/costs/supply_costs.py (3)
152-152: Remove redundant local imports.Several functions have local imports of modules already imported at module level (line 9-13). Consider removing these redundant imports:
- Line 152:
import pandas as pd- Line 195:
import pandas as pd- Lines 256-262: Multiple imports already at module level
- Line 457:
import pandas as pd- Lines 738-739:
import pandas as pdandimport osAlso applies to: 195-195, 256-262, 457-457, 738-739
816-824: Consider using logging levels instead of stdout suppression.The pattern of redirecting stdout to suppress messages is fragile. Consider using Python's logging module with appropriate log levels, allowing callers to control verbosity via configuration.
This is a minor improvement suggestion for future refactoring.
546-547: Avoid silently swallowing exceptions.The bare
except Exception: passhides all errors, making debugging difficult. At minimum, log the exception.except Exception: - pass + pass # Network layout file may not exist yet - this is expectedOr better:
- except Exception: - pass + except Exception as e: + # Network layout may not exist - return empty set + print(f"Warning: Could not read network layout: {e}")cea/analysis/costs/system_costs.py (1)
246-246: Consider terminal compatibility for emoji.The emoji
⚠️may not render correctly in all terminal environments (e.g., Windows cmd.exe, some CI logs). Consider using a text-based alternative.- print('\n⚠️ WARNING: This script is deprecated. Please use "cea baseline-costs" instead.\n') + print('\nWARNING: This script is deprecated. Please use "cea baseline-costs" instead.\n')cea/analysis/costs/format_simplified.py (1)
100-109: Consider defensive access for service_costs keys.The code directly accesses keys like
service_costs['scale'],service_costs['capex_total_USD'], etc. without checking if they exist. If the upstream data structure is malformed, this will raiseKeyError.Consider using
.get()with defaults for non-critical fields, or add validation at the function entry to ensure the expected structure:# Example defensive access scale = service_costs.get('scale', 'BUILDING') capex_total = service_costs.get('capex_total_USD', 0.0)cea/optimization_new/building.py (1)
257-268: The self-assignment on line 268 is a no-op.Line 268 assigns
self.demand_flow.energy_carrierto itself, which has no effect. If the intent is to preserve the existing carrier, simply remove this line or add a clarifying comment.# Otherwise: has demand but no space heating components -> DHW-only building # Set demand flow energy carrier to match what was loaded (T30W for DH, T10W for DC) # DHW fallback logic in supply_costs.py will handle creating the actual DHW system - self.demand_flow.energy_carrier = self.demand_flow.energy_carrier # Keep existing carrier from load_demand_profile + # Keep existing energy carrier from load_demand_profile (T30W for DH, T10W for DC) + passcea/analysis/costs/solar_costs.py (1)
416-419: Semantic mismatch: storing area incapacity_kWcolumn.The column name
capacity_kWimplies power capacity, but solar panel costs are area-based (m²). This could confuse downstream consumers. Consider renaming tocapacity_m2or adding a separate column.detail_rows.append({ 'name': building_name, 'network_type': '', 'service': 'SOLAR', 'code': tech_code, - 'capacity_kW': costs['area_m2'], # Store area in the capacity column + 'capacity_m2': costs['area_m2'], # Solar uses area, not power capacity 'placement': facade_suffix.replace('_m2', ''), # e.g., 'roofs_top'If schema constraints require
capacity_kW, add a comment explaining this is actually area for solar systems.cea/analysis/costs/main.py (5)
31-31: Importosinside function.Consider moving to module-level imports for consistency, unless lazy loading is intentional.
import pandas as pd from cea.inputlocator import InputLocator import cea.config +import osThen remove the
import osfrom insidevalidate_network_results_exist.
121-149: Significant code duplication between standalone and network modes.The solar cost calculation and result merging logic (lines 121-149 for standalone, lines 263-297 for network mode) is nearly identical. Extract to a helper function.
def _calculate_and_merge_solar_costs(config, locator, final_results, detailed_results): """Calculate solar costs and merge into results.""" from cea.analysis.costs.solar_costs import calculate_building_solar_costs, merge_solar_costs_to_buildings import geopandas as gpd zone_gdf = gpd.read_file(locator.get_zone_geometry()) all_building_names = zone_gdf['name'].tolist() if config.system_costs.buildings: all_building_names = [b for b in all_building_names if b in config.system_costs.buildings] solar_details, solar_summary = calculate_building_solar_costs(config, locator, all_building_names) if not solar_details.empty: detailed_results = pd.concat([detailed_results, solar_details], ignore_index=True) print(f" Added {len(solar_details)} solar panel component rows") if not solar_summary.empty: final_results = merge_solar_costs_to_buildings(final_results, solar_summary) print(f" Updated costs for {len(solar_summary)} buildings with solar panels") return final_results, detailed_resultsAlso applies to: 263-297
325-327: Fragile check: assumes network names start with 'N'.
name.startswith('N')to identify networks could match building names starting with 'N'. Consider a more robust check.# Check if any networks were found - has_networks = any(name.startswith('N') for name in final_results['name']) + # Networks have 'type' == 'network' or 'plant' in the results + has_networks = 'type' in final_results.columns and final_results['type'].eq('plant').any()Or use a dedicated column/flag to identify network entries.
227-243: Error handling catches ValueError but may hide unexpected issues.The check for specific error messages (lines 230-239) is fragile. If error messages change, this logic breaks silently.
Consider using custom exception types instead of string matching:
class NoValidSupplySystemError(ValueError): """Raised when no valid supply systems are found.""" passThen catch this specific exception type.
82-89: Docstring return type mismatch.The docstring says
:return: DataFrame with cost resultsbut the function returnsNonein standalone mode (line 164) andfinal_resultsin network mode. Update for accuracy.""" Calculate baseline costs for heating and/or cooling systems. :param locator: InputLocator instance :param config: Configuration instance - :return: DataFrame with cost results + :return: DataFrame with cost results (network mode) or None (standalone mode) """cea/config.py (1)
1269-1519: DistrictSupplyTypeParameter logic looks solid; watch decode/choices I/O costThe
DistrictSupplyTypeParameterimplementation correctly:
- Builds dynamic choices from ASSEMBLIES/SUPPLY CSVs with explicit BUILDING vs DISTRICT scale.
- Prioritises options currently in use in
supply.csv.- Enforces at most one building‑scale and one district‑scale selection in
encode, with clear error messages.- Returns labelled values from
get()while storing unlabelled codes in the config, and auto‑defaults fromsupply.csvwhen the config is empty.Two minor points to consider:
_get_all_supply_options,_get_supply_options_in_use, and_get_auto_defaultall hit the filesystem / pandas, and are invoked from_choices,decode, andget(). Depending on how often these parameters are accessed, this could become noticeable. If it does, you could cache results per‑scenario or per‑parameter instance with simple invalidation (e.g. onset).- The string
"Custom (use component settings below)"appears in several places; extracting it into a small constant on the class would reduce duplication and avoid future typos.Functionally this looks correct and matches the documented 1‑building + 1‑district selection rule.
cea/analysis/costs/AGENTS.md (1)
7-8: Keep API signature and examples forget_component_codes_from_categoriesin sync; tighten wordingIn this AGENTS doc, the top‑level API section documents:
get_component_codes_from_categories(locator, component_categories, network_type) → list[str]but the DC/DH examples later call:
cooling_codes = get_component_codes_from_categories(locator, cooling_cats, network_type, peak_demand_kw) heat_rejection_codes = get_component_codes_from_categories(locator, heat_rejection_cats, network_type, peak_demand_kw)If the implementation now accepts
peak_demand_kw(as implied by the example and “Filter by peak demand” principle), the main signature line should be updated to include that parameter and its meaning; otherwise, the example should be adjusted to match the actual function. Keeping the doc consistent makes it much easier to use from other modules.Also, in the “Key behaviour” / fallback sections, there is a repeated adverb (“only ... only applies”) that could be simplified for clarity (e.g. “Checks services the building provides standalone – applies only if Building Properties/Supply has a DISTRICT‑scale code”).
Also applies to: 82-117, 260-305
cea/visualisation/special/cost_breakdown.py (1)
29-67: Guard against zero total area during normalisation and avoid per-row recomputation
process_data_by_groupingnormalises costs per area wheny_normalised_by != 'no_normalisation'by computing:if y_normalised_by == 'gross_floor_area': total_area = architecture_df['GFA_m2'].sum() else: total_area = architecture_df['Af_m2'].sum() ... for col in selected_cost_cols: df_agg[col] = df_agg[col] / total_areaand similarly uses per‑group
normaliservalues for theby_building_and_networkcase, with a few fallbacks to1to avoid divide‑by‑zero.Two improvements would make this more robust:
- If
total_areais 0 (e.g. malformedTotal_demand.csvor all‑zero areas), the aggregated views will divide by zero and produce infinities. Consider explicitly checking fortotal_area <= 0and either:
- Skipping normalisation (and perhaps emitting a warning), or
- Returning an empty frame or raising a clear error, rather than silently producing invalid numbers.
- Inside the
by_building_and_networkbranch,get_network_buildings_and_areais called once per network row; that’s fine for small networks but scales linearly. If this becomes hot, you could memoise results per(network_name, network_type, area_col)to avoid re-reading the same shapefile repeatedly.The high‑level logic looks correct; this is mainly about defensive handling of degenerate area data and minor performance tuning.
Also applies to: 140-267
cea/schemas.yml (4)
1859-1935: Unify currency units and mark primary key; fill used_by for traceability
- Currency unit uses '[$USD(2015)]' here but most tables use '[USD 2015]'. Pick one and apply consistently to avoid downstream unit parsing issues.
- Consider marking name as primary for easier validation and joins.
- used_by is empty; add downstream consumers (e.g., multi_criteria_analysis) for pipeline traceability.
Apply minimally:
@@ - Capex_total_USD: - description: Total capital expenditure for all supply systems and components - type: float - unit: '[$USD(2015)]' + Capex_total_USD: + description: Total capital expenditure for all supply systems and components + type: float + unit: '[USD 2015]' @@ - Capex_total_building_scale_USD: + Capex_total_building_scale_USD: description: Total capital expenditure for building-scale systems type: float - unit: '[$USD(2015)]' + unit: '[USD 2015]' @@ - Capex_total_district_scale_USD: + Capex_total_district_scale_USD: description: Total capital expenditure for district-scale systems (networks + piping) type: float - unit: '[$USD(2015)]' + unit: '[USD 2015]' @@ - name: - description: Building or network name - type: string - unit: NA + name: + description: Building or network name + type: string + primary: true + unit: NA @@ - used_by: [] + used_by: + - multi_criteria_analysisBased on learnings, Opex_var_a_USD should reflect FEEDSTOCKS costs joined to assemblies; this schema matches that split.
1937-1994: Constrain enums and add lookups for stronger validation
- Add choices for network_type and scale; constrain placement values to the documented set; make service values explicit to avoid drift.
- Consider lookups for component codes and energy carriers to existing databases.
@@ - network_type: - description: Network type (DC for district cooling, DH for district heating) or empty for standalone buildings - type: string - unit: NA + network_type: + description: Network type (empty for standalone buildings) + type: string + unit: NA + choice: + values: ["", "DC", "DH"] @@ - service: - description: Energy service (e.g., GRID_cs for grid cooling, DC_network for district cooling network, NONE for buildings with no systems) - type: string - unit: NA + service: + description: Energy service label + type: string + unit: NA + choice: + values: ["GRID_cs","GRID_hs","GRID_ww","DC_network","DC_plant","DH_network","DH_plant","NONE"] @@ - code: - description: Component code (e.g., CH1 for chiller type 1, CT1 for cooling tower type 1, PIPES for distribution piping) or energy carrier code (e.g., E230AC for electricity, NATURALGAS for natural gas) - type: string - unit: NA + code: + description: Component code (CONVERSION) or energy carrier code (ENERGY_CARRIERS) + type: string + unit: NA + choice: + lookup: + path: get_database_components_feedstocks_energy_carriers + column: code @@ - placement: + placement: description: Component placement in system (primary for heat/cooling generation, secondary for intermediate conversion, tertiary for heat rejection, distribution for piping, energy_carrier for fuel/electricity costs) - type: string - unit: NA + type: string + unit: NA + choice: + values: ["primary","secondary","tertiary","distribution","energy_carrier"] @@ - scale: - description: System scale (BUILDING for individual building systems, DISTRICT for district network systems) - type: string - unit: NA + scale: + description: System scale + type: string + unit: NA + choice: + values: ["BUILDING","DISTRICT"]Also align currency units to '[USD 2015]' like in the previous comment.
2043-2085: Add enums/lookups; align units and scale
- Constrain network_type/scale, and add lookups for energy_carrier (ENERGY_CARRIERS) and component_code (CONVERSION tables).
- Align units for currency if present (future-proofing).
@@ - network_type: + network_type: description: Network type (DC for district cooling, DH for district heating) or empty for standalone buildings - type: string - unit: NA + type: string + unit: NA + choice: + values: ["", "DC", "DH"] @@ - component_code: - description: Component code (e.g., CH1 for chiller type 1, CT1 for cooling tower type 1) - type: string - unit: NA + component_code: + description: Component code (see CONVERSION databases) + type: string + unit: NA + choice: + lookup: + path: get_database_components_conversion_vapor_compression_chillers + column: code @@ - energy_carrier: - description: Energy carrier code for heat rejection medium (e.g., T_AIR_20C for air at 20C, T_AIR_ODB for outdoor air) - type: string - unit: NA + energy_carrier: + description: Energy carrier code for heat rejection medium + type: string + unit: NA + choice: + lookup: + path: get_database_components_feedstocks_energy_carriers + column: code @@ - scale: + scale: description: System scale (BUILDING for individual building systems, DISTRICT for district network plants) - type: string - unit: NA + type: string + unit: NA + choice: + values: ["BUILDING","DISTRICT"]
2086-2103: Use date type for timestamp; align column naming with repo convention
- DATE is typed as string here but other time-series use type: date with '[datetime]'.
- Consider 'Date' (capitalization used widely) for consistency.
@@ - DATE: - description: Timestamp for this hour (8760 hours total, no leap day) - type: string - unit: NA + Date: + description: Timestamp for this hour (8760 hours total, no leap day) + type: date + unit: '[datetime]' + values: '{yyyy-mm-dd hh:mm:ss-Z}'cea/inputlocator.py (1)
1217-1220: Solar potentials folder consolidation is good; docstrings slightly staleSwitching PV/SC/PVT totals to use
get_potentials_solar_folder()standardises all solar potentials underscenario/outputs/data/potentials/solar, which matches the new API surface and simplifies downstream consumers. Implementation is straightforward and correct.Minor nit: several existing docstrings around
PV_totals,SC_totals, etc. still refer to older, more specific filenames (and even contain typos like a duplicated.csv). Consider updating those docstrings in a follow-up to avoid confusion, but no functional issue here.Also applies to: 1243-1248, 1260-1265, 1274-1278
cea/import_export/result_summary.py (4)
266-275: Heat rejection results path: be explicit about error handling for missing inputsThe new
cea_feature == 'heat_rejection'branch readsheat_rejection_buildings.csvto discover entities and then builds per-entity hourly paths viaget_heat_rejection_hourly_building.This is structurally correct and matches the new heat-rejection output layout. However, unlike most other features, any missing or misnamed
heat_rejection_buildings.csvwill raise immediately viapd.read_csv, instead of being handled more gracefully likeload_cea_results_from_csv_filesdoes for missing files.Consider wrapping the read in an existence check / try–except to emit a clear, user-facing message such as “heat rejection results not found – run
anthropogenic-heatfirst”, rather than a bareFileNotFoundError.
681-683: Heat rejection column mapping vs. writer: ensure units and naming are consistentYou’ve introduced:
- A metrics→column mapping:
"heat_rejection[kWh]" -> ["heat_rejection_kWh"]- A loader-side rename:
heat_rejection_kW -> heat_rejection_kWhfor CSVs that currently storeheat_rejection_kW.This keeps the internal mapping coherent and allows
map_metrics_and_cea_columnsto find the heat-rejection series, but there are two points to double‑check:
save_heat_rejection_outputsstill writes the hourly files with aheat_rejection_kWcolumn name. After the rename, downstream users may see the same series under two different unit labels (kWin the raw file,kWhin summaries).- If the underlying data truly represents power (kW) rather than hourly energy (kWh), treating it as energy in aggregations (sum over time) is conceptually inconsistent.
I’d recommend either:
- Switching the writer to emit
heat_rejection_kWh(and ensuring the series is hourly energy, not instantaneous power), or- Keeping it as
heat_rejection_kWand mapping a dedicated “power” metric (e.g.heat_rejection[kW]) rather than renaming it to_kWh.Please verify the intended physical quantity of
SupplySystem.heat_rejectionand align names/metrics accordingly.Also applies to: 862-865
1405-1419: Re-readingheat_rejection_buildings.csvthree times is redundantIn the annually, monthly, and seasonally branches you re-read
heat_rejection_buildings.csveach time to deriveentity_names. This is correct but a bit wasteful.Once
network_nameis passed intoexec_aggregate_building, consider loading theheat_rejection_buildingstable once perappendix == 'heat_rejection'and reusingentity_namesacross the three blocks, e.g. by:
- Computing
entity_namesandnum_entities_per_periodonce before theif annually_rows/if monthly_rows/if seasonally_rowsblocks, and- Sharing the computed list instead of re-reading the CSV.
This keeps behaviour identical but avoids unnecessary disk I/O in large scenarios.
Also applies to: 1453-1464
2740-2793:copy_costs_to_summarylogic is sound; minor cleanups possibleThe new helper does the right things:
- Resolves source cost files via
get_baseline_costs/get_baseline_costs_detailed, respectingnetwork_nameand the"no-network"fallback.- Optionally filters both buildings and components by
name ∈ list_buildings.- Writes filtered copies into
scenario/export/results/{folder_name}/costs/…via the new InputLocator helpers, creating the subfolder as needed.- Returns
(success, error_message)for structured error handling in Step 8.5.Two small polish points:
- The module already imports
pandas as pdat the top; you can drop the innerimport pandas as pdhere.- The user-facing error messages (“Please run 'system-costs' first.”) are clear and in British English, aligned with the guidelines.
Overall this is a good encapsulation of the cost-export step.
cea/analysis/heat/heat_rejection.py (3)
229-354: DHW heat-rejection system creation is powerful but quite complex; consider guarding against subtle failures
create_dhw_heat_rejection_systemreimplements a good chunk of the supply-system construction pipeline to build a DHW-onlySupplySystemfor heat-rejection extraction. It:
- Looks up the building’s DHW assembly code from
supply.csv.- Resolves the assembly to a feedstock in
SUPPLY_HOTWATER.csv.- Uses
get_dhw_component_fallbackto pick an appropriate component code.- Builds an hourly DHW demand series from
Qww_sys_kWhin the building’s demand file.- Constructs
EnergyFlow,SupplySystemStructure, andSupplySystemobjects, initialisingComponentglobals via a temporaryDomain.This is a reasonable approach, but several failure paths already return
None(missing supply entry, missing assemblies file,feedstock == 'NONE', zero demand, etc.). Combined with the broadexcept Exceptionat the end, it can be hard to diagnose why DHW heat rejection is missing for a given building.Two suggestions to make this easier to maintain and debug:
- Narrow the
try/exceptscope to just theSupplySystemStructure/SupplySystembuild/evaluate block, and let earlier validation failures log a short, explicit reason (e.g. “no DHW assembly for building X”).- Log which of the early return conditions was hit (e.g. feedstock is
NONE, missingQww_sys_kWh, zero DHW demand) so users can tell those apart from genuine internal errors.The existing functional behaviour is fine; this is about observability and future debugging.
748-771: Coordinate extraction should handle bothNameandnamecolumns in zone geometry
get_building_coordinates_from_geometrycurrently does:zone_geom = gpd.read_file(zone_geom_path).set_index('name')and falls back to
(0.0, 0.0)if anything goes wrong. In other parts of the codebase (e.g.exec_aggregate_buildingfor architecture), you already handle theNamevsnamenaming inconsistency explicitly.For robustness across older scenarios and reference cases, consider mirroring that logic here:
- zone_geom = gpd.read_file(zone_geom_path).set_index('name') - - if building_name in zone_geom.index: - centroid = zone_geom.loc[building_name].geometry.centroid + zone_geom_raw = gpd.read_file(zone_geom_path) + if 'name' in zone_geom_raw.columns: + zone_geom = zone_geom_raw.set_index('name') + elif 'Name' in zone_geom_raw.columns: + zone_geom = zone_geom_raw.set_index('Name') + else: + print(f" Warning: Zone geometry missing 'name'/'Name' column, using (0.0, 0.0)") + return 0.0, 0.0 + + if building_name in zone_geom.index: + centroid = zone_geom.loc[building_name].geometry.centroid return centroid.x, centroid.yThis avoids spurious
(0.0, 0.0)coordinates when only the capitalisedNamecolumn is present.
820-877: Hourly writer assumes 8760+ values; consider guarding against short series
save_heat_rejection_outputsassumes eachhourly_profilehas at least 8760 points, with an optional leap-day removal whenlen(hourly_profile) > 8760. If, for any reason, a shorter series is passed (e.g. partial-year simulations or trimming upstream),pd.DataFrame({'date': timestamps, 'heat_rejection_kW': hourly_profile[:8760]})will raise due to mismatched lengths.It may be worth asserting the expected length before writing, for clearer failures:
- # Ensure exactly 8760 hours (remove leap day if present) - if len(hourly_profile) > 8760: + # Ensure exactly 8760 hours (remove leap day if present) + if len(hourly_profile) > 8760: # Remove Feb 29 (day 60, hours 1416-1439 in leap year) hourly_profile = remove_leap_day(hourly_profile) + if len(hourly_profile) < 8760: + raise ValueError( + f"Expected at least 8760 hourly values for '{building_name}', " + f"got {len(hourly_profile)}." + )or, if partial years are expected, explicitly documenting and handling that case (e.g. by reindexing with NaNs) instead of relying on implicit errors.
cea/default.config (1)
529-575: Semantic duplication between[system-costs]and[anthropogenic-heat]sections.Both sections redundantly declare identical parameter structures (network-name, network-type, supply-type-cs/hs/dhw, cooling/heating/heat-rejection-components). While functionally acceptable if they serve distinct workflows, consider whether a shared base configuration or DRY principle could eliminate this duplication.
| buildings_in_network = network_data.get('DH', {}).get('buildings') or \ | ||
| network_data.get('DC', {}).get('buildings', []) | ||
| total_gfa = 0.0 | ||
| for building in buildings_in_network: | ||
| building_demand = demand[demand['name'] == building.identifier] | ||
| if not building_demand.empty: | ||
| total_gfa += building_demand.iloc[0]['GFA_m2'] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find how buildings are stored in network data
rg -n "buildings.*=" cea/analysis/costs/format_simplified.py -A 3 -B 3
# Search for building class definitions and identifier attribute
rg -n "class.*[Bb]uilding" cea/ --type py | head -20
# Look for how identifier is used in building contexts
rg -n "\.identifier" cea/analysis/costs/ --type py | head -20
# Search for network data structure definition
rg -n "network_data.*get.*buildings" cea/ --type py | head -20Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 4918
🏁 Script executed:
# Check the demand DataFrame and how it uses 'name'
rg -n "demand\['name'\]" cea/ --type py | head -20
# Look for building object definitions in the codebase
fd "building" cea/ --type py | xargs grep -l "identifier" | head -10
# Search for how network.buildings is populated
rg -n "\.buildings\s*=" cea/ --type py | head -20Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1605
🏁 Script executed:
# Check if buildings_in_network is ever populated with strings vs objects
rg -n "buildings_in_network" cea/ --type py -B 5 -A 5
# Look for building data structures in network-related code
rg -n "class.*Network" cea/ --type py | head -10Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 6494
🏁 Script executed:
# Find the full context of format_simplified.py to see where network_data comes from
cat -n cea/analysis/costs/format_simplified.py | head -100
# Search for where network_data is passed to this function
rg -n "format_simplified" cea/ --type py -B 5 -A 5Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 7496
🏁 Script executed:
# Look for Network class definition to see if it has a buildings attribute
cat -n cea/optimization_new/network.py | head -150Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 8320
🏁 Script executed:
# Search for where network_data dict is constructed with 'buildings' key
rg -n "network_data\[.*\]\[.*buildings" cea/ --type py -B 3 -A 3
rg -n "'buildings':" cea/ --type py -B 5 -A 5Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 36731
🏁 Script executed:
# Find where network_buildings is created in supply_costs.py
rg -n "network_buildings\s*=" cea/analysis/costs/supply_costs.py -B 5 -A 2 | head -50Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 986
🏁 Script executed:
# Find how network_buildings is stored in results dict for networks
rg -n "results\[network_id\]\s*=" cea/analysis/costs/supply_costs.py -A 10 | grep -A 10 "buildings"Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 899
🏁 Script executed:
# Also check the standalone building case to see what 'buildings' structure should be
rg -n "results\[building_id\]\s*=" cea/analysis/costs/supply_costs.py -B 2 -A 15Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1145
🏁 Script executed:
# Find the context around line 1422 where buildings is defined
sed -n '1350,1430p' cea/analysis/costs/supply_costs.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 4334
🏁 Script executed:
# Find where merge_network_type_costs is called and trace the results
rg -n "merge_network_type_costs" cea/analysis/costs/ -A 5 -B 5Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2488
🏁 Script executed:
# Find the merge_network_type_costs function definition
rg -n "def merge_network_type_costs" cea/analysis/costs/ -A 30Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1988
🏁 Script executed:
# Get full context around line 1315 and 1422 to understand the code flow
sed -n '1280,1330p' cea/analysis/costs/supply_costs.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2312
🏁 Script executed:
# Get context around line 1422
sed -n '1380,1430p' cea/analysis/costs/supply_costs.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2384
🏁 Script executed:
# Find which function is actually called - trace from baseline_costs_main
sed -n '82,300p' cea/analysis/costs/main.py | cat -n | grep -A 50 "baseline_costs_main"Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2724
🏁 Script executed:
# Search for calls to calculate_network_costs or the function with line 1315
rg -n "def calculate" cea/analysis/costs/supply_costs.py | head -20Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 824
🏁 Script executed:
# Find calculate_costs_for_network_type and see what it returns
sed -n '1433,1550p' cea/analysis/costs/supply_costs.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 6257
🏁 Script executed:
# Check if there's any object conversion happening between calculate_costs_for_network_type and format_simplified
sed -n '1480,1700p' cea/analysis/costs/supply_costs.py | head -100Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 5804
Line 54 will raise AttributeError: buildings in network_data are strings, not objects.
The buildings_in_network list contains building identifier strings (from nodes_df['building'].unique().tolist() in supply_costs.py), not Building objects. Accessing building.identifier on a string will fail. Use building directly in the comparison since it's already the identifier:
building_demand = demand[demand['name'] == building]🤖 Prompt for AI Agents
In cea/analysis/costs/format_simplified.py around lines 50 to 56, the loop
assumes each item in buildings_in_network is a Building object and uses
building.identifier, but buildings_in_network actually contains identifier
strings; change the comparison to use the string directly (e.g.,
demand[demand['name'] == building]) and remove the .identifier access so the
code filters by the building id string and avoids AttributeError.
| # Annualize (default network lifetime: 20 years) | ||
| network_lifetime_yrs = 20.0 | ||
| piping_cost_annual = piping_cost_total / network_lifetime_yrs |
There was a problem hiding this comment.
Piping cost annualization should use the proper formula.
Per coding guidelines, annualized CAPEX should use: CAPEX × IR% / (1 - (1 + IR%)^-LT).
The current simple division (piping_cost_total / network_lifetime_yrs) doesn't account for the time value of money and will underestimate annualized costs.
# Annualize (default network lifetime: 20 years)
network_lifetime_yrs = 20.0
- piping_cost_annual = piping_cost_total / network_lifetime_yrs
+ # Use default interest rate (typical for infrastructure)
+ ir_pct = 0.05 # 5% - consider making this configurable
+ if ir_pct > 0:
+ piping_cost_annual = piping_cost_total * ir_pct / (1 - (1 + ir_pct) ** -network_lifetime_yrs)
+ else:
+ piping_cost_annual = piping_cost_total / network_lifetime_yrsBased on coding guidelines: "Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)".
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cea/analysis/costs/supply_costs.py around lines 1303-1305, the piping cost is
currently annualized by simple division which ignores discounting; replace it
with the standard annualized CAPEX formula using the model's real discount rate
(IR) and the network_lifetime_yrs: compute piping_cost_annual =
piping_cost_total * ir / (1 - (1 + ir) ** (-network_lifetime_yrs)), ensuring ir
is a decimal (e.g., 0.03 for 3%); also add a safe fallback that if ir == 0 uses
piping_cost_total / network_lifetime_yrs to avoid division by zero.
| def anthropogenic_heat_main(locator, config): | ||
| """ | ||
| Main function to calculate anthropogenic heat rejection. | ||
| This follows the same 6-case building connectivity logic as system-costs: | ||
| - Case 1: Standalone mode (no networks) | ||
| - Cases 2-6: Network + standalone combinations | ||
| :param locator: InputLocator instance | ||
| :param config: Configuration instance with anthropogenic-heat parameters | ||
| """ | ||
| print("\n" + "=" * 70) | ||
| print("ANTHROPOGENIC HEAT ASSESSMENT") | ||
| print("=" * 70) | ||
|
|
||
| # Get network configuration | ||
| network_name = config.anthropogenic_heat.network_name | ||
| network_types = config.anthropogenic_heat.network_type | ||
|
|
||
| # Validate network types | ||
| if not network_types or len(network_types) == 0: | ||
| raise ValueError("No network types selected. Please select at least one from: DH, DC") | ||
|
|
||
| # Check for standalone-only mode | ||
| is_standalone_only = (not network_name or network_name.strip() == "" or | ||
| network_name.strip() == "(none)") | ||
|
|
||
| if is_standalone_only: | ||
| print("Mode: STANDALONE ONLY (all buildings assessed as standalone systems)") | ||
| print("District-scale supply systems will be treated as building-scale for heat calculations.") | ||
| print(f"Network types (for supply system selection): {', '.join(network_types)}\n") | ||
| else: | ||
| print(f"Mode: NETWORK + STANDALONE") | ||
| print(f"Network layout: {network_name}") | ||
| print(f"Network types: {', '.join(network_types)}\n") |
There was a problem hiding this comment.
Fix Ruff errors: unused imports, unused variable, and f-strings without placeholders
Ruff is correctly flagging several issues that will block CI:
- Line 18:
from cea.optimization_new.domain import Domainat module level is unused (Domain is only needed insidecreate_dhw_heat_rejection_system). - Line 81 & 101:
print(f"Mode: NETWORK + STANDALONE")andprint(f"\n-" * 70)are f-strings without interpolation (F541). - Line 205:
from cea.analysis.costs.supply_costs import apply_dhw_component_fallbackincalculate_standalone_heat_rejectionis unused. - Line 246:
dhw_system = apply_dhw_component_fallback(...)increate_dhw_heat_rejection_systemis assigned but never used (and the subsequent code builds its own DHW system anyway). - Line 471:
import cea.configincreate_network_supply_systemis unused.
You can address all of these with a small, safe refactor:
@@
-from cea.optimization_new.domain import Domain
@@
- if is_standalone_only:
- print("Mode: STANDALONE ONLY (all buildings assessed as standalone systems)")
- print("District-scale supply systems will be treated as building-scale for heat calculations.")
- print(f"Network types (for supply system selection): {', '.join(network_types)}\n")
- else:
- print(f"Mode: NETWORK + STANDALONE")
- print(f"Network layout: {network_name}")
- print(f"Network types: {', '.join(network_types)}\n")
+ if is_standalone_only:
+ print("Mode: STANDALONE ONLY (all buildings assessed as standalone systems)")
+ print("District-scale supply systems will be treated as building-scale for heat calculations.")
+ print(f"Network types (for supply system selection): {', '.join(network_types)}\n")
+ else:
+ print("Mode: NETWORK + STANDALONE")
+ print(f"Network layout: {network_name}")
+ print(f"Network types: {', '.join(network_types)}\n")
@@
- for network_type in network_types:
- print(f"\n-" * 70)
+ for network_type in network_types:
+ print("\n" + "-" * 70)
@@ def calculate_standalone_heat_rejection(locator, config, network_types):
- from cea.analysis.costs.supply_costs import filter_supply_code_by_scale
+ from cea.analysis.costs.supply_costs import filter_supply_code_by_scale
@@
- from cea.analysis.costs.supply_costs import apply_dhw_component_fallback
dhw_system = create_dhw_heat_rejection_system(locator, building_obj, supply_system)
@@ def create_dhw_heat_rejection_system(locator, building, main_supply_system):
- from cea.analysis.costs.supply_costs import apply_dhw_component_fallback
@@
- dhw_system = apply_dhw_component_fallback(locator, building, main_supply_system)
-
- # apply_dhw_component_fallback returns costs dict, not the supply system
- # We need to recreate the DHW supply system to get heat rejection
- # Actually, we need a different approach - let's directly call the DHW system creation
- #
- # Import the necessary modules
+ # Apply DHW fallback logic directly by constructing a DHW-only SupplySystem
@@ def create_network_supply_system(locator, config, network_type, network_name,
- from cea.optimization_new.containerclasses.energyFlow import EnergyFlow
- import cea.config
+ from cea.optimization_new.containerclasses.energyFlow import EnergyFlowThis preserves behaviour while making the file lint-clean.
Also applies to: 205-207, 246-247, 471-471
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 81-81: F541: f-string without any placeholders. Remove extraneous 'f' prefix.
🤖 Prompt for AI Agents
In cea/analysis/heat/heat_rejection.py around lines 49-83 (and additionally
address the related occurrences at 205-207, 246-247, and 471), fix Ruff F541 and
unused-import/unused-variable issues: remove or relocate unused imports (drop
module-level "from cea.optimization_new.domain import Domain" and instead import
Domain inside create_dhw_heat_rejection_system where it is needed; remove "from
cea.analysis.costs.supply_costs import apply_dhw_component_fallback" at line
~205 and the unused "import cea.config" at ~471), remove the unused assignment
dhw_system = apply_dhw_component_fallback(...) in
create_dhw_heat_rejection_system, and convert f-strings that have no
placeholders to regular string literals (e.g., change print(f"Mode: NETWORK +
STANDALONE") to print("Mode: NETWORK + STANDALONE") and print(f"\n" + "-" * 70)
or print("\n" + "-" * 70) as appropriate). Ensure behavior is unchanged and run
linter to confirm warnings are resolved.
| [system-costs] | ||
| network-name = | ||
| network-name.type = NetworkLayoutChoiceParameter | ||
| network-name.nullable = true | ||
| network-name.help = Select an existing network layout for the cost calculation, or select "(none)" to assess all buildings as standalone systems (ignoring district-scale supply settings). Networks are created by the 'thermal-network' script (part 1 and part 2 must be completed). | ||
|
|
||
| network-type = DH, DC | ||
| network-type.type = MultiChoiceParameter | ||
| network-type.choices = DH, DC | ||
| network-type.help = Type(s) of district network to calculate costs for: "DH" (district heating), "DC" (district cooling), or both. Select multiple by separating with commas. | ||
|
|
||
| supply-type-cs = | ||
| supply-type-cs.type = DistrictSupplyTypeParameter | ||
| supply-type-cs.supply-category = SUPPLY_COOLING | ||
| supply-type-cs.help = Cooling supply system type(s) for baseline cost calculations. Select up to 2 assemblies: one building-scale (for standalone buildings) and one district-scale (for district-connected buildings). CEA automatically uses the appropriate scale based on each building's connectivity. If nothing is selected, CEA will fall back to the supply systems specified in Building Properties/Supply. Select "Custom (use component settings below)" to use the component parameters instead of a predefined assembly. | ||
| supply-type-cs.category = Energy Supply Configuration | ||
|
|
||
| supply-type-hs = | ||
| supply-type-hs.type = DistrictSupplyTypeParameter | ||
| supply-type-hs.supply-category = SUPPLY_HEATING | ||
| supply-type-hs.help = Heating supply system type(s) for baseline cost calculations. Select up to 2 assemblies: one building-scale (for standalone buildings) and one district-scale (for district-connected buildings). CEA automatically uses the appropriate scale based on each building's connectivity. If nothing is selected, CEA will fall back to the supply systems specified in Building Properties/Supply. Select "Custom (use component settings below)" to use the component parameters instead of a predefined assembly. | ||
| supply-type-hs.category = Energy Supply Configuration | ||
|
|
||
| supply-type-dhw = | ||
| supply-type-dhw.type = DistrictSupplyTypeParameter | ||
| supply-type-dhw.supply-category = SUPPLY_HOTWATER | ||
| supply-type-dhw.help = Domestic hot water supply system type(s) for baseline cost calculations. Select up to 2 assemblies: one building-scale (for standalone buildings) and one district-scale (for district-connected buildings). CEA automatically uses the appropriate scale based on each building's connectivity. If nothing is selected, CEA will fall back to the supply systems specified in Building Properties/Supply. Select "Custom (use component settings below)" to use the component parameters instead of a predefined assembly. | ||
| supply-type-dhw.category = Energy Supply Configuration | ||
|
|
||
| cooling-components = VAPOR_COMPRESSION_CHILLERS, ABSORPTION_CHILLERS | ||
| cooling-components.type = MultiChoiceParameter | ||
| cooling-components.choices = VAPOR_COMPRESSION_CHILLERS, ABSORPTION_CHILLERS | ||
| cooling-components.help = Technologies to consider for district cooling plants. Order indicates priority for deployment. Used both as fallback when supply-type-cs is not set AND to determine component activation order when SUPPLY assemblies are used. | ||
| cooling-components.category = Energy Supply Configuration | ||
|
|
||
| operational = true | ||
| operational.type = BooleanParameter | ||
| operational.help = Estimate Operational costs (variable and fixed) due to supply systems operation (allocated according to the supply-systems input database) | ||
| heating-components = BOILERS, HEAT_PUMPS | ||
| heating-components.type = MultiChoiceParameter | ||
| heating-components.choices = BOILERS, HEAT_PUMPS, COGENERATION_PLANTS | ||
| heating-components.help = Technologies to consider for district heating plants. Order indicates priority for deployment. Used both as fallback when supply-type-hs is not set AND to determine component activation order when SUPPLY assemblies are used. | ||
| heating-components.category = Energy Supply Configuration | ||
|
|
||
| heat-rejection-components = COOLING_TOWERS | ||
| heat-rejection-components.type = MultiChoiceParameter | ||
| heat-rejection-components.choices = COOLING_TOWERS | ||
| heat-rejection-components.help = Technologies used for heat rejection. Used both as fallback when supply assemblies don't specify heat rejection components AND to determine component activation order when SUPPLY assemblies are used. | ||
| heat-rejection-components.category = Energy Supply Configuration | ||
|
|
||
| buildings = | ||
| buildings.type = BuildingsParameter | ||
| buildings.help = Buildings to include in solar technology installation. | ||
| buildings.category = Solar Technology Configuration | ||
|
|
||
| panels-on-roof = | ||
| panels-on-roof.type = SolarPanelChoiceParameter | ||
| panels-on-roof.help = Type(s) of solar panels to consider for rooftop installation cost calculations. | ||
| panels-on-roof.category = Solar Technology Configuration | ||
|
|
||
| panels-on-wall-north = | ||
| panels-on-wall-north.type = SolarPanelChoiceParameter | ||
| panels-on-wall-north.help = Type(s) of solar panels to consider for north-facing facade installation cost calculations. | ||
| panels-on-wall-north.category = Solar Technology Configuration | ||
|
|
||
| panels-on-wall-south = | ||
| panels-on-wall-south.type = SolarPanelChoiceParameter | ||
| panels-on-wall-south.help = Type(s) of solar panels to consider for south-facing facade installation cost calculations. | ||
| panels-on-wall-south.category = Solar Technology Configuration | ||
|
|
||
| panels-on-wall-east = | ||
| panels-on-wall-east.type = SolarPanelChoiceParameter | ||
| panels-on-wall-east.help = Type(s) of solar panels to consider for east-facing facade installation cost calculations. | ||
| panels-on-wall-east.category = Solar Technology Configuration | ||
|
|
||
| panels-on-wall-west = | ||
| panels-on-wall-west.type = SolarPanelChoiceParameter | ||
| panels-on-wall-west.help = Type(s) of solar panels to consider for west-facing facade installation cost calculations. | ||
| panels-on-wall-west.category = Solar Technology Configuration | ||
|
|
There was a problem hiding this comment.
Add default values or declare .nullable = true for empty configuration parameters.
Per the coding guidelines, all parameters in default.config must maintain default values. Several parameters in the new [system-costs] section lack explicit defaults and are missing .nullable = true declarations:
network-name(line 381): empty, has.nullable = true✓supply-type-cs/hs/dhw(lines 391, 397, 403): empty, no.nullabledeclaredbuildings(line 427): empty, no.nullabledeclaredpanels-on-*(lines 432–455): empty, no.nullabledeclared
Either assign meaningful defaults or explicitly mark parameters as nullable for clarity.
supply-type-cs =
supply-type-cs.type = DistrictSupplyTypeParameter
supply-type-cs.supply-category = SUPPLY_COOLING
+ supply-type-cs.nullable = true
supply-type-cs.help = ...
buildings =
buildings.type = BuildingsParameter
+ buildings.nullable = true
buildings.help = Buildings to include in solar technology installation.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cea/default.config around lines 380 to 456, several parameters in the
[system-costs] section (supply-type-cs at ~391, supply-type-hs ~397,
supply-type-dhw ~403, buildings ~427, panels-on-roof and panels-on-wall-*
~432–455) are empty and missing .nullable = true or a default; update each empty
parameter to either provide a sensible default value or explicitly add a
.nullable = true declaration (matching nearby parameter style), ensuring you add
the .type and .help entries remain unchanged and keep categories consistent.
| if config.result_summary.metrics_heat_rejection: | ||
| try: | ||
| calc_ubem_analytics_normalised(locator, hour_start, hour_end, "heat-rejection", summary_folder, | ||
| list_selected_time_period, bool_aggregate_by_building, bool_use_acronym, | ||
| bool_use_conditioned_floor_area_for_normalisation, plot=plot) | ||
| except Exception as e: | ||
| error_msg = f"Step 9 (Advanced Analytics - heat-rejection): {str(e)}" | ||
| errors_encountered.append(error_msg) | ||
| print(f"Warning: {error_msg}") | ||
| print(" Continuing with remaining analytics...") |
There was a problem hiding this comment.
Advanced analytics for heat rejection uses wrong feature key (heat-rejection vs heat_rejection)
In Step 9, you’ve added:
if config.result_summary.metrics_heat_rejection:
calc_ubem_analytics_normalised(locator, hour_start, hour_end, "heat-rejection", summary_folder, ...)However, throughout the rest of the module:
- The feature key is
"heat_rejection"(underscore), e.g. indict_plot_metrics_cea_featureandmap_metrics_cea_features. "heat-rejection"(hyphen) is used only as the plot folder slug via that mapping.
Passing "heat-rejection" into calc_ubem_analytics_normalised causes:
map_metrics_cea_features(..., direction="features_to_metrics")to return no metrics, since"heat-rejection"is not a key in the mapping dict.results_writer_time_periodto later receive an emptylist_metrics, which can cascade into incorrect folder resolution or aTypeErrorwhen computingtarget_path.
Use the canonical feature key with underscore here:
- if config.result_summary.metrics_heat_rejection:
- try:
- calc_ubem_analytics_normalised(locator, hour_start, hour_end, "heat-rejection", summary_folder,
+ if config.result_summary.metrics_heat_rejection:
+ try:
+ calc_ubem_analytics_normalised(locator, hour_start, hour_end, "heat_rejection", summary_folder,
list_selected_time_period, bool_aggregate_by_building, bool_use_acronym,
bool_use_conditioned_floor_area_for_normalisation, plot=plot)This keeps it consistent with the rest of the mapping and allows dict_plot_metrics_cea_feature to handle the hyphenated plot folder name when plot=True.
🤖 Prompt for AI Agents
In cea/import_export/result_summary.py around lines 3049 to 3058, the call to
calc_ubem_analytics_normalised passes the hyphenated feature key
"heat-rejection" which is not present in the module's feature-to-metric
mappings; change the fourth argument to the canonical underscored feature key
"heat_rejection" so map_metrics_cea_features and dict_plot_metrics_cea_feature
resolve metrics correctly (leave plot and other flags unchanged so the plotting
logic can still derive the hyphenated folder slug when needed).
| @pytest.mark.skip(reason="Requires reference case to be extracted and demand to be run") | ||
| def test_anthropogenic_heat_reference_case(config): |
There was a problem hiding this comment.
Missing config fixture definition (same issue as test_baseline_costs.py).
The test accepts a config parameter but no fixture is defined.
Add a fixture or ensure it exists in conftest.py.
🤖 Prompt for AI Agents
In cea/tests/test_anthropogenic_heat.py around lines 23-24, the test function
declares a config parameter but no config fixture is defined; add a pytest
fixture named config (either in this test module or centrally in conftest.py)
that yields the needed configuration object or import the existing fixture, and
update the test (or conftest) so pytest can resolve the config parameter before
running the test.
| @pytest.mark.skip(reason="Requires reference case to be extracted and demand to be run") | ||
| def test_baseline_costs_reference_case(config): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "def config" cea/tests/conftest.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 152
🏁 Script executed:
fd -name "conftest.py" .Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 321
🏁 Script executed:
cat -n cea/tests/test_baseline_costs.py | head -50Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2080
🏁 Script executed:
fd "test_.*\.py" cea/tests/ --type fRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1042
🏁 Script executed:
rg -l "def test_" cea/tests/ | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -40 {}'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 7516
🏁 Script executed:
rg "@pytest.fixture" cea/tests/ -A 2Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 576
Missing config fixture definition.
The test function accepts a config parameter but no fixture is defined in this file or in cea/tests/conftest.py. Define a config fixture at module level:
@pytest.fixture
def config():
return cea.config.Configuration()Alternatively, create cea/tests/conftest.py if it doesn't exist and add this fixture there to share it across multiple test files (test_baseline_costs.py and test_anthropogenic_heat.py both use this pattern).
🤖 Prompt for AI Agents
In cea/tests/test_baseline_costs.py around lines 23-24, the test function uses
an undefined pytest fixture named `config`; add a module-level pytest fixture
that returns a Configuration instance (or create cea/tests/conftest.py to
provide it globally) so tests receive a valid config object; implement a fixture
named `config` that returns cea.config.Configuration() and ensure pytest imports
pytest and cea.config as needed.
| # Run baseline-costs | ||
| baseline_costs_main(config) |
There was a problem hiding this comment.
Incorrect function signature: missing locator argument.
baseline_costs_main expects (locator, config) but only config is passed. This will cause a TypeError when the test is unskipped.
# Run baseline-costs
- baseline_costs_main(config)
+ locator = cea.inputlocator.InputLocator(config.scenario)
+ baseline_costs_main(locator, config)📝 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.
| # Run baseline-costs | |
| baseline_costs_main(config) | |
| # Run baseline-costs | |
| locator = cea.inputlocator.InputLocator(config.scenario) | |
| baseline_costs_main(locator, config) |
🤖 Prompt for AI Agents
In cea/tests/test_baseline_costs.py around lines 36-37, the call to
baseline_costs_main is missing the required locator argument; change the call to
baseline_costs_main(locator, config) and ensure a valid locator is available in
the test (either add a locator fixture parameter to the test function or
create/import a Locator instance before the call).
|
|
||
| baseline_costs_main(config) |
There was a problem hiding this comment.
Same signature issue: missing locator argument.
- baseline_costs_main(config)
+ locator = cea.inputlocator.InputLocator(config.scenario)
+ baseline_costs_main(locator, config)🤖 Prompt for AI Agents
In cea/tests/test_baseline_costs.py around lines 74-75, the call
baseline_costs_main(config) is missing the required locator argument; update the
test to call baseline_costs_main(config, locator) (providing the appropriate
locator fixture or constructing one) or modify the test setup to supply the
locator variable expected by the function signature so the function is invoked
with both parameters.
|
|
||
| baseline_costs_main(config) |
There was a problem hiding this comment.
Same signature issue: missing locator argument.
- baseline_costs_main(config)
+ locator = cea.inputlocator.InputLocator(config.scenario)
+ baseline_costs_main(locator, config)🤖 Prompt for AI Agents
In cea/tests/test_baseline_costs.py around lines 91-92, the call
baseline_costs_main(config) is missing the required locator argument matching
the function signature; update the test to call baseline_costs_main(config,
locator) where locator is the appropriate project locator used in other tests
(import or construct it in the test setup if not already available) so the call
matches the function signature and provides a valid locator instance.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
cea/analysis/costs/supply_costs.py (2)
769-819: Still mutatingsupply.csvin-place when applying config fallbacks
apply_config_fallbacks_for_service_needseditssupply_dfand then writes it back to the originalsupply.csv:This permanently mutates input data during a calculation run, which is risky:
- Breaks the inputs/outputs separation (scenario configuration changes without explicit user action).
- Can cause hard‑to‑trace differences between runs and issues in multi‑script workflows.
- Creates potential race conditions if scenarios are processed concurrently.
Prefer keeping these fallbacks in memory only: e.g. compute a per‑building fallback map and pass it into the domain/sizing logic, or write a temporary copy under
/outputs/and have all downstream readers use that path instead of overwriting the original.
1385-1388: Annualise piping CAPEX using the standard formula, not simple divisionPiping costs are currently annualised as:
Per the cost guidelines, annualised CAPEX should use:
CAPEX × IR% / (1 - (1 + IR%)^-LT)
Using straight division ignores discounting and underestimates annual costs for non‑zero interest rates.
Consider switching to the standard formula with a configurable interest rate (falling back to simple division when IR = 0), e.g.:
- network_lifetime_yrs = 20.0 - piping_cost_annual = piping_cost_total / network_lifetime_yrs + network_lifetime_yrs = 20.0 + ir = 0.05 # example default; ideally read from config or database + if ir > 0: + piping_cost_annual = piping_cost_total * ir / (1 - (1 + ir) ** -network_lifetime_yrs) + else: + piping_cost_annual = piping_cost_total / network_lifetime_yrscea/import_export/result_summary.py (1)
3060-3069: Advanced analytics for heat rejection still uses wrong feature key (heat-rejectionvsheat_rejection)This block still calls:
calc_ubem_analytics_normalised(..., "heat-rejection", ...)but everywhere else in this module the feature key is
"heat_rejection"(underscore), and"heat-rejection"is only the plot slug indict_plot_metrics_cea_feature. Passing the hyphenated string leads to:
map_metrics_cea_features(..., direction="features_to_metrics")returning an empty metric list,results_writer_time_periodlater seeingcea_feature=Noneand constructing an invalid folder path (TypeError).Use the canonical feature key here.
- if config.result_summary.metrics_heat_rejection: - try: - calc_ubem_analytics_normalised(locator, hour_start, hour_end, "heat-rejection", summary_folder, + if config.result_summary.metrics_heat_rejection: + try: + calc_ubem_analytics_normalised(locator, hour_start, hour_end, "heat_rejection", summary_folder, list_selected_time_period, bool_aggregate_by_building, bool_use_acronym, bool_use_conditioned_floor_area_for_normalisation, plot=plot)
🧹 Nitpick comments (8)
cea/config.py (3)
1025-1056: Avoid mutatingconfiginside DC/DH network layout parameters
DCNetworkLayoutChoiceParameter._get_available_networks(and DH equivalent) temporarily overwriteconfig.thermal_network.network_typeandconfig.optimization_new.network_type. This couples a pure “choices” helper to global configuration state and can surprise callers, especially if configuration is reused concurrently or in long‑lived UIs.Given that
InputLocatoris instantiated only withscenarioinside_get_available_networks, these temporary assignments likely have no effect on the locator path resolution and just add risk.Prefer deriving DC/DH‑specific layouts directly from locator methods (e.g. using network‑type‑specific folders or file presence) without mutating
config, or at least centralise this logic in a small, well‑documented helper with no external side‑effects.
1269-1563: DistrictSupplyTypeParameter logic looks solid; consider small clarity tweaksThe multi‑select + scale‑aware behaviour (auto‑defaults from
supply.csv, bidirectional auto‑add of missing scales, and strict validation inencode) is consistent with the documented 6‑case logic and cost workflows. The use of labels inget()while storing bare codes is also clean.Two minor clarity improvements you may want to consider:
- In
_get_auto_default/get, explicitly document in the docstring that internal storage is unlabeled butget()always returns labels (this is already implied, but being explicit will help future maintainers).- In
decode, you silently drop values not present inall_options; if that is expected only for legacy/bad configs, a short comment noting that this is intentional backward‑compatibility behaviour would avoid confusion.Functionally this class looks good to ship.
1883-1985: Align SolarPanelChoiceParameter.decode with guidelines and its docstringTwo points here:
The docstring says “Returns empty string if value is None or empty”, but
decodeactually returns'No solar technology installed'. Either the behaviour or the docstring should be updated so they match (current behaviour seems sensible given_choices).
decodecalls_get_available_solar_technologies(), which hits the filesystem viaInputLocatorandglob. Per the config guidelines,decodeshould be lenient and avoid expensive I/O and business‑rule validation; that work is better done inencode, using_choicesas the authoritative allowed set. You can typically implementdecodeas:
- map empty/None →
'No solar technology installed'- otherwise, return the raw string (possibly stripped), deferring existence checks to
encode.This would also avoid duplicating logic already present in
_choices.cea/analysis/costs/AGENTS.md (2)
7-7: Document fullget_component_codes_from_categoriessignatureThe API line currently omits the optional
peak_demand_kwparameter, but the implementation insupply_costs.pyand the examples below use it.Consider updating the signature here to:
get_component_codes_from_categories(locator, component_categories, network_type, peak_demand_kw=None) → list[str]so docs match the public API.
243-260: Add languages to fenced code blocks to satisfy markdownlintSeveral newly added code blocks use bare triple backticks without a language (e.g. decision trees and ASCII diagrams around lines 243, 408, 424, 548, 599, 654). markdownlint is flagging these with MD040.
To fix, add appropriate languages, for example:
- Use ```python for Python snippets.
- Use
text (ornone) for decision trees / ASCII diagrams.- Use ```csv where showing CSV samples.
This keeps the file linter‑clean and improves syntax highlighting without changing semantics.
Also applies to: 408-414, 424-432, 548-554, 599-608, 654-675
cea/analysis/costs/main.py (1)
82-165: Clarifybaseline_costs_mainreturn type (standalone vs network mode)In standalone mode (network-name empty/
"(none)"),baseline_costs_mainwrites CSVs and returnsNone; in network mode it returnsfinal_resultsafter saving.Since
main()ignores the return value, this is harmless, but the docstring advertises “DataFrame with cost results” unconditionally. Either update the docstring to note theNonereturn in standalone mode, or standardise to always returnfinal_results(even if the caller currently discards it).cea/analysis/costs/supply_costs.py (1)
1069-1276: District network component selection and fallbacks are consistent with AGENTS specThe
calculate_district_network_costsimplementation correctly mirrors the documented behaviour:
- Uses
filter_supply_code_by_scaleto respect BUILDING vs DISTRICT scale for SUPPLY assemblies.- When assemblies are missing or mismatched, falls back to
cooling_components/heating_components/heat_rejection_components, filtered bypeak_demand_kwviaget_component_codes_from_categories.- For baseline, limits to the first viable component per category to avoid over‑complex mixes, and excludes absorption chillers by default in DC mode.
One minor nit: the comment at line 1281 (
# ... None for fallback, dict for SUPPLY assembly) is now reversed:user_component_selectionis a dict in both modes. Updating that comment would avoid confusion; otherwise the logic looks good.cea/import_export/result_summary.py (1)
2751-2804:copy_costs_to_summaryis correct overall; minor cleanups and behaviour checksThe helper correctly:
- uses
InputLocatorfor all paths (good per guidelines),- filters both
costs_buildingsandcosts_componentsbylist_buildings,- and writes into a dedicated summary costs folder.
Two small points:
- The inner
import pandas as pdis redundant sincepdis already imported at module level; you can drop it.- Filtering
df_costs_componentsbynamewill also drop any network/plant-level rows whosenamedoes not match selected buildings; if you intend to keep plant costs even when filtering buildings, you may want a different criterion (e.g. keep non-building rows unfiltered).Would you like to retain non-building components in
costs_components.csvwhenlist_buildingsis provided, or should costs be strictly limited to the selected buildings?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cea/analysis/costs/AGENTS.md(2 hunks)cea/analysis/costs/main.py(1 hunks)cea/analysis/costs/supply_costs.py(1 hunks)cea/config.py(4 hunks)cea/import_export/result_summary.py(16 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Extract meaningful patterns into helper functions only when logic must be done in a specific way to avoid bugs, complexity warrants abstraction, algorithm requires deep understanding, or pattern encapsulates important business rules
Don't extract trivial wrappers: avoid extracting 1-2 line wrappers around existing functions, standard library usage, abstractions that obscure rather than clarify intent, or code that would be clearer inline
Always useInputLocatorfor file path resolution instead of hardcoding paths
Use British English spelling in all user-facing print statements, docstrings for public APIs, and error messages shown to users
Don't modifyconfigdirectly in worker processes; use kwargs or create new instance instead
Checkconfig.multiprocessingbefore usingPoolfor parallelization
Respect scenario structure conventions: use/inputs/and/outputs/directories for file organization
Use f-strings only when the string contains variables (e.g.,f"Value: {x}"); use regular strings otherwise (e.g.,"No variables") to avoid linter warnings
Files:
cea/analysis/costs/main.pycea/config.pycea/import_export/result_summary.pycea/analysis/costs/supply_costs.py
cea/analysis/costs/**/*.py
📄 CodeRabbit inference engine (cea/analysis/costs/AGENTS.md)
cea/analysis/costs/**/*.py: Calculate Total CAPEX as: peak_kW × efficiency × CAPEX_USD2015kW
Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Files:
cea/analysis/costs/main.pycea/analysis/costs/supply_costs.py
cea/config.py
📄 CodeRabbit inference engine (AGENTS.md)
After modifying
config.py, regenerateconfig.pyiby runningpixi run python cea/utilities/config_type_generator.py
Files:
cea/config.py
cea/**/config.py
📄 CodeRabbit inference engine (cea/AGENTS.md)
cea/**/config.py: Separate parsing from validation in Parameter classes: decode() should only perform security checks (path traversal, injection) and be lenient, while encode() should validate both security concerns and business rules
Do not validate business rules in decode() - only validate security concerns in decode() to ensure fast and lenient config file loading
Use @Property decorator for dynamic choices in ChoiceParameter subclasses to scan resources on each access rather than caching statically
Declare dependencies in ChoiceParameter.initialize() using the depends_on list when dynamic choices depend on other parameters
Extract shared validation logic into separate helper methods: use _validate_security() for checks in both decode() and encode(), and _validate_business() for checks in encode() only
Avoid validating in decode() as it causes fragile config loading - expensive I/O operations and breaking old configs when resources are deleted
Avoid caching dynamic choices without invalidation mechanisms - use @Property to ensure choices are recalculated on each access
Regenerate config.pyi type stubs after modifying parameter classes by running: pixi run python cea/utilities/config_type_generator.py
Implement Parameter.decode(value: str) → Any to parse values from config files with only security checks and no business rule validation
Implement Parameter.encode(value: Any) → str to validate values before saving with both security checks and business rule validation
Files:
cea/config.py
cea/**/AGENTS.md
📄 CodeRabbit inference engine (cea/CLAUDE.md)
cea/**/AGENTS.md: Document agent implementations with clear descriptions of purpose, inputs, outputs, and configuration in AGENTS.md
Maintain AGENTS.md as the single source of truth for agent specifications and configurations
Files:
cea/analysis/costs/AGENTS.md
cea/analysis/costs/**/AGENTS.md
📄 CodeRabbit inference engine (cea/analysis/costs/CLAUDE.md)
Document agent implementations and configurations in AGENTS.md
Files:
cea/analysis/costs/AGENTS.md
**/AGENTS.md
📄 CodeRabbit inference engine (AGENTS.md)
**/AGENTS.md: Always create context-specific documentation asAGENTS.md(notCLAUDE.md)
Write AGENTS.md with concise, actionable content (aim for <150 lines), lead with API signatures, use code examples over prose, focus on patterns not details, use scannable structure, reference related files instead of duplicating, and never add emoticons to code files or comments
Prune verbose sections in AGENTS.md - if >200 lines, look for opportunities to condense and move detailed explanations to separate*_GUIDE.mdfiles for human readers
Files:
cea/analysis/costs/AGENTS.md
🧠 Learnings (26)
📓 Common learnings
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use final energy columns (`GRID_*`, `NG_*`, `DH_*`, `DC_*`) for energy cost and emissions calculations
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Applied to files:
cea/analysis/costs/main.pycea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:06:48.123Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T23:06:48.123Z
Learning: Applies to **/cea/**/script.py : Create `main(config: Configuration)` as the entry point for new scripts in `cea/<module>/script.py`, register in `scripts.yml`, and the script auto-registers via `cea.api`
Applied to files:
cea/analysis/costs/main.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
Applied to files:
cea/analysis/costs/main.pycea/analysis/costs/AGENTS.mdcea/import_export/result_summary.pycea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use COMPONENTS/CONVERSION/*.csv files for district energy optimization with detailed equipment specifications including efficiency and capacity-dependent cost curves
Applied to files:
cea/config.pycea/analysis/costs/AGENTS.mdcea/import_export/result_summary.pycea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:00.021Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:00.021Z
Learning: Applies to cea/**/config.py : Use property decorator for dynamic choices in ChoiceParameter subclasses to scan resources on each access rather than caching statically
Applied to files:
cea/config.py
📚 Learning: 2025-11-26T23:07:00.021Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:00.021Z
Learning: Applies to cea/**/config.py : Declare dependencies in ChoiceParameter.initialize() using the depends_on list when dynamic choices depend on other parameters
Applied to files:
cea/config.py
📚 Learning: 2025-11-26T23:06:18.144Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/CLAUDE.md:0-0
Timestamp: 2025-11-26T23:06:18.144Z
Learning: Applies to cea/analysis/costs/**/AGENTS.md : Document agent implementations and configurations in AGENTS.md
Applied to files:
cea/analysis/costs/AGENTS.md
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/HVAC_*.csv : Classify HVAC components (AHU, radiators, etc.) as distribution/emission systems with no associated costs in database
Applied to files:
cea/analysis/costs/AGENTS.mdcea/import_export/result_summary.pycea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the `calc_capex_annualized(capex_total, IR_%, LT_yr)` function to convert total CAPEX to annual cost
Applied to files:
cea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Applied to files:
cea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Applied to files:
cea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Total CAPEX as: peak_kW × efficiency × CAPEX_USD2015kW
Applied to files:
cea/analysis/costs/AGENTS.md
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use ASSEMBLIES/SUPPLY/*.csv files (not COMPONENTS) for building-level demand calculations with conservative efficiency values and single baseline costs
Applied to files:
cea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
Applied to files:
cea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Read CAPEX parameters from SUPPLY assemblies using files matching pattern `databases/{region}/ASSEMBLIES/SUPPLY/SUPPLY_*.csv`
Applied to files:
cea/analysis/costs/AGENTS.mdcea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Applied to files:
cea/analysis/costs/AGENTS.mdcea/import_export/result_summary.pycea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Read variable OPEX (energy purchase costs) from feedstocks using files matching pattern `databases/{region}/COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/*.csv`
Applied to files:
cea/analysis/costs/AGENTS.md
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use final energy columns (`GRID_*`, `NG_*`, `DH_*`, `DC_*`) for energy cost and emissions calculations
Applied to files:
cea/analysis/costs/AGENTS.mdcea/import_export/result_summary.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use end-use columns (`Q*_sys_kWh`) for HVAC system sizing and capacity calculations
Applied to files:
cea/import_export/result_summary.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/thermal_loads.py : Calculate final energy from end-use energy by dividing by efficiency: `final_energy = Qhs_sys / efficiency_average_year` in thermal load calculations
Applied to files:
cea/import_export/result_summary.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Document the distinction between end-use energy (includes HVAC losses) and final energy (includes efficiency losses) in output datasets
Applied to files:
cea/import_export/result_summary.py
📚 Learning: 2025-11-26T23:08:17.967Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/technologies/network_layout/AGENTS.md:0-0
Timestamp: 2025-11-26T23:08:17.967Z
Learning: Applies to cea/technologies/network_layout/**/*.py : Build thermal network connectivity by calling `calc_connectivity_network_with_geometry(streets_gdf, buildings_gdf, connection_candidates=k)` to produce NetworkX graph with preserved geometries, normalized node coordinates, and building_terminals metadata
Applied to files:
cea/import_export/result_summary.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Ensure CAPEX is annualized before comparing with annual OPEX figures
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Applied to files:
cea/analysis/costs/supply_costs.py
🧬 Code graph analysis (3)
cea/analysis/costs/main.py (4)
cea/inputlocator.py (3)
InputLocator(28-1456)get_baseline_costs(1370-1379)get_baseline_costs_detailed(1381-1390)cea/analysis/costs/supply_costs.py (2)
calculate_all_buildings_as_standalone(459-481)calculate_standalone_building_costs(668-766)cea/analysis/costs/format_simplified.py (1)
format_output_simplified(19-202)cea/analysis/costs/solar_costs.py (2)
calculate_building_solar_costs(252-448)merge_solar_costs_to_buildings(451-513)
cea/config.py (2)
cea/inputlocator.py (2)
get_building_supply(917-919)get_potentials_solar_folder(1217-1219)cea/utilities/__init__.py (1)
parse_string_to_list(90-96)
cea/import_export/result_summary.py (1)
cea/inputlocator.py (4)
get_heat_rejection_buildings(1397-1406)get_heat_rejection_hourly_building(1419-1429)get_baseline_costs(1370-1379)get_baseline_costs_detailed(1381-1390)
🪛 GitHub Actions: Ruff
cea/import_export/result_summary.py
[error] 1384-1384: F821 Undefined name network_name
[error] 1407-1407: F821 Undefined name network_name
[error] 1456-1456: F821 Undefined name network_name
cea/analysis/costs/supply_costs.py
[error] 678-679: F401 [*] os imported but unused
🪛 LanguageTool
cea/analysis/costs/AGENTS.md
[style] ~357-~357: Consider using an alternative to strengthen your wording.
Context: ...designed for standalone buildings, user wants to explore district networks --- ####...
(WANT_KEEN)
[style] ~481-~481: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ices the building provides standalone - Only applies if Building Properties/Supply h...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
cea/analysis/costs/AGENTS.md
243-243: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
408-408: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
424-424: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
548-548: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
599-599: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
654-654: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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 (macos-latest)
- GitHub Check: tests (windows-latest)
- GitHub Check: tests (ubuntu-latest)
🔇 Additional comments (6)
cea/analysis/costs/main.py (1)
170-259: Error handling for per‑network failures looks robustThe validation/calculation flow for network mode is sensible:
- Per‑type validation with
validate_network_results_existand clear messages;- Per‑type calculation with selective skipping for “no demand/no valid supply system” cases;
- Aggregation of
validation_errorsandcalculation_errorsinto a singleRuntimeErrorbefore saving, ensuring partial network results are not written.This behaviour matches the multi‑network design described in AGENTS.md and keeps outputs consistent.
Also applies to: 300-329
cea/analysis/costs/supply_costs.py (1)
225-427: DHW fallback logic is well thought‑throughThe DHW fallback path (
get_dhw_component_fallback+apply_dhw_component_fallback) cleanly:
- Maps feedstocks to boiler fuel codes and components using the BOILERS CSV;
- Sizes the fallback system from hourly
Qww_sys_kWh;- Builds a standalone
SupplySystemwith correctT60Wcarrier and re‑labels services to_ww.This is a solid safety net for DHW‑only buildings and aligns with the documented Level‑3B fallback behaviour.
cea/import_export/result_summary.py (4)
69-70: Heat-rejection plot slug wiring looks consistentMapping
'heat_rejection'→'heat-rejection'here aligns with the feature key used elsewhere and the hyphenated folder slug convention; no issues spotted.
134-135: Heat-rejection metric is consistently wired through mappings and selectorsThe additions for
"heat_rejection[kWh]"in:
normalisation_name_mapping,map_metrics_cea_features,map_metrics_and_cea_columns,- and all the
list_metrics_*helpers (with/without date, building, and plot variants),are internally consistent and should allow the new metric to flow correctly through selection, loading, aggregation, and normalisation.
Also applies to: 397-399, 681-683, 2619-2620, 2654-2655, 2682-2683, 2721-2723, 2745-2746
266-275: Heat-rejection paths ignorelist_buildingsfilter — confirm intended behaviourThis branch derives entity names solely from
heat_rejection_buildings.csvand completely ignores thelist_buildingsfilter passed intoget_results_path. That means heat-rejection summaries will always include all entities in the network file, even if the user filtered buildings earlier.Would you like heat-rejection results to honour the same building filters (intersection of
entity_namesandlist_buildings), or is the “all entities in network” behaviour intentional for this feature?
2976-2991: Step 8.5 integration ofcopy_costs_to_summarylooks goodThe new Step 8.5 hooks
metrics_costsinto the pipeline cleanly: it only runs for non-plot summaries, respectsnetwork_name, and follows the same error-handling pattern as earlier steps (collecting and logging warnings instead of aborting).
| import os | ||
|
|
||
| print(" Loading buildings and demands...") | ||
|
|
||
| # 0. Determine STANDALONE mode or NETWORK mode | ||
| if network_name is None or network_name == "(none)": | ||
| all_buildings_standalone = True | ||
| else: | ||
| all_buildings_standalone = False | ||
|
|
There was a problem hiding this comment.
Remove unused inner import os (Ruff F401)
Inside calculate_standalone_building_costs you re‑import os:
os is already imported at module level and not used in this function, which Ruff flags as F401.
You can safely delete the inner import os line to satisfy the linter.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 678-679: F401 [*] os imported but unused
🤖 Prompt for AI Agents
In cea/analysis/costs/supply_costs.py around lines 678 to 687 there is an
unnecessary inner "import os" inside calculate_standalone_building_costs which
is redundant because os is imported at module level and not used in the
function; simply remove that inner import line to satisfy Ruff F401 and keep the
function using the module-level os if needed.
| # Rename heat_rejection_kW to heat_rejection_kWh for consistency with energy units | ||
| if 'heat_rejection_kW' in df.columns: | ||
| df = df.rename(columns={'heat_rejection_kW': 'heat_rejection_kWh'}) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the specific file and context around the problematic lines
cd /dev/shm/repo && sed -n '850,880p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 154
🏁 Script executed:
# Search for 'heat_rejection_kW' across the entire codebase
rg 'heat_rejection_kW' -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2640
🏁 Script executed:
# Search for where heat_rejection columns are created or sourced
rg 'heat_rejection' -n -A 2 -B 2Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 50405
🏁 Script executed:
# Check for any transformations or calculations related to this column
ast-grep --pattern 'heat_rejection'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2981
🏁 Script executed:
sed -n '820,880p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 3179
🏁 Script executed:
# Check what data is being processed/imported at this point
sed -n '1,100p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 4449
🏁 Script executed:
# Look for any documentation or comments about heat_rejection handling
rg -B 5 -A 5 'heat_rejection_kW.*heat_rejection_kWh' cea/import_export/result_summary.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 925
🏁 Script executed:
# Check the file structure and method names
rg -n 'def ' cea/import_export/result_summary.py | head -30Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2766
🏁 Script executed:
# Check what happens downstream with the renamed 'heat_rejection_kWh' column
rg 'heat_rejection_kWh' cea/import_export/result_summary.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 331
🏁 Script executed:
# Check if values are converted or aggregated after the rename in this function
sed -n '844,950p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 6501
🏁 Script executed:
# Verify the schema definition once more to confirm units
grep -A 5 'heat_rejection_kW:' cea/schemas.ymlRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 794
🏁 Script executed:
# Final check: Verify if any conversion is applied to the hourly data to transform it to energy
rg -B 3 -A 3 'heat_rejection.*sum\|sum.*heat_rejection' cea/import_export/result_summary.py | head -40Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 73
🏁 Script executed:
# Check if aggregation or summation converts hourly power to energy
sed -n '937,1050p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 6102
The heat_rejection_kW to heat_rejection_kWh rename is a semantic error that misrepresents power as energy.
The source column heat_rejection_kW contains hourly power rates (documented in cea/schemas.yml as "Heat rejection rate at this hour" with unit [kW]). Renaming to heat_rejection_kWh without converting the values is incorrect—this mislabels power as energy. Downstream aggregation functions then sum these power values thinking they represent energy, which violates the expected energy unit convention used throughout the codebase (e.g., Q*_sys_kWh columns).
To fix: either multiply hourly power by 1 hour to produce actual energy values before renaming, or retain the kW unit and adjust result mappings accordingly.
| # For heat_rejection, use entity names from the data files instead of zone buildings | ||
| if appendix == 'heat_rejection': | ||
| # Get entity names from heat_rejection_buildings.csv | ||
| heat_buildings_df = pd.read_csv(locator.get_heat_rejection_buildings(network_name=network_name)) | ||
| entity_names = heat_buildings_df['name'].tolist() | ||
| num_entities_per_period = len(entity_names) | ||
| else: | ||
| entity_names = list_buildings | ||
| num_entities_per_period = len(list_buildings) | ||
|
|
||
| if len(hourly_annually_df) / len(hourly_annually_df['period'].unique().tolist()) == num_entities_per_period: | ||
| hourly_annually_df.insert(0, 'name', entity_names) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and its size
wc -l cea/import_export/result_summary.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 131
🏁 Script executed:
# Read the function signature and surrounding context
sed -n '1370,1395p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1901
🏁 Script executed:
# Read the other two locations mentioned
sed -n '1395,1420p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2157
🏁 Script executed:
# Read lines 1453-1464
sed -n '1450,1470p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1701
🏁 Script executed:
# Find the function definition of exec_aggregate_building
rg -n "def exec_aggregate_building" cea/import_export/result_summary.py -A 1Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 592
🏁 Script executed:
# Find all calls to exec_aggregate_building
rg -n "exec_aggregate_building(" cea/import_export/result_summary.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 189
🏁 Script executed:
# Use simpler grep search for function calls
grep -n "exec_aggregate_building(" cea/import_export/result_summary.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 550
🏁 Script executed:
# Get more context around those call sites
grep -n "exec_aggregate_building(" cea/import_export/result_summary.py -B 5 -A 2Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1493
🏁 Script executed:
# Check if network_name is available in the caller's context (look for the function containing the call)
sed -n '1700,1750p' cea/import_export/result_summary.py | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 3296
🏁 Script executed:
# Let's find where this is called - search backwards from line 1700+
sed -n '1600,1700p' cea/import_export/result_summary.py | tail -50 | cat -nRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2979
Add missing network_name parameter to exec_aggregate_building function
The function uses network_name at lines 1385, 1413, and 1457 when calling locator.get_heat_rejection_buildings(network_name=network_name), but network_name is not defined as a function parameter. This causes a runtime NameError (F821).
Add network_name to the function signature and pass it from the call site at line 2968 where network_name is available:
-def exec_aggregate_building(locator, hour_start, hour_end, summary_folder, list_metrics, bool_use_acronym, list_list_useful_cea_results, list_buildings, list_appendix, list_selected_time_period, date_column='date', plot=False):
+def exec_aggregate_building(locator, hour_start, hour_end, summary_folder, list_metrics, bool_use_acronym, list_list_useful_cea_results, list_buildings, list_appendix, list_selected_time_period, date_column='date', plot=False, network_name=None):And at the call site (line 2968):
- exec_aggregate_building(locator, hour_start, hour_end, summary_folder, list_metrics, bool_use_acronym, list_list_useful_cea_results, list_buildings, list_appendix, list_selected_time_period, plot=plot)
+ exec_aggregate_building(locator, hour_start, hour_end, summary_folder, list_metrics, bool_use_acronym, list_list_useful_cea_results, list_buildings, list_appendix, list_selected_time_period, plot=plot, network_name=network_name)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 1384-1384: F821 Undefined name network_name
🤖 Prompt for AI Agents
In cea/import_export/result_summary.py around lines 1381 to 1392, the function
exec_aggregate_building uses network_name when calling
locator.get_heat_rejection_buildings at lines ~1385, ~1413 and ~1457 but
network_name is not a parameter, causing a NameError; add network_name to the
exec_aggregate_building function signature, update any internal references to
use that parameter, and then update the call site at line 2968 to pass the
available network_name variable into exec_aggregate_building.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
cea/analysis/costs/supply_costs.py (3)
821-824: Verify if input modification issue is fully resolved.Past review flagged writing to
supply.csvas risky (race conditions, inconsistent state). While marked as addressed, the code still modifiessupply.csvat line 824. Ensure this approach is intentional or consider in-memory-only fallbacks.#!/bin/bash # Verify if there are any safeguards or documentation about this write operation # Check for comments or safeguards around the supply.csv write rg -n -B5 -A5 "supply_df.to_csv\(supply_csv_path" cea/analysis/costs/supply_costs.py # Check if there's any locking mechanism or transaction handling rg -n "lock|transaction|backup" cea/analysis/costs/supply_costs.py
1391-1393: Use proper annualization formula for piping costs.Per coding guidelines, annualized CAPEX should use:
CAPEX × IR% / (1 - (1 + IR%)^-LT). Simple division doesn't account for time value of money and underestimates costs.Apply this diff:
# Annualize (default network lifetime: 20 years) network_lifetime_yrs = 20.0 - piping_cost_annual = piping_cost_total / network_lifetime_yrs + # Use standard annualization with interest rate (default 5% for infrastructure) + ir = 0.05 # 5% interest rate - consider making configurable + if ir > 0: + piping_cost_annual = piping_cost_total * ir / (1 - (1 + ir) ** -network_lifetime_yrs) + else: + piping_cost_annual = piping_cost_total / network_lifetime_yrsBased on coding guidelines: "Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)".
684-684: Remove unused import.
osis already imported at module level (line 9) and not used in this function.Apply this diff:
- import os - print(" Loading buildings and demands...")
🧹 Nitpick comments (1)
cea/optimization_new/containerclasses/energyCarrier.py (1)
338-367: Validate T60W/T100W threshold assumptions and add tests for new tie-breakingThe new 90 °C split for water (forcing all ≤90 °C to 60 and >90 °C to 100) and the “prefer lower temperature on ties” logic both look consistent and deterministic, but they bake in domain assumptions that are not enforced by the data:
- The special-case will always choose 60 or 100 whenever both exist, even if other water temperatures are later added and would be a closer physical match.
- The tie-breaker behaviour (choosing the lower mean_qual) changes previous implicit semantics; any downstream logic relying on the old arbitrary choice should be re-checked.
I recommend:
- Confirming that the water subtype is guaranteed to only have 60 °C and 100 °C entries (now and in foreseeable DB versions), or explicitly documenting that intermediate values will be ignored by design.
- Adding unit tests for:
- water at 80–82 °C and just below/above 90 °C to confirm the T60W/T100W mapping;
- a subtype with symmetric options around a target (e.g. 40/60 around 50) to lock in the “lower temperature wins” behaviour.
This will make the new selection rules explicit and guard against future DB changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cea/analysis/costs/supply_costs.py(1 hunks)cea/optimization_new/containerclasses/energyCarrier.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Extract meaningful patterns into helper functions only when logic must be done in a specific way to avoid bugs, complexity warrants abstraction, algorithm requires deep understanding, or pattern encapsulates important business rules
Don't extract trivial wrappers: avoid extracting 1-2 line wrappers around existing functions, standard library usage, abstractions that obscure rather than clarify intent, or code that would be clearer inline
Always useInputLocatorfor file path resolution instead of hardcoding paths
Use British English spelling in all user-facing print statements, docstrings for public APIs, and error messages shown to users
Don't modifyconfigdirectly in worker processes; use kwargs or create new instance instead
Checkconfig.multiprocessingbefore usingPoolfor parallelization
Respect scenario structure conventions: use/inputs/and/outputs/directories for file organization
Use f-strings only when the string contains variables (e.g.,f"Value: {x}"); use regular strings otherwise (e.g.,"No variables") to avoid linter warnings
Files:
cea/optimization_new/containerclasses/energyCarrier.pycea/analysis/costs/supply_costs.py
cea/analysis/costs/**/*.py
📄 CodeRabbit inference engine (cea/analysis/costs/AGENTS.md)
cea/analysis/costs/**/*.py: Calculate Total CAPEX as: peak_kW × efficiency × CAPEX_USD2015kW
Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Files:
cea/analysis/costs/supply_costs.py
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/HVAC_*.csv : Classify HVAC components (AHU, radiators, etc.) as distribution/emission systems with no associated costs in database
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use ASSEMBLIES/SUPPLY/*.csv files (not COMPONENTS) for building-level demand calculations with conservative efficiency values and single baseline costs
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use final energy columns (`GRID_*`, `NG_*`, `DH_*`, `DC_*`) for energy cost and emissions calculations
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use COMPONENTS/CONVERSION/*.csv files for district energy optimization with detailed equipment specifications including efficiency and capacity-dependent cost curves
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use ASSEMBLIES/SUPPLY/*.csv files (not COMPONENTS) for building-level demand calculations with conservative efficiency values and single baseline costs
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use COMPONENTS/CONVERSION/*.csv files for district energy optimization with detailed equipment specifications including efficiency and capacity-dependent cost curves
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Read CAPEX parameters from SUPPLY assemblies using files matching pattern `databases/{region}/ASSEMBLIES/SUPPLY/SUPPLY_*.csv`
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the `calc_capex_annualized(capex_total, IR_%, LT_yr)` function to convert total CAPEX to annual cost
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Ensure CAPEX is annualized before comparing with annual OPEX figures
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Applied to files:
cea/analysis/costs/supply_costs.py
🪛 GitHub Actions: Ruff
cea/analysis/costs/supply_costs.py
[error] 375-375: Ruff: f-string without placeholders. Remove extraneous f prefix.
[error] 684-684: Ruff: Unused import 'os'.
⏰ 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 (ubuntu-latest)
- GitHub Check: tests (macos-latest)
🔇 Additional comments (16)
cea/analysis/costs/supply_costs.py (16)
25-70: LGTM: Component extraction logic is sound.The function correctly extracts components from SUPPLY assembly definitions with appropriate fallbacks for missing data.
73-182: LGTM: Scale filtering logic is correct.The functions properly handle multi-select supply codes and filter by scale (BUILDING vs DISTRICT) with appropriate fallbacks.
184-222: LGTM: Component code mapping with capacity filtering.The function correctly maps component categories to codes and optionally filters by peak demand capacity.
225-279: LGTM: DHW component fallback logic is sound.The function provides a reasonable fallback mechanism for DHW systems when SUPPLY assemblies don't specify components.
438-487: LGTM: Service filtering and standalone calculation.Both functions are correctly implemented with clear logic.
490-567: LGTM: Config fallback logic for scale mismatch.The function correctly applies config fallbacks when supply.csv has district-scale codes for standalone services.
570-671: LGTM: Network connectivity and service needs determination.All three functions are correctly implemented with proper 4-case logic for determining building service needs.
827-922: LGTM: Heating system calculation with proper network awareness.The function correctly calculates heating systems with appropriate scale determination based on network connectivity.
925-1019: LGTM: Cooling system calculation mirrors heating logic.The function correctly calculates cooling systems with consistent scale determination logic.
1022-1072: LGTM: System merging logic is sound.The function correctly merges heating and cooling systems with proper component and cost aggregation.
1413-1518: LGTM: Network cost calculation with proper aggregation.The function correctly calculates network costs with demand aggregation and potential summation.
1521-1743: LGTM: Comprehensive network-type cost calculation.The function correctly orchestrates cost calculations for DH/DC networks with proper four-case logic and service filtering.
1746-1874: LGTM: Cost extraction with proper TAC calculation.The function correctly extracts costs from supply systems and calculates Total Annualized Cost (TAC) as per guidelines.
1877-1896: LGTM: Scale determination based on actual connectivity.The function correctly determines scale based on actual network connectivity rather than assembly configuration.
1899-1984: LGTM: Component-to-service mapping with smart inference.The function correctly maps components to services with proper fallback logic and temperature-based service inference.
1987-2054: LGTM: Energy carrier mapping with comprehensive coverage.The function correctly maps energy carriers to services with appropriate handling of ambiguous cases.
cea/analysis/costs/supply_costs.py
Outdated
| energy_flow_profile=dhw_demand_profile | ||
| ) | ||
|
|
||
| print(f" DHW fallback: EnergyFlow created with T60W carrier") |
There was a problem hiding this comment.
Remove extraneous f prefix.
This string has no placeholders, so the f prefix is unnecessary and triggers a linter warning.
Apply this diff:
- print(f" DHW fallback: EnergyFlow created with T60W carrier")
+ print(" DHW fallback: EnergyFlow created with T60W carrier")📝 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.
| print(f" DHW fallback: EnergyFlow created with T60W carrier") | |
| print(" DHW fallback: EnergyFlow created with T60W carrier") |
🧰 Tools
🪛 GitHub Actions: Ruff
[error] 375-375: Ruff: f-string without placeholders. Remove extraneous f prefix.
🤖 Prompt for AI Agents
In cea/analysis/costs/supply_costs.py around line 375, remove the unnecessary
f-string prefix on the print literal; replace the f-prefixed print call with a
normal string print so it reads print(" DHW fallback: EnergyFlow created with
T60W carrier") to eliminate the linter warning.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cea/analysis/costs/main.py (3)
22-56: Prefer InputLocator accessors for thermal-network shapefiles instead of hardcoded filenamesThe logic is fine, but you are partly hardcoding paths for
nodes.shp/edges.shp. IfInputLocatorexposes dedicated helpers for these layout shapefiles, it would be more robust to use them end‑to‑end instead of assembling filenames manually, so future path/name changes are centralised in one place.As per coding guidelines, prefer using
InputLocatorfor file path resolution rather than hardcoding filenames.
176-204: Clarify or relax behaviour when some requested network types fail (validation / “no supply system” / “no demand”)The current flow treats several conditions as non-fatal in the moment but still aborts the whole run later:
- Network types that fail
validate_network_results_existare logged under “VALIDATION WARNINGS” and you continue withvalid_network_types, but they are then included inall_errorsand cause a finalRuntimeError.- Network types skipped due to “None of the components chosen … / T30W / T10W” (with logs explicitly saying this is “Expected for scenarios with no {network_type} demand”) are also accumulated in
calculation_errorsand likewise make the overall run fail at theall_errorscheck, even if other types succeeded.That means you never actually persist partial results when any requested type is missing or has zero demand, which is stricter than what the log messages suggest.
Consider one of:
- Treating “expected” cases (e.g. zero demand / no valid systems) as non-fatal and excluding them from
all_errors, so that successful network types still produce saved outputs; or- Keeping the fail‑fast behaviour but updating the earlier log messages to make it clear that these are hard errors for the overall request, not mere warnings.
This would make the UX more predictable when users select multiple network types but only some are meaningful for a given scenario.
Also applies to: 213-259, 299-329
345-351: Heuristicname.startswith('N')for detecting networks may misclassify buildingsUsing
has_networks = any(name.startswith('N') for name in final_results['name'])assumes that all network rows start with'N'and that no ordinary building does. In larger projects, regular buildings can easily start with'N'(e.g. “NORTH01”), which would trigger the network note even when there are no actual network plants.If
final_resultscarries some explicit flag/column for network assets (e.g. a type / category / “is_network” field), or if you know the exact naming convention (e.g.Nfollowed by digits only), it would be more robust to rely on that instead of a single‑letter prefix check.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cea/analysis/costs/main.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Extract meaningful patterns into helper functions only when logic must be done in a specific way to avoid bugs, complexity warrants abstraction, algorithm requires deep understanding, or pattern encapsulates important business rules
Don't extract trivial wrappers: avoid extracting 1-2 line wrappers around existing functions, standard library usage, abstractions that obscure rather than clarify intent, or code that would be clearer inline
Always useInputLocatorfor file path resolution instead of hardcoding paths
Use British English spelling in all user-facing print statements, docstrings for public APIs, and error messages shown to users
Don't modifyconfigdirectly in worker processes; use kwargs or create new instance instead
Checkconfig.multiprocessingbefore usingPoolfor parallelization
Respect scenario structure conventions: use/inputs/and/outputs/directories for file organization
Use f-strings only when the string contains variables (e.g.,f"Value: {x}"); use regular strings otherwise (e.g.,"No variables") to avoid linter warnings
Files:
cea/analysis/costs/main.py
cea/analysis/costs/**/*.py
📄 CodeRabbit inference engine (cea/analysis/costs/AGENTS.md)
cea/analysis/costs/**/*.py: Calculate Total CAPEX as: peak_kW × efficiency × CAPEX_USD2015kW
Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Files:
cea/analysis/costs/main.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/HVAC_*.csv : Classify HVAC components (AHU, radiators, etc.) as distribution/emission systems with no associated costs in database
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use final energy columns (`GRID_*`, `NG_*`, `DH_*`, `DC_*`) for energy cost and emissions calculations
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use ASSEMBLIES/SUPPLY/*.csv files (not COMPONENTS) for building-level demand calculations with conservative efficiency values and single baseline costs
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use COMPONENTS/CONVERSION/*.csv files for district energy optimization with detailed equipment specifications including efficiency and capacity-dependent cost curves
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use end-use columns (`Q*_sys_kWh`) for HVAC system sizing and capacity calculations
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Applied to files:
cea/analysis/costs/main.py
📚 Learning: 2025-11-26T23:06:48.123Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T23:06:48.123Z
Learning: Applies to **/cea/**/script.py : Create `main(config: Configuration)` as the entry point for new scripts in `cea/<module>/script.py`, register in `scripts.yml`, and the script auto-registers via `cea.api`
Applied to files:
cea/analysis/costs/main.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
Applied to files:
cea/analysis/costs/main.py
⏰ 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 (ubuntu-latest)
- GitHub Check: tests (macos-latest)
- GitHub Check: tests (windows-latest)
🔇 Additional comments (1)
cea/analysis/costs/main.py (1)
90-96: Good orchestration: cost logic stays in dedicated modules, consistent with cost-data guidelinesThe orchestration here cleanly delegates all actual cost calculations to
supply_costs,solar_costs, andformat_simplified, and only handles merging, error handling, and I/O. That keeps the CAPEX / OPEX business rules centralised instead of being re‑implemented in this script, which should help avoid divergence between baseline‑costs and other cost analyses.Based on learnings, keeping cost formulas in the dedicated cost modules (using SUPPLY and FEEDSTOCKS data) rather than in orchestration code matches the recommended architecture for
cea/analysis/costs/**/*.py.Also applies to: 205-213, 260-283
| def baseline_costs_main(locator, config): | ||
| """ | ||
| Calculate baseline costs for heating and/or cooling systems. | ||
| :param locator: InputLocator instance | ||
| :param config: Configuration instance | ||
| :return: DataFrame with cost results | ||
| """ |
There was a problem hiding this comment.
Align baseline_costs_main return value with its docstring across both modes
The docstring says baseline_costs_main “returns a DataFrame with cost results”, and the network mode does return final_results, but the standalone‑only branch ends with a bare return, so callers will receive None in that case.
I’d recommend returning final_results in the standalone branch as well to keep the function contract consistent (or, alternatively, adjust the docstring and stop returning in the network branch, but that weakens the API).
- print("\nNote: Standalone-only mode provides simplified cost estimates")
- return
+ print("\nNote: Standalone-only mode provides simplified cost estimates")
+ return final_resultsAlso applies to: 158-164, 352-352
🤖 Prompt for AI Agents
In cea/analysis/costs/main.py around lines 82-89 (also check similar patterns at
158-164 and line 352), the function baseline_costs_main's docstring promises
"returns a DataFrame with cost results" but the standalone-only branch ends with
a bare return (None), causing inconsistent return types; update the standalone
branch to return final_results (or the equivalent DataFrame variable used there)
so both network and standalone branches consistently return the DataFrame, and
make the same change for the other occurrences on lines 158-164 and 352 to
preserve the function contract.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
cea/analysis/costs/main.py (1)
82-89: Inconsistent return value inbaseline_costs_main.The docstring promises "returns a DataFrame with cost results", but standalone mode (line 164) has a bare
returnyieldingNone, while network mode (line 352) returnsfinal_results. This contract inconsistency can causeTypeErrorif callers chain operations on the result.Apply this diff to fix the standalone branch:
print("\nNote: Standalone-only mode provides simplified cost estimates") - return + return final_resultsAlso applies to: 158-164, 352-352
cea/analysis/costs/supply_costs.py (2)
1385-1387: Piping cost annualization should use proper formula.Per coding guidelines, annualized CAPEX should use:
CAPEX × IR% / (1 - (1 + IR%)^-LT). The current simple division doesn't account for the time value of money.# Annualize (default network lifetime: 20 years) network_lifetime_yrs = 20.0 - piping_cost_annual = piping_cost_total / network_lifetime_yrs + # Use standard annualization formula with interest rate + ir_pct = 0.05 # 5% - consider making this configurable or reading from database + if ir_pct > 0: + piping_cost_annual = piping_cost_total * ir_pct / (1 - (1 + ir_pct) ** -network_lifetime_yrs) + else: + piping_cost_annual = piping_cost_total / network_lifetime_yrsAs per coding guidelines: "Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)".
678-678: Remove unusedosimport causing pipeline failure.The
osmodule is imported at line 678 but never used withincalculate_standalone_building_costs. It's already available from the module-level import at line 9.def calculate_standalone_building_costs(locator, config, network_name): - import os - print(" Loading buildings and demands...")
🧹 Nitpick comments (6)
cea/optimization_new/containerclasses/energyCarrier.py (1)
338-354: Tie‑break logic for equidistant temperatures looks correct; consider a small robustness guardThe new distance/tie handling correctly:
- Picks all entries at the minimum absolute distance, and
- Chooses the lower temperature when there is more than one candidate.
This achieves the “prefer lower temperature” requirement without changing behaviour in the unique‑closest case. Nice improvement.
One optional robustness tweak: if, for some reason,
thermal_ec_mean_qualscontained no finite values for a subtype (all NaN or filtered out),tied_indiceswould be empty andtied_indices[0]would raise anIndexError(same as the old implementation, but less obvious here). You could fail more explicitly:- tied_indices = distances[distances == min_distance].index - - if len(tied_indices) > 1: + tied_indices = distances[distances == min_distance].index + + if len(tied_indices) == 0: + raise ValueError( + f"No valid mean_qual values found for thermal energy carrier subtype " + f"'{energy_carrier_subtype}'." + ) + elif len(tied_indices) > 1: # Multiple carriers at same distance - choose the one with LOWER temperature tied_temps = thermal_ec_mean_quals.loc[tied_indices] index_closest_mean_temp = tied_temps.idxmin() else: # Single closest match index_closest_mean_temp = tied_indices[0]This keeps the current semantics while turning a low‑level
IndexErrorinto a clearer data‑quality error.cea/analysis/costs/main.py (2)
109-109: Simplify redundant falsy checks.The condition
not network_name or network_name == ''is redundant since an empty string is already falsy in Python.- if not network_name or network_name == '' or network_name == '(none)': + if not network_name or network_name == '(none)':
346-350: Network detection relies on fragile naming convention.The check
name.startswith('N')assumes all network identifiers begin with 'N'. If network naming conventions change, this will silently fail.Consider using the
is_networkflag from the results structure instead:- has_networks = any(name.startswith('N') for name in final_results['name']) + # Check if any results represent network-level systems + has_networks = any( + merged_results.get(name, {}).get('is_network', False) + for name in final_results['name'] if name in merged_results + )Alternatively, store network identifiers explicitly during result merging.
cea/analysis/costs/supply_costs.py (3)
184-222: Remove redundant pandas import.
pandasis already imported at module level (line 10).def get_component_codes_from_categories(locator, component_categories, network_type, peak_demand_kw=None): - import pandas as pd - component_codes = []
371-379: Heavy initialization just to populate class variables.Creating a full
Domainobject solely to initializeComponent.code_to_class_mappingis expensive. Consider extracting the component initialization logic or caching the initialized state.# Initialize Component class if not already done # This is needed to map component codes to their energy carriers if Component.code_to_class_mapping is None: - # Create a minimal domain just to initialize Component class - domain_config_dhw = cea.config.Configuration() - # Get scenario from locator (locator.scenario is the current scenario path) - domain_config_dhw.scenario = locator.scenario - temp_domain = Domain(domain_config_dhw, locator) - Component.initialize_class_variables(temp_domain) + # Initialize Component class directly using locator + # Note: Component.initialize_class_variables requires a domain with locator attribute + from cea.optimization_new.component import Component + Component._load_components(locator) # If available, otherwise keep current approachIf direct initialization isn't possible, document why the full
Domainis needed.
858-866: Stdout suppression is acceptable for clean output.The use of
sys.stdoutredirection to suppress verbose optimization messages is reasonable for user experience. Consider using a context manager for cleaner resource handling.from contextlib import redirect_stdout import io with redirect_stdout(io.StringIO()): domain.load_potentials() domain._initialize_energy_system_descriptor_classes()Also applies to: 961-969
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cea/analysis/costs/main.py(1 hunks)cea/analysis/costs/supply_costs.py(1 hunks)cea/optimization_new/containerclasses/energyCarrier.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Extract meaningful patterns into helper functions only when logic must be done in a specific way to avoid bugs, complexity warrants abstraction, algorithm requires deep understanding, or pattern encapsulates important business rules
Don't extract trivial wrappers: avoid extracting 1-2 line wrappers around existing functions, standard library usage, abstractions that obscure rather than clarify intent, or code that would be clearer inline
Always useInputLocatorfor file path resolution instead of hardcoding paths
Use British English spelling in all user-facing print statements, docstrings for public APIs, and error messages shown to users
Don't modifyconfigdirectly in worker processes; use kwargs or create new instance instead
Checkconfig.multiprocessingbefore usingPoolfor parallelization
Respect scenario structure conventions: use/inputs/and/outputs/directories for file organization
Use f-strings only when the string contains variables (e.g.,f"Value: {x}"); use regular strings otherwise (e.g.,"No variables") to avoid linter warnings
Files:
cea/optimization_new/containerclasses/energyCarrier.pycea/analysis/costs/main.pycea/analysis/costs/supply_costs.py
cea/analysis/costs/**/*.py
📄 CodeRabbit inference engine (cea/analysis/costs/AGENTS.md)
cea/analysis/costs/**/*.py: Calculate Total CAPEX as: peak_kW × efficiency × CAPEX_USD2015kW
Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Files:
cea/analysis/costs/main.pycea/analysis/costs/supply_costs.py
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/HVAC_*.csv : Classify HVAC components (AHU, radiators, etc.) as distribution/emission systems with no associated costs in database
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use final energy columns (`GRID_*`, `NG_*`, `DH_*`, `DC_*`) for energy cost and emissions calculations
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use ASSEMBLIES/SUPPLY/*.csv files (not COMPONENTS) for building-level demand calculations with conservative efficiency values and single baseline costs
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use COMPONENTS/CONVERSION/*.csv files for district energy optimization with detailed equipment specifications including efficiency and capacity-dependent cost curves
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/*.py : Use end-use columns (`Q*_sys_kWh`) for HVAC system sizing and capacity calculations
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/{costs,lca}/**/*.py : Merge CAPEX data from SUPPLY assemblies with variable OPEX data from FEEDSTOCKS for a complete cost picture
Applied to files:
cea/analysis/costs/main.pycea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:06:48.123Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T23:06:48.123Z
Learning: Applies to **/cea/**/script.py : Create `main(config: Configuration)` as the entry point for new scripts in `cea/<module>/script.py`, register in `scripts.yml`, and the script auto-registers via `cea.api`
Applied to files:
cea/analysis/costs/main.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use ASSEMBLIES/SUPPLY/*.csv files (not COMPONENTS) for building-level demand calculations with conservative efficiency values and single baseline costs
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Use COMPONENTS/CONVERSION/*.csv files for district energy optimization with detailed equipment specifications including efficiency and capacity-dependent cost curves
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Applies to cea/demand/**/SUPPLY_*.csv : Classify SUPPLY components (chiller, boiler, etc.) as generation systems with CAPEX and OPEX costs in database
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the following database parameters from SUPPLY assemblies: CAPEX_USD2015kW (cost per kW), LT_yr (lifetime in years), O&M_% (annual O&M percentage), IR_% (interest rate), and efficiency (system efficiency)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:19.249Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/databases/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:19.249Z
Learning: Applies to cea/databases/**/*.py : Merge SUPPLY_ELECTRICITY.csv (infrastructure CAPEX) with COMPONENTS/FEEDSTOCKS/FEEDSTOCKS_LIBRARY/GRID.csv (hourly operational costs) using a left join on feedstock code to combine infrastructure and variable costs
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Read CAPEX parameters from SUPPLY assemblies using files matching pattern `databases/{region}/ASSEMBLIES/SUPPLY/SUPPLY_*.csv`
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:30.367Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/demand/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:30.367Z
Learning: Maintain distinction between HVAC components (affect demand via losses only, no costs) and SUPPLY components (contain all costs and emissions)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/cea/analysis/**/*.py : Use the `calc_capex_annualized(capex_total, IR_%, LT_yr)` function to convert total CAPEX to annual cost
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Annualized CAPEX using the formula: CAPEX × IR% / (1 - (1 + IR%)^-LT)
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Ensure CAPEX is annualized before comparing with annual OPEX figures
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Fixed OPEX (O&M) as: CAPEX × O&M_% / 100
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:07:11.074Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: cea/analysis/costs/AGENTS.md:0-0
Timestamp: 2025-11-26T23:07:11.074Z
Learning: Applies to cea/analysis/costs/**/*.py : Calculate Total Annualized Cost (TAC) as: Fixed OPEX + Annualized CAPEX + Variable OPEX
Applied to files:
cea/analysis/costs/supply_costs.py
📚 Learning: 2025-11-26T23:06:48.123Z
Learnt from: CR
Repo: architecture-building-systems/CityEnergyAnalyst PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T23:06:48.123Z
Learning: Applies to **/*.py : Use f-strings only when the string contains variables (e.g., `f"Value: {x}"`); use regular strings otherwise (e.g., `"No variables"`) to avoid linter warnings
Applied to files:
cea/analysis/costs/supply_costs.py
🧬 Code graph analysis (2)
cea/analysis/costs/main.py (4)
cea/inputlocator.py (3)
InputLocator(28-1456)get_baseline_costs(1370-1379)get_baseline_costs_detailed(1381-1390)cea/analysis/costs/supply_costs.py (3)
calculate_all_buildings_as_standalone(459-481)calculate_standalone_building_costs(668-766)calculate_costs_for_network_type(1515-1737)cea/analysis/costs/format_simplified.py (1)
format_output_simplified(19-202)cea/analysis/costs/solar_costs.py (2)
calculate_building_solar_costs(252-448)merge_solar_costs_to_buildings(451-513)
cea/analysis/costs/supply_costs.py (9)
cea/optimization_new/domain.py (3)
Domain(58-993)load_potentials(316-350)_initialize_energy_system_descriptor_classes(978-986)cea/optimization_new/building.py (3)
Building(27-348)distribute_building_potentials(312-348)calculate_supply_system(274-309)cea/inputlocator.py (8)
get_database_assemblies_supply_cooling(797-799)get_database_assemblies_supply_heating(805-807)get_database_assemblies_supply_hot_water(809-811)get_db4_components_conversion_conversion_technology_csv(821-823)get_building_supply(917-919)get_total_demand(1289-1291)get_demand_results_file(1297-1299)get_output_thermal_network_type_folder(995-997)cea/utilities/__init__.py (1)
unique(74-87)cea/optimization_new/containerclasses/supplySystemStructure.py (8)
SupplySystemStructure(38-1526)initialize_class_variables(1494-1526)user_component_selection(224-225)user_component_selection(228-242)available_potentials(206-207)available_potentials(210-217)build(334-667)activation_order(245-246)cea/optimization_new/containerclasses/energyFlow.py (2)
EnergyFlow(27-221)isolate_peak(214-221)cea/optimization_new/containerclasses/energyCarrier.py (1)
initialize_class_variables(143-152)cea/plots/thermal_networks/__init__.py (1)
nodes_df(286-302)cea/optimization_new/helperclasses/multiprocessing/memoryPreserver.py (1)
update(56-58)
🪛 GitHub Actions: Ruff
cea/analysis/costs/supply_costs.py
[error] 678-678: F401 [*] os imported but unused
⏰ 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 (10)
cea/analysis/costs/main.py (4)
1-23: Module structure and imports look good.The module-level organization follows CEA conventions with proper imports using
InputLocatorfor path resolution. The metadata block is complete.
22-56: Validation function is well-structured.The
validate_network_results_existfunction correctly checks for required thermal-network output files and provides clear error messages guiding users to run the prerequisite scripts.
59-79: Merge function is clear and correct.The
merge_network_type_costsfunction properly transposes the data structure from{network_type: {building: results}}to{building: {network_type: results}}.
355-366: Entry point follows CEA conventions.The
mainfunction correctly creates anInputLocatorand delegates tobaseline_costs_main. Theif __name__ == '__main__'block enables standalone execution.cea/analysis/costs/supply_costs.py (6)
1-23: Module structure and imports are appropriate.Good organization with module-level imports for commonly used dependencies (
os,pandas,Domain,Building,cea.config).
25-70: Component extraction from SUPPLY assembly is well-implemented.The function correctly handles missing files, empty codes, and parses comma-separated component lists with proper '-' placeholder handling.
73-182: Scale filtering logic correctly implements multi-level fallback.The
filter_supply_code_by_scaleand_verify_scale_matchfunctions properly handle multi-select parameters and returnNoneto trigger Level 3 fallback when scale mismatches occur. This aligns with the documented 3-level fallback approach.
225-279: DHW fallback component mapping is comprehensive.Good coverage of feedstock-to-boiler mappings. The function correctly handles both direct fuel codes (e.g., 'Cgas') and feedstock names (e.g., 'NATURALGAS').
1740-1868: Cost extraction logic is comprehensive.The
extract_costs_from_supply_systemfunction properly separates component costs (CAPEX + fixed OPEX) from energy carrier costs (variable OPEX) and calculates TAC correctly. The service mapping handles both network and standalone scenarios.
1981-2047: Energy carrier mapping is well-documented.Good coverage of energy carrier codes with clear comments indicating the source database structure. The heating/cooling carrier classification is logical.
| if os.path.exists(nodes_file): | ||
| nodes_df = gpd.read_file(nodes_file) | ||
| network_buildings = nodes_df[nodes_df['type'] == 'CONSUMER']['building'].unique().tolist() | ||
| return set([b for b in network_buildings if b and b != 'NONE']) | ||
| except Exception: | ||
| pass | ||
|
|
||
| return set() |
There was a problem hiding this comment.
Silent exception handling masks errors.
The bare except Exception: pass swallows all errors without logging, making debugging difficult when network file reading fails unexpectedly.
try:
network_folder = locator.get_output_thermal_network_type_folder(network_type, network_name)
layout_folder = os.path.join(network_folder, 'layout')
nodes_file = os.path.join(layout_folder, 'nodes.shp')
if os.path.exists(nodes_file):
nodes_df = gpd.read_file(nodes_file)
network_buildings = nodes_df[nodes_df['type'] == 'CONSUMER']['building'].unique().tolist()
return set([b for b in network_buildings if b and b != 'NONE'])
- except Exception:
- pass
+ except Exception as e:
+ # Log warning but don't fail - caller handles empty set
+ import warnings
+ warnings.warn(f"Failed to read network buildings for {network_type}/{network_name}: {e}")
return set()📝 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.
| if os.path.exists(nodes_file): | |
| nodes_df = gpd.read_file(nodes_file) | |
| network_buildings = nodes_df[nodes_df['type'] == 'CONSUMER']['building'].unique().tolist() | |
| return set([b for b in network_buildings if b and b != 'NONE']) | |
| except Exception: | |
| pass | |
| return set() | |
| if os.path.exists(nodes_file): | |
| nodes_df = gpd.read_file(nodes_file) | |
| network_buildings = nodes_df[nodes_df['type'] == 'CONSUMER']['building'].unique().tolist() | |
| return set([b for b in network_buildings if b and b != 'NONE']) | |
| except Exception as e: | |
| # Log warning but don't fail - caller handles empty set | |
| import warnings | |
| warnings.warn(f"Failed to read network buildings for {network_type}/{network_name}: {e}") | |
| return set() |
🤖 Prompt for AI Agents
In cea/analysis/costs/supply_costs.py around lines 584 to 591, replace the bare
"except Exception: pass" that swallows all errors when reading nodes_file with
explicit error handling: catch the exception as e and log it (e.g.,
logger.exception or logging.exception) including the filename and context, and
then return the empty set as current behavior; optionally narrow the exception
to file/IO/geopandas-specific errors if appropriate. Ensure the log message
contains the nodes_file path and the exception details to aid debugging.
| # Write back modified supply.csv temporarily | ||
| if fallback_count > 0: | ||
| print(f" Applied config fallbacks for {fallback_count} building(s) with scale mismatch") | ||
| supply_df.to_csv(supply_csv_path, index=False) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n cea/analysis/costs/supply_costs.py | sed -n '800,835p'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1891
🏁 Script executed:
rg -n "supply_csv_path\s*=" cea/analysis/costs/supply_costs.py -A 2Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 349
🏁 Script executed:
rg -n "def get_building_supply" cea/analysis/costs/ -A 10Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 73
🏁 Script executed:
fd -type f -name "*.py" | xargs rg -l "class InputLocator" | head -5Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 298
🏁 Script executed:
rg -n "def get_building_supply" cea/ -A 5Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 518
🏁 Script executed:
rg -n "def get_building_properties_folder" cea/inputlocator.py -A 2Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 275
🏁 Script executed:
rg -n "apply_supply_code_fallback_for_standalone" cea/analysis/costs/supply_costs.py -B 5 -A 20 | head -60Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2437
🏁 Script executed:
cat -n cea/analysis/costs/supply_costs.py | sed -n '770,820p'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2269
🏁 Script executed:
rg -n "apply_level_1_fallback_config" cea/ -B 2 -A 2Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 73
🏁 Script executed:
rg -n "supply.csv" cea/analysis/costs/supply_costs.py | head -20Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 983
🏁 Script executed:
git log --oneline cea/analysis/costs/supply_costs.py | head -10Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 168
🏁 Script executed:
cat -n cea/analysis/costs/supply_costs.py | sed -n '765,775p'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 539
🏁 Script executed:
rg -n "def.*fallback" cea/analysis/costs/supply_costs.pyRepository: architecture-building-systems/CityEnergyAnalyst
Length of output: 479
🏁 Script executed:
rg -n "supply_df\s*=" cea/analysis/costs/supply_costs.py -B 2 -A 5 | head -50Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 594
🏁 Script executed:
rg -n "apply_config_fallbacks_for_service_needs" cea/ -B 3 -A 3Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1402
🏁 Script executed:
cat -n cea/analysis/costs/supply_costs.py | sed -n '821,860p'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 2060
🏁 Script executed:
rg -n "get_building_supply\|supply.csv" cea/analysis/costs/supply_costs.py | grep -A 2 "calculate_heating_systems"Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 73
🏁 Script executed:
cat -n cea/analysis/costs/supply_costs.py | sed -n '821,850p'Repository: architecture-building-systems/CityEnergyAnalyst
Length of output: 1725
Remove file modification; pass corrected supply data to downstream functions instead.
The code modifies scenario/inputs/building-properties/supply.csv at line 818, which violates the scenario structure convention that inputs should be read-only. While the fallbacks are applied intentionally so that calculate_heating_systems reads the corrected supply codes, persisting these changes to the input file creates reproducibility and concurrency issues.
Instead, collect the fallback corrections in a dict or DataFrame and pass them to calculate_heating_systems (or the Domain/Building classes it uses) as an optional parameter to override input values. Alternatively, write the corrected data to a temporary location in the outputs directory for this calculation only.
| # Network._calculate_piping_cost uses: annualised = total / network_lifetime_yrs | ||
| network_lifetime_yrs = network.configuration_defaults.get('network_lifetime_yrs', 20.0) | ||
| piping_cost_total = piping_cost_annual * network_lifetime_yrs |
There was a problem hiding this comment.
Same annualization issue in calculate_network_costs.
The piping cost calculation here also uses simple division instead of the proper annualization formula.
# Calculate total CAPEX from annualized cost
- # Network._calculate_piping_cost uses: annualised = total / network_lifetime_yrs
- network_lifetime_yrs = network.configuration_defaults.get('network_lifetime_yrs', 20.0)
- piping_cost_total = piping_cost_annual * network_lifetime_yrs
+ # Reverse the annualization to get total CAPEX
+ # Note: If Network uses simple division, this calculation is consistent but both should
+ # ideally use proper annualization. For now, maintain consistency with Network class.
+ network_lifetime_yrs = network.configuration_defaults.get('network_lifetime_yrs', 20.0)
+ piping_cost_total = piping_cost_annual * network_lifetime_yrsIf the Network class uses simple division, consider updating both to use the proper formula for consistency with the cost calculation guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cea/analysis/costs/supply_costs.py around lines 1494-1496 the code converts
piping annual cost to a total cost by multiplying by lifetime (piping_cost_total
= piping_cost_annual * network_lifetime_yrs), which uses simple
division/aggregation instead of the proper annuity annualisation; compute the
annuity factor from the network discount rate and lifetime (discount_rate =
network.configuration_defaults.get('discount_rate', <default>), n =
network.configuration_defaults.get('network_lifetime_yrs', 20.0)), then convert
annual to total with total = annual / annuity_factor where annuity_factor = r /
(1 - (1 + r) ** -n); replace the multiply with this calculation and ensure you
use the same keys/defaults as other cost functions for consistency.
Summary by CodeRabbit
New Features
Configuration
Visualization
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.