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.

Sign up for GitHub

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 array bounds error for interzone windows and fix convexity of mirrored surfaces #10498

Merged
merged 17 commits into from
Jun 25, 2024
Merged

Fix array bounds error for interzone windows and fix convexity of mirrored surfaces #10498

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
17c808c
Fix interior window array size
mjwitte May 6, 2024
7461b39
Fix Surface2D assert
mjwitte May 6, 2024
c40c608
Merge remote-tracking branch 'remotes/origin/develop' into Interzonew…
mjwitte May 8, 2024
bcf2046
Initial unit tests
mjwitte May 8, 2024
259849e
Revert Surface2D assert
mjwitte May 8, 2024
2a93689
Revise unit tests
mjwitte May 8, 2024
85662bb
Fix convexity for mirrored surfaces
mjwitte May 8, 2024
28ec9eb
Streamline MakeMirrorSurface
mjwitte May 9, 2024
b5d41cf
Add some constants
mjwitte May 9, 2024
c4fd8dc
Use new constants
mjwitte May 9, 2024
57e23ee
Use new constants and housekeeping
mjwitte May 9, 2024
ebe5fee
Merge remote-tracking branch 'remotes/origin/develop' into Interzonew…
mjwitte Jun 11, 2024
7243d5a
Rename SmallDistance and add units
mjwitte Jun 11, 2024
06b60e4
More constants correct I/O ref
mjwitte Jun 11, 2024
d36f8ba
Extend unit test, drop param, fix EnclSolRecDifShortFromZ loop
mjwitte Jun 12, 2024
231b3b1
Merge remote-tracking branch 'remotes/origin/develop' into Interzonew…
mjwitte Jun 12, 2024
72f29f7
format
mjwitte Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/EnergyPlus/DataGlobalConstants.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,10 @@ namespace Constant {
Real64 constexpr OneFifth = 1.0 / 5.0; // 1/5 in highest precision
Real64 constexpr OneSixth = 1.0 / 6.0; // 1/6 in highest precision
Real64 constexpr FourFifths = 4.0 / 5.0; // 4/5 in highest precision
Real64 constexpr OneThousandth = 1.0e-3; // Used as a tolerance in various places
Real64 constexpr OneMillionth = 1.0e-6; // Used as a tolerance in various places

Real64 constexpr DistTooSmall(1.e-4); // Geometric tolerance
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some new constants that are used as tolerances in various places in surface geometry and shadowing calculations. DistToolSmall was moved up from a local declaration in Vectors.cc - decided to keep the same name.

Copy link
Member

Choose a reason for hiding this comment

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

Does DistTooSmall represent a unit or is it dimensionless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does DistTooSmall represent a unit or is it dimensionless?

Good questions. After some digging, it's in meters. And after some discussion with @JasonGlazer about other geometric tolerances, I'm going to make some other changes. How do you feel about names like this?
Real64 constexpr 1centimeter = 0.01 // Distance tolerance [m]

Copy link
Member

Choose a reason for hiding this comment

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

Well, I mean that exact name won't compile since it starts with a numeric literal :) But I think oneCentimeter is fine. Or you could just call it oneHundredth and allow it to be used for multiple units. As long as it's pretty obvious I'm not too picky.

Real64 constexpr MaxEXPArg = 709.78; // maximum exponent in EXP() function
Real64 constexpr Pi = 3.14159265358979324; // Pi 3.1415926535897932384626435
Real64 constexpr PiOvr2 = Pi / 2.0; // Pi/2
Expand Down
14 changes: 2 additions & 12 deletions src/EnergyPlus/DataSurfaces.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ namespace EnergyPlus::DataSurfaces {
// Dec 2006, DJS (PSU) added logical ecoroof variable
// Dec 2008, TH added new properties to SurfaceWindowCalc for thermochromic windows
// Jul 2011, M.J. Witte and C.O. Pedersen, add new fields to OSC for last T, max and min
// RE-ENGINEERED na

// Using/Aliasing
using namespace DataVectorTypes;
Expand All @@ -88,15 +87,6 @@ using namespace WindowManager;

Array1D_string const cExtBoundCondition({-6, 0}, {"KivaFoundation", "FCGround", "OSCM", "OSC", "OSC", "Ground", "ExternalEnvironment"});

// Parameters to indicate surface classes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed some comments that were left behind at some point when the parameters were moved to an enum.

// Surface Class (FLOOR, WALL, ROOF (incl's CEILING), WINDOW, DOOR, GLASSDOOR,
// SHADING (includes OVERHANG, WING), DETACHED, INTMASS),
// TDD:DOME, TDD:DIFFUSER (for tubular daylighting device)
// (Note: GLASSDOOR and TDD:DIFFUSER get overwritten as WINDOW
// in SurfaceGeometry.cc, SurfaceWindow%OriginalClass holds the true value)
// why aren't these sequential (LKL - 13 Aug 2007)

// Constructor
Surface2D::Surface2D(ShapeCat const shapeCat, int const axis, Vertices const &v, Vector2D const &vl, Vector2D const &vu)
: axis(axis), vertices(v), vl(vl), vu(vu)
{
Expand Down Expand Up @@ -180,7 +170,7 @@ Surface2D::Surface2D(ShapeCat const shapeCat, int const axis, Vertices const &v,
xt = xte;
}
#endif
assert((shapeCat == ShapeCat::Nonconvex) || (crossEdges.size() == 2));
assert((shapeCat == ShapeCat::Nonconvex) || (crossEdges.size() == 2u));
Comment on lines -183 to +173
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the assert that was failing. Initially, I thought it was incorrect and should be >=2, but in the end the failing surface was ShapeCat:Convex here when it should have been Nonconvex.

for (auto const &edge : crossEdges) {
size_type const iEdge(std::get<2>(edge));
slab.edges.push_back(iEdge); // Add edge to slab
Expand Down Expand Up @@ -470,7 +460,7 @@ Surface2D SurfaceData::computed_surface2d() const

Real64 SurfaceData::get_average_height(EnergyPlusData &state) const
{
if (std::abs(SinTilt) < 1.e-4) {
if (std::abs(SinTilt) < Constant::DistTooSmall) {
return 0.0;
}
using Vertex2D = ObjexxFCL::Vector2<Real64>;
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/HeatBalanceSurfaceManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ void InitSurfaceHeatBalance(EnergyPlusData &state)
if (state.dataHeatBalSurfMgr->InitSurfaceHeatBalancefirstTime) {
DisplayString(state, "Computing Interior Diffuse Solar Exchange through Interzone Windows");
}
ComputeDifSolExcZonesWIZWindows(state, state.dataGlobal->NumOfZones);
ComputeDifSolExcZonesWIZWindows(state, state.dataViewFactor->NumOfSolarEnclosures);
}

Dayltg::initDaylighting(state, state.dataHeatBalSurfMgr->InitSurfaceHeatBalancefirstTime);
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/SolarReflectionManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ namespace SolarReflectionManager {
ObsVec = state.dataSurface->Surface(ObsSurfNum).Vertex(loop);
DotProd = dot(state.dataSolarReflectionManager->SolReflRecSurf(RecSurfNum).NormVec, ObsVec - RecVec);
// CR8251 IF(DotProd > 0.01d0) THEN ! This obstructing-surface vertex is not behind receiving surface
if (DotProd > 1.0e-6) { // This obstructing-surface vertex is not behind receiving surface
if (DotProd > Constant::OneMillionth) { // This obstructing-surface vertex is not behind receiving surface
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that's actually really satisfying to read like this.

ObsBehindRec = false;
break;
}
Expand Down
8 changes: 4 additions & 4 deletions src/EnergyPlus/SolarShading.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5746,7 +5746,7 @@ void SHADOW(EnergyPlusData &state,
state.dataSurface->Surface(NGRS).lcsz.y * state.dataSolarShading->SUNCOS(2) +
state.dataSurface->Surface(NGRS).lcsz.z * state.dataSolarShading->SUNCOS(3);

if (std::abs(ZS) > 1.e-4) {
if (std::abs(ZS) > Constant::DistTooSmall) {
state.dataSolarShading->XShadowProjection = XS / ZS;
state.dataSolarShading->YShadowProjection = YS / ZS;
if (std::abs(state.dataSolarShading->XShadowProjection) < 1.e-8) state.dataSolarShading->XShadowProjection = 0.0;
Expand Down Expand Up @@ -11231,7 +11231,7 @@ void CalcBeamSolarOnWinRevealSurface(EnergyPlusData &state)
if (A2ill < 0.0) A2ill = 0.0;

// Quantities related to outside reveal
if (A1ill > 1.0e-6) {
if (A1ill > Constant::OneMillionth) {

state.dataSurface->SurfWinBmSolAbsdOutsReveal(SurfNum) +=
A1ill * state.dataSurface->SurfWinOutsideRevealSolAbs(SurfNum) * CosBeta * tmp_SunlitFracWithoutReveal;
Expand Down Expand Up @@ -11259,7 +11259,7 @@ void CalcBeamSolarOnWinRevealSurface(EnergyPlusData &state)

if (NOT_SHADED(ShadeFlag) || ShadeFlag == WinShadingType::SwitchableGlazing) {

if (A2ill > 1.0e-6) {
if (A2ill > Constant::OneMillionth) {

DiffReflGlass = state.dataConstruction->Construct(ConstrNum).ReflectSolDiffBack;
if (ShadeFlag == WinShadingType::SwitchableGlazing) {
Expand Down Expand Up @@ -12751,7 +12751,7 @@ void CalcComplexWindowOverlap(EnergyPlusData &state,
ZSp = -SdotZ;

// Projection of shadows for current basis direciton
if (std::abs(ZSp) > 1.e-4) {
if (std::abs(ZSp) > Constant::DistTooSmall) {
XShadowProjection = XSp / ZSp;
YShadowProjection = YSp / ZSp;
if (std::abs(XShadowProjection) < 1.e-8) XShadowProjection = 0.0;
Expand Down
Loading
Loading