Fixing supply systems graphics #3927
Conversation
Fixed a few small errors in the supply systems graphics script. These errors were primarily introduced with the changes in folder naming introduced with the "retain-run-results" switch.
|
Caution Review failedThe pull request is closed. Walkthroughmain(config) now creates and wires an InputLocator, sets the latest optimization run, and produces Plotly figures per energy/supply-system (optionally combined into one interactive figure). SupplySystemGraphInfo and create_supply_system_graph accept a locator; new image entries and a classmethod exclusion in memoryPreserver were added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User / CLI
participant M as main(config)
participant L as InputLocator
participant FS as File System / Outputs
participant G as create_supply_system_graph
participant C as create_combined_figure_with_dropdowns
U->>M: invoke main(config)
M->>L: create locator from config
M->>FS: find latest optimization run
FS-->>M: latest run id (if any)
M->>L: set locator.optimization_run
loop per energy system / supply system
M->>G: create_supply_system_graph(energy_id, supply_id, locator)
G->>FS: load run-specific data and icons via locator
FS-->>G: data & icon paths (use `unknown` fallback if missing)
G-->>M: return Plotly figure
end
alt multiple figures
M->>C: create_combined_figure_with_dropdowns(figures_dict,...)
C-->>M: combined interactive figure
end
M-->>U: return figures (or combined figure HTML)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
cea/plots/optimization_new/images/component_img/furnace_icon.pngis excluded by!**/*.pngcea/plots/optimization_new/images/component_img/unknownComponent_icon.pngis excluded by!**/*.png
📒 Files selected for processing (2)
cea/plots/optimization_new/b_supply_system.py(7 hunks)cea/plots/optimization_new/images/image_lib.yml(1 hunks)
⏰ 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 (4)
cea/plots/optimization_new/b_supply_system.py (3)
26-26: LGTM! Lazy initialization of locator.The change to initialize
_locatortoNoneand set it at runtime inmain()appropriately addresses the folder naming changes introduced with the "retain-run-results" feature.
262-262: LGTM! Robust fallback for component colors.The nested
.get()calls provide a robust fallback chain: component-specific color → unknown component color → grey. This prevents KeyError exceptions for missing component definitions.
370-370: Good practice: using raw string for regex.The change to a raw string (
r"N\d{4}") is a best practice for regex patterns, improving code readability even though it's functionally equivalent in this specific case.cea/plots/optimization_new/images/image_lib.yml (1)
63-71: Approve icon references: Bothfurnace_icon.pngandunknownComponent_icon.pngexist incea/plots/optimization_new/images/component_img, so the new entries are good to merge.
Moves the locator from a class-level variable to an instance variable in SupplySystemGraphInfo, and introduces a global APP_CONFIG dictionary to manage configuration and state. Updates related code to use APP_CONFIG for passing locator and system IDs, improving clarity and reducing reliance on class variables.
Eliminated the Dash web app and related callbacks, replacing interactive UI with automated generation and saving of supply system plots as HTML files for all energy and supply systems. The script now prints progress and displays the first generated figure in a browser for quick review.
Introduces a new function to generate a single Plotly figure with dropdown menus for selecting different energy and supply systems. The script now saves and optionally displays this combined interactive figure, improving usability and visualization of multiple supply system scenarios.
The main function now takes a config parameter instead of creating it internally.
Introduces a new script entry 'plot-supply-system' for plotting components of energy supply systems. The script is available via CLI and dashboard interfaces and uses the supply system summary file as input.
Introduces a new script for visualizing supply systems by wrapping the optimization plot function and returning the result as HTML. This script can be run standalone and is intended for integration with the CEA visualization tools.
The code that saves the combined supply systems figure as an HTML file has been removed. Now, the function only returns the combined figure object without writing it to disk.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
cea/plots/optimization_new/b_supply_system.py (2)
664-666: Add error handling for missing icon files.The fallback logic has two potential failure points previously identified:
- If the
"unknown"key is missing fromComponentGraphInfo.image_paths,.get()returnsNone, causingImage.open(None)to raise aTypeError.- If the icon file doesn't exist on disk,
Image.open()raises aFileNotFoundError.Apply this diff to add robust error handling:
# Use the unknown component icon as fallback for missing component images icon_path = ComponentGraphInfo.image_paths.get(code, ComponentGraphInfo.image_paths.get("unknown")) +if icon_path is None: + raise ValueError(f"No icon path found for component '{code}' and 'unknown' fallback is not defined") +if not os.path.exists(icon_path): + raise FileNotFoundError(f"Icon file not found: {icon_path}") icon = Image.open(icon_path)
560-566: Add robust error handling for optimization run detection.The logic to detect the latest optimization run has several critical issues that were previously flagged:
- Fragile parsing:
f.split('_')[-1]assumes folder names strictly followcentralized_run_N. Non-standard names would cause incorrect extraction.- Missing error handling:
int()conversion could raiseValueErroron unexpected folder names.- No existence check: The code doesn't verify
optimization_pathexists before listing.- Silent failure: If no folders are found, execution continues without setting
locator.optimization_run, which could cause downstream failures.Apply this diff to add robust error handling:
+import re + def main(config): locator = cea.inputlocator.InputLocator(config.scenario) # Find and use the latest optimization run - optimization_path = locator.get_optimization_results_folder() - if os.path.exists(optimization_path): - run_folders = [f for f in os.listdir(optimization_path) if f.startswith('centralized_run_')] - if run_folders: - latest_run_num = max([int(f.split('_')[-1]) for f in run_folders]) + optimization_path = locator.get_optimization_results_folder() + if not os.path.exists(optimization_path): + raise FileNotFoundError(f"Optimization path does not exist: {optimization_path}") + + run_folders = [] + for f in os.listdir(optimization_path): + match = re.match(r'^centralized_run_(\d+)$', f) + if match: + run_folders.append(int(match.group(1))) + + if run_folders: + latest_run_num = max(run_folders) - locator.optimization_run = latest_run_num - print(f"Using latest optimization run: centralized_run_{latest_run_num}") + locator.optimization_run = latest_run_num + print(f"Using latest optimization run: centralized_run_{latest_run_num}") + else: + raise FileNotFoundError(f"No valid optimization runs found in {optimization_path}")
🧹 Nitpick comments (4)
cea/visualisation/special/supply_system.py (1)
14-17: Add error handling and document return type.The function lacks error handling for
supply_system_main(config), which could raise exceptions during figure generation. Additionally, the return type switches between HTML string and empty string without documentation.Apply this diff to add error handling:
def main(config): - fig = supply_system_main(config) - - return fig.to_html() if fig is not None else "" + try: + fig = supply_system_main(config) + return fig.to_html() if fig is not None else "" + except Exception as e: + print(f"Error generating supply system plot: {e}") + return ""Also consider adding a docstring to document the return type:
def main(config): """ Generate supply system visualization. Args: config: CEA Configuration object Returns: str: HTML representation of the figure, or empty string if generation fails """cea/plots/optimization_new/b_supply_system.py (3)
604-620: Add error handling for individual figure generation.The loop generates figures for all energy and supply systems but lacks error handling. If
create_supply_system_graphfails for one system, the entire generation process stops.Apply this diff to add error handling:
for energy_system_id in des_solution_folders: supply_systems = des_supply_systems_dict[energy_system_id] print(f"\nEnergy System: {energy_system_id}") print(f" Supply systems: {len(supply_systems)}") for supply_system_id in supply_systems: print(f" Generating graphic for {supply_system_id}...") - fig = create_supply_system_graph(energy_system_id, supply_system_id, locator) - - # Store the figure - key = f"{energy_system_id}_{supply_system_id}" - figures[key] = fig + try: + fig = create_supply_system_graph(energy_system_id, supply_system_id, locator) + key = f"{energy_system_id}_{supply_system_id}" + figures[key] = fig + except Exception as e: + print(f" Error generating graphic for {supply_system_id}: {e}") + continue
599-602: Validate optimization results exist.While the code handles empty
des_solution_foldersgracefully through the for loop, it would be clearer to explicitly check and provide a user-friendly message when no optimization results are found.Apply this diff to add an explicit check:
# Load the supply system data des_supply_systems_dict = {} des_solution_folders = locator.get_new_optimization_des_solution_folders() + + if not des_solution_folders: + print("No optimization solution folders found. Please run the optimization first.") + return None + for district_energy_system in des_solution_folders: supply_systems = locator.get_new_optimization_optimal_supply_system_ids(district_energy_system) des_supply_systems_dict[district_energy_system] = supply_systems
700-716: Add error handling for source/sink/consumer icons.Similar to component icons, the code should handle cases where source, sink, or consumer icon files don't exist.
Apply this diff to add error handling:
# Add sink and source images and tooltips for sinks_or_sources in supply_system.sinks_and_sources.values(): for code, sink_or_source in sinks_or_sources.items(): + icon_path = SourceAndSinkGraphInfo.image_paths.get(code) + if icon_path is None or not os.path.exists(icon_path): + print(f"Warning: Icon not found for {code}, skipping") + continue - icon = Image.open(SourceAndSinkGraphInfo.image_paths[code]) + icon = Image.open(icon_path) fig.add_layout_image( source=icon, xref="x", yref="y", x=sink_or_source.position[0], y=sink_or_source.position[1], xanchor="center", yanchor="middle", sizex=sink_or_source.size[0], sizey=sink_or_source.size[1], ) # Add consumer image + consumer_icon_path = ConsumerGraphInfo.image_paths.get(supply_system.consumer.category) + if consumer_icon_path is None or not os.path.exists(consumer_icon_path): + print(f"Warning: Consumer icon not found for {supply_system.consumer.category}") + return fig - icon = Image.open(ConsumerGraphInfo.image_paths[supply_system.consumer.category]) + icon = Image.open(consumer_icon_path) fig.add_layout_image( source=icon,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cea/plots/optimization_new/b_supply_system.py(8 hunks)cea/scripts.yml(1 hunks)cea/visualisation/special/supply_system.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
cea/visualisation/special/supply_system.py (1)
cea/config.py (1)
Configuration(35-277)
cea/plots/optimization_new/b_supply_system.py (2)
cea/inputlocator.py (3)
get_new_optimization_optimal_supply_system_file(580-584)InputLocator(28-1500)get_optimization_results_folder(306-308)cea/plots/optimization_new/a_pareto_front.py (1)
main(457-506)
⏰ 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 (3)
cea/plots/optimization_new/b_supply_system.py (3)
23-26: Good refactor for dependency injection.Accepting the locator as a constructor parameter and storing it as an instance variable improves testability and removes the need for a class-level locator. This is a solid architectural improvement.
783-889: Well-structured combined figure implementation.The
create_combined_figure_with_dropdownsfunction effectively combines multiple figures into a single interactive visualization with dropdown menus. The visibility logic for traces, shapes, and images is properly managed.
364-364: Verify supply_system_id naming patternThe script returned no
_supply_system_structure.csvfiles—please confirm that yoursupply_system_idvalues actually follow^N\d{4}$, or adjust the regex to match your district IDs.
Raises a ValueError if no icon path is found for a component and a FileNotFoundError if the icon file does not exist. This improves robustness by ensuring missing or misconfigured icons are reported clearly.
3 participants
Fixed a few small errors in the supply systems graphics script. These errors were primarily introduced with the changes in folder naming introduced with the "retain-run-results" switch.
@reyery I think it'd probably be best to create a new branch to make these flow-charts available in the front-end.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes