update building radiation utils for external radiative heat transfer#170
update building radiation utils for external radiative heat transfer#170ecosang wants to merge 1 commit into
Conversation
|
@s2t2 |
|
Nice @ecosang , thank you! There is a lot of code - I will work on reviewing by later this week / early next week. |
|
@gemini-code-assist please review this pull request |
|
@gemini-code-assist are there any opportunities to refactor the function-oriented code in "smart_control/simulator/building_radiation_utils.py" to use class and be object oriented? |
s2t2
left a comment
s2t2
left a comment
There was a problem hiding this comment.
@ecosang thanks for the PR!
Overall this looks really great.
I just had some minor comments about type hints, and refactoring a variable, otherwise LGTM.
Also I made some separate comments to test the capabilities of the Gemini code assistant - I'm still learning how to best incorporate it.
| from collections import deque | ||
| import math | ||
| from typing import Optional, Tuple | ||
| from typing import Any, Dict, List, Mapping, Optional, Sequence, Set, Tuple, Union |
There was a problem hiding this comment.
The Google Python style guide recommends prefers using X | None style instead of typing.Optional.
They also prefer using normal list and dict instead of typing.List and typing.Dict.
| solar_radiation.IrradianceComponents, | ||
| Mapping[str, Any], | ||
| ], | ||
| solar_zenith: Optional[float] = None, |
There was a problem hiding this comment.
let's use float | None = None (here and in related places below / throughout)
|
|
||
|
|
||
| def _ensure_irradiance_components( | ||
| irradiance_components: Union[ |
There was a problem hiding this comment.
Google style guide prefers This | That instead of Union[This, That]
| if col == cols - 1: | ||
| return (0, -1) | ||
|
|
||
| directions = [(-1, 0), (1, 0), (0, -1), (0, 1)] |
There was a problem hiding this comment.
I see this directions variable duplicated in a few places - let's move to a shared constant DIRECTIONS at the top of the file, and refactor all references
| - 180° = South (down/increasing row) | ||
| - 270° = West (left/decreasing col) | ||
|
|
||
| The computed azimuth is then offset by floor_plan_orientation. |
| -43, | ||
| -43, | ||
| -1, | ||
| ], # Interior fenestration (adj. to air) |
There was a problem hiding this comment.
FYI if you find it easier to read this list of numbers if they are on the same line, and to be consistent with the other lines, feel free to use #pylint:disable=line-too-long whenever you feel it is helpful.
There was a problem hiding this comment.
I appreciate the detailed, thorough, and well documented tests!
2 participants
Adding several external radiative heat transfer-related utils.
New functions in
building_radiation_utils.py:_ensure_irradiance_components()— Normalizes irradiance input (dict or dataclass) to a guaranteedIrradianceComponentsinstance with validation of required keys (ghi, dni, dhi, solar_zenith, solar_azimuth)validate_fenestration_connectivity()— Validates that fenestration nodes in a floor plan properly bridge exterior space to interior air (checks air adjacency, exterior exposure, surrounded-by-air detection, and chain connectivity)_validate_fenestration_chain_connectivity()— Ensures each fenestration node can reach both exterior and interior air through the fenestration chain via BFS, and validates interior-facing direction is not blocked_determine_interior_direction()— Infers the direction vector pointing from exterior toward interior air based on node positions relative to grid boundaries or adjacent exterior space_find_connected_groups()— Finds all 4-connected groups of cells with a given value in the floor plan using BFSmark_fenestration_positions()— Classifies fenestration nodes into exterior, interior, or in-between categories based on adjacencygroup_fenestrations()— Groups adjacent fenestration nodes and computes surface properties (azimuth, tilt, view factors F_sky/F_gnd/F_air, beta)_determine_fenestration_azimuth()— Determines fenestration azimuth from exterior adjacency using atan2, with floor_plan_orientation offset supportgroup_air_nodes()— Groups connected interior air nodes and annotates which fenestration groups are adjacent to each air groupcalculate_solar_absorbed_for_fenestration_group()— Computes absorbed solar flux per node for a fenestration group using POA irradiancenet_solar_absorbed_heatflux_fenestration()— Returns array of absorbed solar heat flux at each fenestration node across all groupscalculate_solar_transmitted_for_fenestration_group()— Computes total transmitted solar radiation for a fenestration groupnet_solar_transmitted_heatflux_fenestration()— Distributes transmitted solar heat flux to air nodes connected to windowsmark_interior_surface_adjacent_to_air()— Marks interior surfaces (walls + fenestration) adjacent to air with boolean maskcalculate_exterior_lwr_for_fenestration_group()— Computes net exterior longwave radiative heat flux for a fenestration group (sky + ground + air exchange)net_exterior_radiative_heatflux()— Computes net exterior LWR heat flux array for all fenestration nodesget_exterior_wall_boundary_mask()— Identifies exterior wall nodes forming the building boundary (excluding walls facing enclosed interior air)determine_exterior_wall_azimuth_array()— Determines azimuth for each exterior wall boundary node based on adjacent exterior space directionNew tests in
building_radiation_utils_test.py:_validate_fenestration_chain_connectivity()— Testing valid chains, blocked-from-exterior, blocked-from-air, interior-direction blockage, thick fenestration, and L-shaped fenestrationCode cleanup:
interior_wall_valueparameter from_validate_fenestration_chain_connectivity()and all callers