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

Jump to bottom

Vertical blind warning not reported from material get input function #10743

Merged
merged 4 commits into from
Oct 15, 2024
Merged

Vertical blind warning not reported from material get input function #10743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e48f54
Fix vertical blind warning
rraustad Sep 16, 2024
8d8de9f
CppCheck shadow and const
rraustad Sep 16, 2024
4c180e9
Update IO Ref
rraustad Sep 16, 2024
6acb0e7
Merge remote-tracking branch 'origin/develop' into 10742-vertical-bli…
Myoldmopar Oct 15, 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
16 changes: 8 additions & 8 deletions doc/input-output-reference/src/overview/group-surface-construction-elements.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2667,41 +2667,41 @@ \subsubsection{Inputs}\label{inputs-24-003}

\paragraph{Field: Slat Width}\label{field-slat-width-1}

The width (m) of the venetian slats.~ Used only for ShadingLayerType = Venetian.
The width (m) of the venetian slats.~ Used only for ShadingLayerType = VenetianHorizontal or VenetianVertical.

\paragraph{Field: Slat Spacing}\label{field-slat-spacing}

The distance (m) between front sides of the venetian slats.~ Used only for ShadingLayerType = Venetian.
The distance (m) between front sides of the venetian slats.~ Used only for ShadingLayerType = VenetianHorizontal or VenetianVertical.

\paragraph{Field: Slat Thickness}\label{field-slat-thickness-1}

The thickness (m) of the venetian slats.~ Used only for ShadingLayerType = Venetian.
The thickness (m) of the venetian slats.~ Used only for ShadingLayerType = VenetianHorizontal or VenetianVertical.

\paragraph{Field: Slat Angle}\label{field-slat-angle-1}

The slat tilt angle (degrees) of the venetian slats.~ Used only for ShadingLayerType = Venetian.~ Range of slat angle is from -90 to 90 degrees.
The slat tilt angle (degrees) of the venetian slats.~ Used only for ShadingLayerType = VenetianHorizontal or VenetianVertical.~ Range of slat angle is from -90 to 90 degrees.

\paragraph{Field: Slat Conductivity}\label{field-slat-conductivity-1}

The conductivity (W/mK) of the venetian slats.~ Used only for ShadingLayerType = Venetian.
The conductivity (W/mK) of the venetian slats.~ Used only for ShadingLayerType = VenetianHorizontal or VenetianVertical.

\paragraph{Field: Slat Curve}\label{field-slat-curve}

The curvature radius (m) of the venetian slats.~ Setting this value to zero means there is no curvature in the slat (it is flat), while a non-zero value is the radius of the slat curve.~ This value cannot be smaller than Slat Width / 2.~ Used only for ShadingLayerType = Venetian.
The curvature radius (m) of the venetian slats.~ Setting this value to zero means there is no curvature in the slat (it is flat), while a non-zero value is the radius of the slat curve.~ This value cannot be smaller than Slat Width / 2.~ Used only for ShadingLayerType = VenetianHorizontal or VenetianVertical.

\begin{figure}[hbtp] % fig 18
\centering
\includegraphics[width=0.9\textwidth, height=0.9\textheight, keepaspectratio=true]{media/image042.png}
\caption{Side view of horizontal venetian blind slats or top view of blinds with vertical slats. Front face of slats is marked with red line. \protect \label{fig:side-view-of-horizontal-venetian-blind-slats}}
\end{figure}

An IDF example for ShadingLayerType = Venetian
An IDF example for ShadingLayerType = VenetianHorizontal

\begin{lstlisting}

