-
Notifications
You must be signed in to change notification settings - Fork 389
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
Fix #10624 - Fix Transition from 23.2.0 to 24.2.0 when a HeatExchanger:AirToAir:SensibleAndLatent has blank effectiveness fields #10639
Conversation
…that when retrieving HX effectivenesses This will effectively set the values to 0.0 if blank, since that's what the IDD default is for these fields.
…ariable` were always written
Comparing
FillHXBlanks_V2320.idfNotice the useless Table:IndependentVariable and Table:IndependentVariableList in develop but not this PR NoBlanksDifferentEffectiveness_V2320.idfWithHXBlanks_V2320.idfdevelop segfaults WithHXBlanks_Different_Effectiveness_V2320.idfdevelop segfaults |
FUNCTION GetFieldOrIDDDefault(InArgString, FldDefaultString) RESULT (ResultString) | ||
|
||
! PURPOSE OF THIS SUBROUTINE: | ||
! If InArgString is Blank, replace with FldDefaultString | ||
|
||
IMPLICIT NONE ! Enforce explicit typing of all variables in this routine | ||
|
||
|
||
! FUNCTION ARGUMENT DEFINITIONS: | ||
CHARACTER(len=*), INTENT(IN) :: InArgString ! Input String | ||
CHARACTER(len=*), INTENT(IN) :: FldDefaultString ! Default String | ||
CHARACTER(len=LEN(InArgString)) :: ResultString ! Result String | ||
|
||
ResultString=InArgString | ||
IF (ResultString == Blank) THEN | ||
ResultString = FldDefaultString | ||
ENDIF | ||
|
||
RETURN | ||
|
||
END FUNCTION GetFieldOrIDDDefault |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New helper
! Sensible Effectiveness at 75% Heating Air Flow | ||
HxEffectAt75Airflow(1) = GetFieldOrIDDDefault(InArgs(6), FldDefaults(6)) | ||
! Latent Effectiveness at 75% Heating Air Flow | ||
HxEffectAt75Airflow(2) = GetFieldOrIDDDefault(InArgs(7), FldDefaults(7)) | ||
! Sensible Effectiveness at 75% Cooling Air Flow | ||
HxEffectAt75Airflow(3) = GetFieldOrIDDDefault(InArgs(10), FldDefaults(10)) | ||
! Latent Effectiveness at 75% Cooling Air Flow | ||
HxEffectAt75Airflow(4) = GetFieldOrIDDDefault(InArgs(11), FldDefaults(11)) | ||
! Sensible Effectiveness at 100% Heating Air Flow | ||
HxEffectAt100Airflow(1) = GetFieldOrIDDDefault(InArgs(4), FldDefaults(4)) | ||
! Latent Effectiveness at 100% Heating Air Flow | ||
HxEffectAt100Airflow(2) = GetFieldOrIDDDefault(InArgs(5), FldDefaults(5)) | ||
! Sensible Effectiveness at 100% Cooling Air Flow | ||
HxEffectAt100Airflow(3) = GetFieldOrIDDDefault(InArgs(8), FldDefaults(8)) | ||
! Latent Effectiveness at 100% Cooling Air Flow | ||
HxEffectAt100Airflow(4) = GetFieldOrIDDDefault(InArgs(9), FldDefaults(9)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use it.
@@ -138,7 +138,7 @@ SUBROUTINE CreateNewIDFUsingRules(EndOfFile,DiffOnly,InLfn,AskForInput,InputFile | |||
CHARACTER(20), DIMENSION(4) :: HxEffectAt75Airflow | |||
CHARACTER(20), DIMENSION(4) :: HxEffectAt100Airflow | |||
CHARACTER(MaxNameLength + 2), DIMENSION(4) :: HxTableName | |||
LOGICAL :: tableAdded | |||
LOGICAL :: tableAdded = .false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid writting Table:IndependentVariable(List) when not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems cool, I've never paid attention to the FldDefaults before, but it looks look a great use of it. And it's just one of the args passed to GetObjectDefInIDD, so it should be safe. This is good stuff.
I'll do a quick test but this should merge soon. @mjwitte am I missing anything here?
! PURPOSE OF THIS SUBROUTINE: | ||
! If InArgString is Blank, replace with FldDefaultString | ||
|
||
IMPLICIT NONE ! Enforce explicit typing of all variables in this routine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implicit None is already declared at the top of the module, so probably not necessary, but totally fine.
HxEffectAt100Airflow(3) = TRIM(InArgs(8)) ! Sensible Effectiveness at 100% Cooling Air Flow | ||
HxEffectAt100Airflow(4) = TRIM(InArgs(9)) ! Latent Effectiveness at 100% Cooling Air Flow | ||
! Sensible Effectiveness at 75% Heating Air Flow | ||
HxEffectAt75Airflow(1) = GetFieldOrIDDDefault(InArgs(6), FldDefaults(6)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So FldDefaults will always be populated with meaningful or blank values? I've never used that array so just confirming.
@jmarrec @Myoldmopar This works. Nothing to add. |
Awesome, merging this. Thanks @jmarrec |
Pull request overview
GetFieldOrIDDDefault
and using that when retrieving HX effectiveness fieldsPull 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.