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

Incorrect local variable scope in CalcInteriorSolarDistribution #8369

Closed
2 of 3 tasks
xuanluo113 opened this issue Nov 5, 2020 · 2 comments · Fixed by #8358
Closed
2 of 3 tasks

Incorrect local variable scope in CalcInteriorSolarDistribution #8369

xuanluo113 opened this issue Nov 5, 2020 · 2 comments · Fixed by #8358
Assignees
Labels
Defect Includes code to repair a defect in EnergyPlus

Comments

@xuanluo113
Copy link
Contributor

xuanluo113 commented Nov 5, 2020

A potential bug in solar calculations in CalcInteriorSolarDistribution.

The reporting at the end of the function starts a new surface loop, but the WinTransBmBmSolar, WinTransBmDifSolar, TBmBm + TBmDif are not re-calculated. The value, WinTransBmBmSolar, WinTransBmBmSolar, and TBmBm + TBmDif used in the reporting loop are always the last calculated value for the last surface in the previous loop.

Defect file: testfiles/PurchAirWindowBlind_BlockBeamSolar.idf

Details

Some additional details for this issue (if relevant):

  • Platform (Operating system, version)
  • Version of EnergyPlus (if using an intermediate build, include SHA)
  • Unmethours link or helpdesk ticket number

Checklist

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

  • Defect file added (list location of defect file here)
  • Ticket added to Pivotal for defect (development team task)
  • Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)
@xuanluo113 xuanluo113 self-assigned this Nov 5, 2020
@xuanluo113 xuanluo113 added the Defect Includes code to repair a defect in EnergyPlus label Nov 5, 2020
@xuanluo113 xuanluo113 added this to the EnergyPlus Future milestone Nov 5, 2020
@mjwitte
Copy link
Contributor

mjwitte commented Nov 6, 2020

@xuanluo113 If I am following this correctly, this only impacts the following output variables. It does not affect the actual heat balance of the zone.

Window Transmitted Beam-to-Beam Solar
Window Transmitted Beam-to-Diffuse Solar
Window Transmitted Beam-to-Beam Solar Energy
Window Transmitted Beam-to-Diffuse Solar Energy

And these variables have been wrong since they were first added in 2009/2010.

@xuanluo113
Copy link
Contributor Author

xuanluo113 commented Nov 7, 2020

@mjwitte I agree.

  1. The only reportable variables affected are:
  • Surface Window Transmitted Beam To Beam Solar Radiation Rate
  • Surface Window Transmitted Beam To Diffuse Solar Radiation Rate
  • Surface Window Transmitted Beam To Beam Solar Radiation Energy
  • Surface Window Transmitted Beam To Diffuse Solar Radiation Energy

This may only affect the result of PurchAirWindowBlind_BlockBeamSolar.idf that currently reports those. However, there is only one window per zone in this model, so using calculations of the last (and only) window in the reporting loop does not affect anything, coincidentally.

  1. The other affected variable SurfWinDirSolTransAtIncAngle(SurfNum)// For TDD:DIFFUSER this is the TDD transmittance are not actually used anywhere, since the transmittance for the diffuser surface should be calculated as windows. My understanding is the variable declaration and calculation could be deleted. Would you mind confirming this by taking another look?

The fixes in PR #8358 show no diff.

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 a pull request may close this issue.

2 participants