WindowMaterial:ComplexShade, !- venetian blind layer
Shade_30001_Layer, !- name
Venetian, !- shading layer type
VenetianHorizontal, !- shading layer type
0.005, !- thickness
160, !- layer conductivity
0.0, !- IR transmittance
Expand Down
57 changes: 12 additions & 45 deletions src/EnergyPlus/Material.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ constexpr std::array<Material::Gas, 10> gases = {

constexpr std::array<std::string_view, (int)EcoRoofCalcMethod::Num> ecoRoofCalcMethodNamesUC = {"SIMPLE", "ADVANCED"};

int GetMaterialNum(EnergyPlusData &state, std::string const &matName)
int GetMaterialNum(EnergyPlusData const &state, std::string const &matName)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[src/EnergyPlus/Material.cc:87]:(style),[constParameter],Parameter 'state' can be declared as reference to const

{
auto const &s_mat = state.dataMaterial;
auto found = s_mat->materialMap.find(Util::makeUPPER(matName));
Expand Down Expand Up @@ -2496,14 +2496,14 @@ void GetMaterialData(EnergyPlusData &state, bool &ErrorsFound) // set to true if
continue;
}

auto const &instances = itInstances.value();
auto itObj = instances.begin();
auto const &instances2 = itInstances.value();
auto itObj = instances2.begin();
// Can't use find here because epJSON keys are not upper-cased
for (; itObj != instances.end(); ++itObj) {
for (; itObj != instances2.end(); ++itObj) {
if (Util::makeUPPER(itObj.key()) == s_ipsc->cAlphaArgs(3)) break;
}

if (itObj == instances.end()) {
if (itObj == instances2.end()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[src/EnergyPlus/Material.cc:2499]:(style),[shadowVariable],Local variable 'instances' shadows outer variable
[src/EnergyPlus/Material.cc:2526]:(style),[shadowVariable],Local variable 'instances' shadows outer variable

ShowSevereItemNotFound(state, eoh, s_ipsc->cAlphaFieldNames(3), s_ipsc->cAlphaArgs(3));
ErrorsFound = true;
continue;
Expand All @@ -2523,15 +2523,15 @@ void GetMaterialData(EnergyPlusData &state, bool &ErrorsFound) // set to true if
continue;
}

auto const &instances = itInstances.value();
auto const &instances3 = itInstances.value();

auto itObj = instances.begin();
auto itObj = instances3.begin();
// Can't use find here because epJSON keys are not upper-cased
for (; itObj != instances.end(); ++itObj) {
for (; itObj != instances3.end(); ++itObj) {
if (Util::makeUPPER(itObj.key()) == s_ipsc->cAlphaArgs(4)) break;
}

if (itObj == instances.end()) {
if (itObj == instances3.end()) {
ShowSevereItemNotFound(state, eoh, s_ipsc->cAlphaFieldNames(4), s_ipsc->cAlphaArgs(4));
ErrorsFound = true;
continue;
Expand Down Expand Up @@ -2668,45 +2668,12 @@ void GetMaterialData(EnergyPlusData &state, bool &ErrorsFound) // set to true if
}

if ((mat->LayerType == TARCOGParams::TARCOGLayerType::VENETBLIND_HORIZ) ||
(mat->LayerType == TARCOGParams::TARCOGLayerType::VENETBLIND_HORIZ)) {
if (s_ipsc->rNumericArgs(11) <= 0.0) {
ErrorsFound = true;
ShowSevereCustomMessage(
state, eoh, format("{} must be >0, entered value = {:.2R}", s_ipsc->cNumericFieldNames(11), s_ipsc->rNumericArgs(11)));
}

if (s_ipsc->rNumericArgs(12) <= 0.0) {
ErrorsFound = true;
ShowSevereCustomMessage(
state, eoh, format("{} must be >0, entered value = {:.2R}", s_ipsc->cNumericFieldNames(12), s_ipsc->rNumericArgs(12)));
}

if (s_ipsc->rNumericArgs(13) <= 0.0) {
ErrorsFound = true;
ShowSevereCustomMessage(
state, eoh, format("{} must be >0, entered value = {:.2R}", s_ipsc->cNumericFieldNames(13), s_ipsc->rNumericArgs(13)));
}

if ((s_ipsc->rNumericArgs(14) < -90.0) || (s_ipsc->rNumericArgs(14) > 90.0)) {
ErrorsFound = true;
ShowSevereCustomMessage(
state,
eoh,
format("{} must be >=-90 and <=90, entered value = {:.2R}", s_ipsc->cNumericFieldNames(14), s_ipsc->rNumericArgs(14)));
}

if (s_ipsc->rNumericArgs(15) <= 0.0) {
ErrorsFound = true;
ShowSevereCustomMessage(
state, eoh, format("{} must be >0, entered value = {:.2R}", s_ipsc->cNumericFieldNames(15), s_ipsc->rNumericArgs(15)));
}

if ((s_ipsc->rNumericArgs(16) < 0.0) ||
((s_ipsc->rNumericArgs(16) > 0.0) && (s_ipsc->rNumericArgs(16) < (s_ipsc->rNumericArgs(11) / 2)))) {
(mat->LayerType == TARCOGParams::TARCOGLayerType::VENETBLIND_VERT)) {
if ((s_ipsc->rNumericArgs(16) > 0.0) && (s_ipsc->rNumericArgs(16) < (s_ipsc->rNumericArgs(11) / 2))) {
Copy link
Contributor Author

@rraustad rraustad Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

[src/EnergyPlus/Material.cc:2670]:(style),[duplicateExpression],Same expression on both sides of '||'.

These other checks are caught by InputProcessor and are never executed. For example if I make slat thickness < 0 this input processor warning is reported.

** Severe  ** <root>[WindowMaterial:ComplexShade][Shade_14_Layer][slat_thickness] - "-0.001000" - Expected number greater than 0.000000
**  Fatal  ** Errors occurred on processing input file. Preceding condition(s) cause termination.

WindowMaterial:ComplexShade,

 N11 , \field Slat Width
     \units m
     \type real
     \minimum> 0
     \default 0.016
 N12 , \field Slat Spacing
     \note Distance between adjacent slat faces
     \units m
     \type real
     \minimum> 0
     \default 0.012
 N13 , \field Slat Thickness
     \note Distance between top and bottom surfaces of slat
     \note Slat is assumed to be rectangular in cross section and flat
     \type real
     \units m
     \minimum> 0
     \default 0.0006
 N14 , \field Slat Angle
     \type real
     \units deg
     \default 90
     \minimum -90
     \maximum 90
 N15 , \field Slat Conductivity
     \type real
     \units W/m-K
     \minimum> 0
     \default 160.0
 N16 ; \field Slat Curve
     \note this value represents curvature radius of the slat.
     \note if the slat is flat use zero.
     \note if this value is not zero, then it must be > SlatWidth/2.
     \type real
     \units m
     \minimum 0.0
     \default 0

Copy link
Member

Choose a reason for hiding this comment

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

Yes! There is so much duplicated between the IDD/input processor range checks and then inaccessible checks in the get input routines. I love it.

ErrorsFound = true;
ShowSevereCustomMessage(state,
eoh,
format("{} must be =0 or greater than SlatWidth/2, entered value = {:.2R}",
format("{} must be = 0 or greater than SlatWidth/2, entered value = {:.2R}",
s_ipsc->cNumericFieldNames(16),
s_ipsc->rNumericArgs(16)));
}
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/Material.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ namespace Material {
~MaterialGlassTC() = default;
};

int GetMaterialNum(EnergyPlusData &state, std::string const &matName);
int GetMaterialNum(EnergyPlusData const &state, std::string const &matName);
MaterialBase *GetMaterial(EnergyPlusData &state, std::string const &matName);

void GetMaterialData(EnergyPlusData &state, bool &errorsFound); // set to true if errors found in input
Expand Down
Loading