Correction of Window Face Temperature Reporting for Non-Complex Windows #7474
Conversation
The problem noted by a user is that when requesting Surface Window Front (or Back) Face Temperature Layer X from EnergyPlus that for a non-BSDF window type that these variables are not defined by anything and thus show up as zero in the output. This fixes this by assigning the correct value at least at the outer and inner most faces and will not output anything for in-between layers that are not calculated.
A unit test to exercise the new code and documentation edits to the Input Output Reference are included in this commit.
|
This all looks good. My only question would be why the inner/outer surface temps could not also be reported for non-BSDF windows? To be clear, this change addresses the issue, but for the future shouldn't all window surface temps be available? |
mjwitte
added
the
OutputChange
|
Added OutputChange label - at first glance this may have to wait until after release due to changes in output variables? Not sure though. |
|
@rraustad But this change specifically did allow the reporting of non-BSDF windows--at the outer most outward facing layer and the inner most inward facing layer. The unit test documents the setting of those variables and the documentation also shows that. So, you get all layers in both directions for BSDF windows because it is calculating things layer by layer. For non-BSDF windows, you just get what gets calculated at the inside and outside surface heat balance. Those variables were already available like they would be for any other surface so this can be verified pretty easily. So, I think this work addresses your concern already...or is something missing??? |
|
Maybe I was just confused by the description. Reading again I guess it says outer/inner layers of non-BSDF windows will be reported but others will not. "For other layers of non-BSDF windows, the output will not be produced at all." |
mjwitte
removed
the
OutputChange
|
Removed outputchange label - this is just fixing existing variables. |
| @@ -2632,15 +2632,16 @@ \subsection{Window Output Variables}\label{window-output-variables} | |||
|
|
|||
| \subsubsection{Surface Window Total Absorbed Shortwave Radiation Rate Layer \textless{}x\textgreater{} {[}W{]}}\label{surface-window-total-absorbed-shortwave-radiation-rate-layer-x-w} | |||
|
|
|||
| This will output shortwave radiation absorbed in a window layer. The key values for this output variable are the surface name. Layers are numbered from the outside to the inside of the surface. The full listing will appear in the RDD file. | |||
| This will output shortwave radiation absorbed in a window layer. The key values for this output variable are the surface name. Layers are numbered from the outside to the inside of the surface. The full listing will appear in the RDD file. Note that this variable is only defined for constructions defined by a \hyperref[constructioncomplexfenestrationstate]{Construction:ComplexFenestrationState}. | |||
| @@ -5608,6 +5608,10 @@ namespace HeatBalanceManager { | |||
| for (SurfNum = 1; SurfNum <= TotSurfaces; ++SurfNum) { | |||
| DataSurfaces::Surface(SurfNum).MovInsulIntPresentPrevTS = DataSurfaces::Surface(SurfNum).MovInsulIntPresent; | |||
| } | |||
|
|
|||
| // For non-complex windows, update a report variable so this shows up in the output as something other than zero | |||
| UpdateWindowFaceTempsNonBSDFWin(); | |||
| "Zone", | ||
| "Average", | ||
| Surface(SurfLoop).Name); | ||
| } |
There was a problem hiding this comment.
OK, shortwave absorbed is only defined for BSDF.
| "Zone", | ||
| "Average", | ||
| Surface(SurfLoop).Name); | ||
| } |
There was a problem hiding this comment.
But the front and back layers are available for all types, and for every layer in BSDF.
|
|
||
| DataHeatBalance::clear_state(); | ||
| DataSurfaces::clear_state(); | ||
| DataHeatBalSurface::clear_state(); |
There was a problem hiding this comment.
Definitely should not be necessary. These should've been cleaned up by the cleanup from the unit test that ran before this. It's OK that they are here right now, but just note this shouldn't be needed. If it is then something else is wrong.
|
|
||
| // Second surface is a window so these should be set | ||
| EXPECT_NEAR(FenLaySurfTempFront(1,2),DataHeatBalSurface::TH(1,1,2),0.0001); | ||
| EXPECT_NEAR(FenLaySurfTempBack(SurfsForRegWindow,2),DataHeatBalSurface::TH(2,1,2),0.0001); |
|
@RKStrand this needs the example files cleaned up to get rid of the error diffs due to the warning message about unused output variable. While making that change, might as well go ahead and fix up the unnecessary clear_state calls at the beginning of the new unit test. I'm going to build this locally and verify anyway, so I'll likely just make those changes and push the fixes up. |
|
@Myoldmopar Thanks for sorting that out (if you end up doing it). My apologies for some sloppy clear_state calls in the unit test. I was operating under the mode that each unit test has to perform on its own and not rely on anything from any previous unit test. My thought was that if the previous unit test changes or gets deleted (would that ever happen?) that would then potentially cause a problem with this one. I understand that could be somewhat wasteful though with lots and lots of calls to clear_state routines that aren't really needed. |
|
You are generally right, a unit test should operate on their own. However, clear_state calls are handled at the end of every single unit test automatically by the EnergyPlusFixture base class. It is expected that through these clear_state calls, the entire global state of EnergyPlus will be reset fully to its raw state again. If a clear_state needs to be called within a unit test, it indicates that something wasn't properly added to the fixture base class. I pulled this branch and tested locally. With develop pulled in, and the unneeded clear_state calls in place, the unit test suite runs successfully. I also re-ran the example files after cleaning out the output variable issue and the error files are fully clean again. Merging this. |
Pull request overview
A user noted that the window face temperatures were reporting as zero. Further investigation showed that these variables were only being produced for complex (BSDF) windows. This pull request includes a fix that allows the outer most surface outward facing temperature and the inner most surface inward facing temperature to be produced. For other layers of non-BSDF windows, the output will not be produced at all. In addition, the variable Surface Window Total Absorbed Shortwave Radiation Rate Layer will only be produced for BSDF windows, not for other types. This addresses GitHub Issue #4420.
Work Checklist
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Review Checklist
This will not be exhaustively relevant to every PR.