Refactoring of interpolation functions for blinds Prof and Slat Angle #8591
Conversation
xuanluo113
added
the
Performance
| { | ||
| return Lower + InterpFac * (Upper - Lower); | ||
| }; | ||
|
|
There was a problem hiding this comment.
I'm sure declaring this function constexpr vs. just inline doesn't hurt anything, but I doubt any of the arguments will actually be constexpr and enable any compile-time reduction. Same for InterpProfSlat.
|
@mjwitte This branch is ready for review. Please let me know if you have any availability in reviewing this, or you have some recommendations for reviewers. I also wonder if this could come before the 9.5 release. Thanks! |
There was a problem hiding this comment.
@xuanluo113 Nice work here. I have a little more testing to do, but looks good so far.
| zone_info.Emissivity(ZoneSurfNum) = | ||
| InterpSlatAng(state.dataSurface->SurfWinSlatAngThisTS(SurfNum), state.dataSurface->SurfWinMovableSlats(SurfNum), surface_window.EffShBlindEmiss) + | ||
| InterpSlatAng(state.dataSurface->SurfWinSlatAngThisTS(SurfNum), state.dataSurface->SurfWinMovableSlats(SurfNum), surface_window.EffGlassEmiss); | ||
| zone_info.Emissivity(ZoneSurfNum) = state.dataHeatBal->ITABSF(SurfNum); |
There was a problem hiding this comment.
So, if I'm following correctly, ITABSF is set in HeatBalanceSurfaceManager::ComputeIntThermalAbsorpFactors (same as before, with some adjustments in this branch) so no need to recompute it here.
| @@ -318,8 +314,7 @@ namespace HeatBalanceIntRadExchange { | |||
| // and emiss is used here that is a weighted combination of shade/blind and glass temp and emiss. | |||
| } else if (ANY_INTERIOR_SHADE_BLIND(state.dataSurface->SurfWinShadingFlag(SurfNum))) { | |||
| SurfaceTempRad[ZoneSurfNum] = state.dataSurface->SurfWinEffInsSurfTemp(SurfNum); | |||
| SurfaceEmiss[ZoneSurfNum] = InterpSlatAng(state.dataSurface->SurfWinSlatAngThisTS(SurfNum), state.dataSurface->SurfWinMovableSlats(SurfNum), surface_window.EffShBlindEmiss) + | |||
| InterpSlatAng(state.dataSurface->SurfWinSlatAngThisTS(SurfNum), state.dataSurface->SurfWinMovableSlats(SurfNum), surface_window.EffGlassEmiss); | |||
| SurfaceEmiss[ZoneSurfNum] = state.dataHeatBal->ITABSF(SurfNum); | |||
There was a problem hiding this comment.
Same here, just use ITABSF directly. Ok.
| state.dataConstruction->Construct( ConstrNumSh).AbsDiffBlind) * | ||
| (SkySolarInc + GndSolarInc); | ||
| Real64 AbsDiffBlind; | ||
| if (state.dataSurface->SurfWinMovableSlats(SurfNum)) { |
There was a problem hiding this comment.
Minor detail - these if blocks could be combined with the corresponding ones above.
| @@ -10762,10 +10988,7 @@ namespace EnergyPlus::SolarShading { | |||
|
|
|||
| // Accumulate Window and Zone total distributed diffuse solar to check for conservation of energy | |||
| // For opaque surfaces all incident diffuse is either absorbed or reflected | |||
| // WinDifSolarDistAbsorbedTotl += DifSolarAbsW; // debug [W] | |||
| @@ -1513,6 +1538,12 @@ struct SurfacesData : BaseGlobalStruct | |||
| Array1D<bool> SurfWinSlatAngThisTSDegEMSon; // flag that indicate EMS system is actuating SlatAngThisTSDeg | |||
| Array1D<Real64> SurfWinSlatAngThisTSDegEMSValue; // value that EMS sets for slat angle in degrees | |||
| Array1D<bool> SurfWinSlatsBlockBeam; // True if blind slats block incident beam solar | |||
| Array1D<int> SurfWinSlatsAngIndex; | |||
There was a problem hiding this comment.
6 new SurfWin* arrays to avoid recomputing values.
There was a problem hiding this comment.
@xuanluo113 Thanks for addressing the comments. Since the performance tests don't have any blinds, I dug up the notorious file from last year with 100s of windows with blinds in a large zone. It's still warming up after running for 12 hrs. So, instead I took PurchAirWindowBlind_BlockBeamSolar and ran that for 10yrs and 20yrs with develop and this branch. This branch runs about 10% faster. Nice! Merging.
Pull request overview
This branch includes the refactoring of the listed functions (e.g. inlining, cache intermediate results, reduce redundant calcs).
Also, the MACROs in checking shading flag status were converted to
constexpr.The small diffs with the 3 blind related testfiles result from the floating-point error explained here. Previously
InterpSlatAnguses* DeltaAng_inv;, whileInterpProfSlatAnguses/ DeltaAnginstate.dataSurface->SurfWinSlatsAngInterpFac(ISurf)calculation. The diffs are less than 10e-15.Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fixIf feature, test running new feature, try creative ways to break itVerify IDF naming conventions and styles, memos and notes and defaultsIf new idf included, locally check the err file and other outputs