Skip to content

Conversation

@reyery
Copy link
Member

@reyery reyery commented Oct 21, 2025

Add guards for division by zero and checks for physical limitations.

@reyery reyery marked this pull request as ready for review October 23, 2025 02:53
@reyery reyery requested a review from Copilot October 23, 2025 02:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds validation checks for division by zero and physical limitations in thermal network demand calculations. The changes prevent runtime errors and ensure physically valid configurations by validating temperature differences, pipe dimensions, and thermodynamic constraints before performing calculations.

Key Changes:

  • Added validation for division by zero in temperature, flow rate, and heat transfer calculations
  • Introduced new constants for minimum temperature differences and convergence tolerances
  • Enhanced error messages to provide detailed diagnostic information

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
cea/constants.py Adds new constants for minimum temperature differences and convergence tolerances
cea/technologies/thermal_network/thermal_network_loss.py Adds division by zero check for pipe outlet temperature calculation
cea/technologies/thermal_network/thermal_network.py Adds validation for Reynolds number, Darcy friction factor, and pipe dimension calculations
cea/technologies/thermal_network/substation_matrix.py Adds validation for heat exchanger effectiveness iteration and temperature differences
cea/technologies/thermal_network/simplified_thermal_network.py Adds validation for pipe diameter and thermal loss coefficient calculations
cea/technologies/substation.py Adds temperature difference validation for heat capacity and heat exchanger calculations
cea/technologies/pumps.py Adds validation for pump efficiency and water density constants
cea/technologies/heatpumps.py Adds temperature validation for heat pump COP calculations across multiple heat pump types
cea/technologies/chiller_vapor_compression.py Adds temperature validation and Carnot limit checks for chiller COP calculations
cea/demand/rc_model_SIA.py Adds validation for building area and heat transfer coefficients in RC model
cea/demand/electrical_loads.py Adds temperature difference validation for pump auxiliary energy calculations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

cea/constants.py Outdated
# Temperature difference tolerances for numerical stability
# Used to avoid division by near-zero in heat transfer and mass flow calculations
MIN_TEMP_DIFF_FOR_MASS_FLOW_K = 0.001 # Minimum temperature difference for mass flow calculation [K or °C]
# Physical basis: Below 1 millikelvin, water flow becomes negligible and pump energy ≈ 0
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states 'Below 1 millikelvin' but the constant value is 0.001 K, which equals 1 millikelvin. The comment should say 'Below or equal to 1 millikelvin' or 'At 1 millikelvin or below' for accuracy.

Suggested change
# Physical basis: Below 1 millikelvin, water flow becomes negligible and pump energy ≈ 0
# Physical basis: At 1 millikelvin or below, water flow becomes negligible and pump energy ≈ 0

Copilot uses AI. Check for mistakes.


from math import log, ceil
import pandas as pd
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import statement is missing the numpy module which was removed from line 14 but is still used in the file (e.g., np.isclose at line 80). The numpy import should be retained.

Suggested change
import pandas as pd
import pandas as pd
import numpy as np

Copilot uses AI. Check for mistakes.
def efficiencies_not_converged(previous_efficiency, current_efficiency):
tolerance = 0.00000001
# Convergence tolerance for heat exchanger efficiency iteration
tolerance = 1e-08 # Same as HEAT_EXCHANGER_EFFICIENCY_CONVERGENCE_TOL
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment references 'HEAT_EXCHANGER_EFFICIENCY_CONVERGENCE_TOL' but this constant is not imported or defined in this file or in constants.py. Either this constant should be imported and used instead of the hardcoded value, or the comment should be updated to remove the reference.

Suggested change
tolerance = 1e-08 # Same as HEAT_EXCHANGER_EFFICIENCY_CONVERGENCE_TOL
tolerance = 1e-08 # Convergence tolerance value

