Improve test coverage for ConvexWeilerAtherton polygon clipping algorithm#11554
Open
joseph-robertson wants to merge 7 commits intodevelopfrom
Open
Improve test coverage for ConvexWeilerAtherton polygon clipping algorithm#11554joseph-robertson wants to merge 7 commits intodevelopfrom
joseph-robertson wants to merge 7 commits intodevelopfrom
Conversation
Comment on lines
+203
to
+208
| ShadowCalculation, | ||
| PolygonClipping, !- Shading Calculation Method | ||
| Periodic, !- Shading Calculation Update Frequency Method | ||
| 1, !- Shading Calculation Update Frequency | ||
| , !- Maximum Figures in Shadow Overlap Calculations | ||
| ConvexWeilerAtherton; !- Polygon Clipping Algorithm |
Collaborator
Author
There was a problem hiding this comment.
This is the new stuff.
There was a problem hiding this comment.
Pull request overview
Adds a new EnergyPlus regression/simulation test IDF that exercises the ConvexWeilerAtherton polygon clipping path in ShadowCalculation, addressing the existing coverage gap noted in #5558.
Changes:
- Added a new shading-focused IDF test case configured to use
PolygonClipping+ConvexWeilerAtherton. - Registered the new IDF in the
testfilesCMake simulation test list. - Fixed/cleaned up a documentation line for the “Surface Outside Face Sunlit Fraction” output variable units.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| testfiles/SolarShadingTest_ConvexWeilerAtherton.idf | New test IDF to run shading with ConvexWeilerAtherton polygon clipping algorithm. |
| testfiles/CMakeLists.txt | Adds the new IDF to the simulation test suite. |
| doc/input-output-reference/src/overview/group-thermal-zone-description-geometry.tex | Adjusts the units text for the “Sunlit Fraction” output variable entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ! Design Days: PHOENIX_AZ_USA Annual Heating 99.6%, MaxDB=2.9°C (Winter design day has solar) | ||
| ! PHOENIX_AZ_USA Annual Cooling (DB=>MWB) .4%, MaxDB=43.4°C MWB=21.1°C | ||
| ! | ||
| ! Run Period (Weather File): 1/1 through 12/31, PHOENIX_AZ_USA TMY2 |
There was a problem hiding this comment.
The header comment says the run period uses a TMY2 weather file, but this test is wired up in CMake to use a TMY3 EPW. Please update the comment to match the actual EPW being used (or change the EPW if TMY2 is required).
Suggested change
| ! Run Period (Weather File): 1/1 through 12/31, PHOENIX_AZ_USA TMY2 | |
| ! Run Period (Weather File): 1/1 through 12/31, PHOENIX_AZ_USA TMY3 |
Comment on lines
+112
to
+125
| RunPeriod, | ||
| Run Period 1, !- Name | ||
| 1, !- Begin Month | ||
| 1, !- Begin Day of Month | ||
| , !- Begin Year | ||
| 12, !- End Month | ||
| 31, !- End Day of Month | ||
| , !- End Year | ||
| Tuesday, !- Day of Week for Start Day | ||
| Yes, !- Use Weather File Holidays and Special Days | ||
| Yes, !- Use Weather File Daylight Saving Period | ||
| No, !- Apply Weekend Holiday Rule | ||
| Yes, !- Use Weather File Rain Indicators | ||
| Yes; !- Use Weather File Snow Indicators |
There was a problem hiding this comment.
This test runs the full weather-file run period (1/1–12/31) in addition to the design days. Since this is primarily intended to exercise the ConvexWeilerAtherton clipping path, consider shortening the RunPeriod (or switching to DESIGN_DAY_ONLY) to reduce CI runtime while still covering the algorithm.
|
|
Collaborator
|
@joseph-robertson I have restarted regressions here. |
|
|
|
Collaborator
|
@joseph-robertson regression results look to be back on track. |
joseph-robertson
changed the title
|
|
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request overview
InitSolarCalculations.Output:Variable,*,Zone Mean Radiant Temperature,hourly;lines in a handful of test filesSurface Outside Face Sunlit Fractionoutput variable unitsDescription of the purpose of this PR
Pull Request Author
Reviewer