Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix floor solar heat gain issue from incident solar multiplier #10010

Merged
merged 9 commits into from
Sep 20, 2023

Conversation

yujiex
Copy link
Collaborator

@yujiex yujiex commented May 11, 2023

Pull request overview

  • Fixes Incorrect solar heat gain for opaque surfaces w/ SurfaceProperty:IncidentSolarMultiplier object #10001
  • In the calculation of beam solar entering the zones, the environment beam solar radiation is multiplied to a zone-level overall transmittance term. Thus the incident solar multiplier was not applied as it is at the surface level. The PR fixes the issue by adding the multiplier to the transmittance in the calculation of BTOTZone and BTOTWinZone.
  • The effect of the fix is evaluated using several example file

validation with 1ZoneEvapCooler_4Win_incidentSolarMultiplier.idf

test_1ZoneEvapCooler_4Win_incidentSolarMultiplier.zip

image

Before fixing, the ZN001:FLR001:Surface Inside Face Solar Radiation Heat Gain Energy is non-zero even if the incident solar multiplier of all windows is set to 0 (full-shading, no sunlight).

image

After fixing, the ZN001:FLR001:Surface Inside Face Solar Radiation Heat Gain Energy is zero (expected).

image

validation with 5ZoneAirCooled.idf

image

The model is also verified using 5ZoneAirCooled.idf, with the idf file and the output csv files before after fixing the multiplier. F1-1 through F5-1 are the floor surfaces for each zone. Before the fix, the floor surface has non-zero solar radiation heat gain even when the incident solar multiplier is set to 0 for all windows (glass doors are removed in this experiment).

test_5ZoneAirCooled.zip

before

image

after

image

validation with AirflowNetwork_Simple_House.idf

image

test_AirflowNetwork_Simple_House.zip

When validated using AirflowNetwork_Simple_House.idf, the floor solar heat gain did not reduced to 0. It is fixed by adding the adjustment here

image

before

image

after

image

validation with AbsorptionChiller.idf

image

test_AbsorptionChiller.zip

before

image

after

image

regression diff

Regression diffs are anticipated in the example file 1ZoneEvapCooler_4Win_incidentSolarMultiplier.idf. During the day time, this fix corrects the inside surface solar heat gain, as previously the multiplier is not properly applied to the amount of solar radiation entering the zones. After correction, the floor surface solar heat gain is anticipated to drop, which is indeed the case. As a result, the sensible cooling energy should drop as well as a result of reduced solar radiation heat gain on the interior surfaces. However for the hour 1-4 (highlighted), the sun is not up yet (see first column). I did some investigation to see whether it is reasonable that the differences in sensible cooling load takes place even before the sun is up.

image

After some investigation, it seems this behavior is reasonable. Even if the floor solar heat gain differs only during the day time when the sun is up, the resulting zone temperature difference can sustain longer. This zone temperature is carried over to later warmup periods as well as simulation run periods. We can see during the first warmup period, the floor solar heat gain starts to differ in the 5th hour, zone temperature starts to differ in the 6th hour, and zone sensible cooling load starts to differ in the 8th hour. The temperature difference carries over to the 2nd to 6th warmup periods and then the design day simulation period. Similarly, the sensible cooling load difference also carries on to other warmup periods and the design day simulation period as a result of the air temperature difference.

image image image

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@yujiex yujiex added the Defect Includes code to repair a defect in EnergyPlus label May 11, 2023
@yujiex yujiex requested a review from shorowit May 11, 2023 15:57
@yujiex yujiex self-assigned this May 11, 2023
Copy link
Contributor

@shorowit shorowit left a comment

Choose a reason for hiding this comment

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

I spot tested this on several of our residential models; the agreement between our current shading approach (hacky workaround using a combination of shading surfaces and EMS-actuated view factors) and this approach is now much better.

I cannot speak to the correctness of the code changes, but can confirm that this resolves the issue I was seeing. Thanks, @yujiex! I can't wait to finally switch our models to use this object in the next release.

@yujiex
Copy link
Collaborator Author

yujiex commented May 12, 2023

@shorowit Thanks for spotting the issue and checking on the modified code. I'll do some more testing on my side too :)

@Myoldmopar
Copy link
Member

I'll do some more testing on my side too :)

@yujiex does this mean you'll be doing more testing on this PR? Just wanting to confirm the status of it before spending any review time on it. Thanks!

@yujiex
Copy link
Collaborator Author

yujiex commented May 16, 2023

I'll do some more testing on my side too :)

@yujiex does this mean you'll be doing more testing on this PR? Just wanting to confirm the status of it before spending any review time on it. Thanks!

Hi @Myoldmopar, sorry for the late reply. Yes, I'll add some more testing to make sure this type of un-adjusted solar transmission is all caught.

@Myoldmopar
Copy link
Member

@yujiex I see Clang Format failed on the last run, but I'll hold off on doing anything here as you stated you had additional testing to do.

@nrel-bot
Copy link

nrel-bot commented Jul 6, 2023

@yujiex @Myoldmopar it has been 28 days since this pull request was last updated.

@shorowit
Copy link
Contributor

Hi @yujiex, I just wanted to make sure that you're planning to get this into the next E+ release. Thank you!

@nrel-bot-2b
Copy link

@yujiex it has been 22 days since this pull request was last updated.

@nrel-bot-2
Copy link

@yujiex it has been 7 days since this pull request was last updated.

@Myoldmopar Myoldmopar added this to the EnergyPlus 23.2 milestone Aug 15, 2023
@nrel-bot-2c
Copy link

@yujiex it has been 17 days since this pull request was last updated.

Copy link
Contributor

@shorowit shorowit left a comment

Choose a reason for hiding this comment

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

I retested the latest code on a couple residential IDFs and the results still look good to me.

@yujiex
Copy link
Collaborator Author

yujiex commented Sep 20, 2023

I retested the latest code on a couple residential IDFs and the results still look good to me.

Thanks. @shorowit

@Myoldmopar
Copy link
Member

OK cool, then let's get this in. Thanks @shorowit and @yujiex, CI is all happy here, I'm merging.

@Myoldmopar Myoldmopar merged commit 3b44598 into develop Sep 20, 2023
@Myoldmopar Myoldmopar deleted the fixIncSolarMultiplier branch September 20, 2023 13:19
@yujiex
Copy link
Collaborator Author

yujiex commented Sep 20, 2023

OK cool, then let's get this in. Thanks @shorowit and @yujiex, CI is all happy here, I'm merging.

Thanks @Myoldmopar and @shorowit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Includes code to repair a defect in EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect solar heat gain for opaque surfaces w/ SurfaceProperty:IncidentSolarMultiplier object
8 participants