Copilot uses AI. Check for mistakes.
Comment on lines 756 to 757
# Check if primary side has meaningful temperature drop (thi != tho)
# If temperatures are essentially equal, no heat transfer occurs, skip iteration
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment spans lines 756-757 but line 757 is not a continuation of the same logical statement needing a fix. The comment should be consolidated to a single line or properly structured as a multi-line comment block.

Suggested change
# Check if primary side has meaningful temperature drop (thi != tho)
# If temperatures are essentially equal, no heat transfer occurs, skip iteration
# Check if primary side has meaningful temperature drop (thi != tho); if temperatures are essentially equal, no heat transfer occurs, skip iteration

Copilot uses AI. Check for mistakes.
Comment on lines 786 to 788
# Check if secondary side has meaningful temperature rise (tci != tco)
# Even if primary side had temperature drop, heat exchanger may produce negligible secondary-side change
# This validates the RESULT of the iteration, while the check above validates the INPUT
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This multi-line comment (lines 786-788) describes validation logic but could be more concise. Consider consolidating to: 'Validate secondary side temperature rise from iteration results (unlike primary side input validation above).'

Suggested change
# Check if secondary side has meaningful temperature rise (tci != tco)
# Even if primary side had temperature drop, heat exchanger may produce negligible secondary-side change
# This validates the RESULT of the iteration, while the check above validates the INPUT
# Validate secondary side temperature rise from iteration results (unlike primary side input validation above).

Copilot uses AI. Check for mistakes.
Comment on lines 879 to 880
# Handle case where cc_kWperK is zero (no heat capacity flow)
# This can occur when temperature difference between supply and return is negligible
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on lines 879-880 describes handling zero cc_kWperK but the actual check on line 881 also checks for Q_heating_W == 0.0. The comment should mention both conditions being checked.

Suggested change
# Handle case where cc_kWperK is zero (no heat capacity flow)
# This can occur when temperature difference between supply and return is negligible
# Handle cases where either cc_kWperK is zero (no heat capacity flow)
# or Q_heating_W is zero (no heating load). This can occur when temperature difference between supply and return is negligible or there is no demand.

Copilot uses AI. Check for mistakes.
reyery and others added 26 commits October 27, 2025 16:15
Replaced direct requests.post calls with post_with_retry in post_started, post_success, and post_error to improve reliability of job status updates to the server.
Added a filter to ignore UserWarning messages during job execution to reduce unnecessary warning output.
Changed the type annotation for CEAServerSettings from dict to Settings to reflect the actual return type of get_settings.
Introduced the put_with_retry function to perform HTTP PUT requests with configurable retry logic and exponential backoff. This improves robustness when communicating with unreliable endpoints by handling transient failures automatically.
Replaces direct requests.put call with put_with_retry in stream_poster to improve reliability when posting stream data.
Commented out the strict check for missing terminal nodes in the Steiner tree preparation to allow optimization on partial graphs. A warning is now printed instead, and the process continues even if some terminal nodes are missing.
Enhanced the GET /jobs endpoint to support filtering by job state, excluding deleted jobs by default, and limiting the number of returned jobs. Results are now ordered by creation time in descending order, improving usability for clients retrieving recent jobs.
Changed the delete_job endpoint to perform a soft delete by setting the job state to DELETED and updating the end_time, instead of removing the job row from the database. This preserves job records while marking them as deleted.
Replaces direct process deletion with a new cleanup_worker_process function that handles both graceful and forceful termination of worker processes, depending on job outcome (success, error, or cancellation). Ensures robust cleanup, improved logging, and consistent removal from worker_processes tracking.
The cancel_job endpoint now saves any remaining stream output to the job's stdout before clearing streams. This ensures that all output generated up to the point of cancellation is preserved.
Introduces separate TTL settings for stream cache, worker process tracking, and a default cache TTL to improve cache management and cleanup behavior.
Introduces default TTL (time-to-live) support for AsyncDictCache and updates cache provider functions to accept and propagate TTL values. This allows cache entries for worker processes and streams to expire automatically based on configurable settings, improving cache management and resource utilization.
…hopper-bracket-issue-on-z-coordinates

