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

Bug in TDD related calculations in CalcInteriorSolarDistribution #8356

Closed
3 tasks
xuanluo113 opened this issue Oct 27, 2020 · 2 comments · Fixed by #8358
Closed
3 tasks

Bug in TDD related calculations in CalcInteriorSolarDistribution #8356

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

Comments

@xuanluo113
Copy link
Contributor

xuanluo113 commented Oct 27, 2020

Issue overview

Three potential bugs in TDD related calculations in CalcInteriorSolarDistribution.

  1. The reporting at the end of the function starts a new surface loop, but the CosInc here is not re-calculated. The value, CosInc(Surfnum), used here is always the last calculated value for the last surface in the previous loop.

  2. There are multiple special calculations (that are supposed to happen) for TDD Diffusers in the calculation loop (branch 1, branch 2, branch 3, branch 4), but Diffusers are not ExtSolar surface, and they get bypassed after this line.

  3. The branch here for TDD dome can never be reached as it's under an else if condition of SurfWinWindowModelType(SurfNum) != WindowBSDFModel && SurfWinWindowModelType(SurfNum) != WindowEQLModel, which also applies to TDD Domes.

Defect file: testfiles/DaylightingDeviceTubular.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 Oct 27, 2020
@xuanluo113 xuanluo113 added the Defect Includes code to repair a defect in EnergyPlus label Oct 27, 2020
@xuanluo113 xuanluo113 added this to the EnergyPlus Future milestone Oct 27, 2020
@mjwitte
Copy link
Contributor

mjwitte commented Oct 27, 2020

@xuanluo113 All true.

  1. This has been broken since these calculations were first added in 2003.
  2. This has been broken since 2010. Would it work to set ExtSolar = true for TDD_Diffuser? But that could have unwanted side effects. It can't be set on input, because that would be an error (Sunlit = Yes on an interior surface), but maybe it could be set internally.
  3. This has been broken since 2012. TDD_Diffuser and TDD_Dome need to be first.

@xuanluo113
Copy link
Contributor Author

@mjwitte Thanks.

  1. ExtSolar is fixed by adding an && clause, the previous check of if (((Surface(SurfNum).ExtBoundCond != ExternalEnvironment) && (Surface(SurfNum).ExtBoundCond != OtherSideCondModeledExt)) && SurfWinOriginalClass(SurfNum) != SurfaceClass_TDD_Diffuser) is deleted since duplicated.

  2. The branch is reordered. Regular windows and EQL windows are merged as they seem to have identical calculations.

Tested with DaylightingDevicesTubular.idf and DaylightingDeviceShelf.idf to make sure the branches get visited. Diffs are expected. (Although not seen for DaylightingDeviceShelf.idf in CI)

2 and 3 are temporary fixes. The TDD branches could be sorted out of the loop later.

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