Fixing Grasshopper import/export issue where Z-coordinates get extra brackets after a round-trip.
Changed the async session factory to use expire_on_commit=True, ensuring that objects are expired after commit for data freshness and to prevent memory leaks. Updated the docstring to explain the rationale and implications of this change.
Replaces ZIP_DEFLATED with ZIP_STORED when creating scenario zip files, resulting in no compression. This may improve performance at the cost of larger file sizes.
Ensure the 'name' column exists before performing timeline aggregation in aggregate_or_combine_dataframes. Also, change building name concatenation to use commas instead of no separator.
Temporarily disables a failing assertion in TestGraphCorrector until the related function is fixed. The test still checks that the error message contains 'not connected'.
Added @unittest.skip to test_validate_steiner_tree_ready_missing_terminals in TestGraphCorrector to disable it until the related function is fixed.
…ion-timline

Fixing emission timeline to handle buildings with different construction years
Introduces a function to extract module-level constants from cea.config and includes them in the generated type stub file. This enhances the stub with type hints for constants, improving type safety and code completion.
ShiZhongming and others added 30 commits November 10, 2025 18:48
… works as intended without any changes needed
The simplify_street_network_geometric function now accepts a coord_precision parameter to control the decimal precision for coordinate rounding, improving flexibility for different use cases. The docstring has been updated to reflect this new parameter.
The calc_connectivity_network_with_geometry function signature was updated to always return a GeoDataFrame, removing the return_graph parameter and simplifying the return type. The docstring was updated accordingly to reflect this change.
Added a check in gdf_to_nx to skip processing rows where the geometry is None or empty, preventing potential errors during network graph construction.
The snap_endpoints_to_nearby_lines function now accepts a split_lines parameter to control whether lines are split at snapped endpoints or just snapped without splitting. This provides more flexibility for cleaning street network geometries, allowing users to choose between snapping only or also creating explicit T-junctions.
Refactored the connectivity_potential module to return a NetworkX graph with preserved geometries and building terminal metadata instead of a GeoDataFrame. Updated main.py and tests to use the new graph-based interface, and simplified/streamlined the connectivity pipeline for improved precision and maintainability. Documentation and comments were updated to reflect the new workflow and guarantees.
Adds detailed criteria for when to create AGENTS.md files, writing style recommendations, and example structures. Clarifies update procedures and provides directory-specific examples to improve consistency and maintainability of documentation.
Updated AGENTS.md files in analysis/costs, databases, demand, and dashboard to provide clearer, more concise technical explanations and quick references for CEA cost, database, and demand calculation logic. Added new DATABASE_GUIDE.md and DEMAND_GUIDE.md files with in-depth, human-readable documentation on database structure, file relationships, and demand calculation methodology. Added placeholder AGENTS.md for network_layout. These changes improve developer onboarding and clarify the separation of HVAC/SUPPLY, end-use vs final energy, and database abstraction levels.
Streamlined the AGENTS.md file for clarity and brevity, focusing on core architecture, setup instructions, and key development patterns. Removed redundant details, merged sections, and emphasized essential workflows and best practices for contributors.
Added a new best practice to remind developers to regenerate `config.pyi` using the config type generator script after modifying `config.py`.
Replaces all references to the 'pipe length' column in edge_df with 'length_m' for consistency and clarity throughout the thermal_network.py module.
Update tests in test_network_layout_integration.py to use NetworkX graph objects instead of GeoDataFrames, reflecting changes in the connectivity network API. Adjust assertions and round-trip conversion logic accordingly. Also, update the test for disconnected components to expect an error instead of automatic connection. Fix a minor comment typo in test_graph_utils.py.
…assive_components_to_draw_from_environment_and_grids

Install passive components to draw from environment and grids
Updated comments in substation.py to improve clarity regarding convergence tolerance, temperature difference checks, and handling of zero heat capacity flow or heating load in heat exchanger calculations. No functional changes were made.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants