Correction of Defect in Transition Program and Recent Bug Fix #8997

RKStrand · 2021-08-23T16:50:59Z

Pull request overview

Fixes AIRLOOPHVAC:OUTDOORAIRSYSTEM transitions for the 3-line cases #8995
Fixes Sizing:Zone below-zero (degree C) Zone Cooling Design Supply Air Temperature scenarios for freezer spaces #8968
Minor issues with two recent defects. In AIRLOOPHVAC:OUTDOORAIRSYSTEM transitions for the 3-line cases #8995, the transition program was truncating valid input for the AirLoopHVAC:OutdoorAirSystem. The old input (pre-v9.6) had four fields. The last field was never used and thus removed in the previous defect fix. However, the transition program changes ended up with a truncated input line under certain conditions. This is now fixed. In Sizing:Zone below-zero (degree C) Zone Cooling Design Supply Air Temperature scenarios for freezer spaces #8968, there were a few minor bugs that didn't get noticed before merging. These are also fixed with this PR.
Fixes error introduced for transitions older than v9.6 (introduced in Add Space Concept to EnergyPlus Zone Structure, Part 1 #8394 )

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

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.

Correction to a problem in the transition that caused input files to be truncated inappropriately. Also, included another minor fix from a different defect resolution that got merged into develop already.

jcyuan2020 · 2021-08-23T17:53:43Z

src/Transition/CreateNewIDFUsingRulesV9_6_0.f90

@@ -397,7 +397,9 @@ SUBROUTINE CreateNewIDFUsingRules(EndOfFile,DiffOnly,InLfn,AskForInput,InputFile
                  CALL GetNewObjectDefInIDD(ObjectName,NwNumArgs,NwAorN,NwReqFld,NwObjMinFlds,NwFldNames,NwFldDefaults,NwFldUnits)
                  nodiff=.false.
                  OutArgs(1:3)=InArgs(1:3)
-                  CurArgs = CurArgs - 1
+                  IF (CurArgs == 4) THEN
+                      CurArgs = 3


That looks good! I tested again on both a 4-line case and a 3-line case, it works fine on both cases.

mjwitte

Small changes, looks good on paper. When CI comes back green, will merge.

…pTransitionAirLoopHVACOutdoorAirSystem

mjwitte · 2021-08-23T21:02:39Z

So, I realized that the transition changes to VCompareGlobalRoutines.f90 in the Space branch broke any transition prior to v9.6 when writing surface objects with grouped veritces (x,y,z on the same idf line) which is just about always. I added a fix to this branch to set the field numbers based on whether the transition version is <9.6. @jcyuan2020 @Myoldmopar I've tested locally, but a few random older transitions should be tested as well as the v9.6 transition to be sure this is all good.

jcyuan2020 · 2021-08-23T21:31:58Z

src/EnergyPlus/SizingManager.cc

                    ReportTemperatureInputError(
-                        state, cCurrentModuleObject, 1, state.dataSize->ZoneSizingInput(ZoneSizIndex).CoolDesTemp, true, ErrorsFound);
+                        state, cCurrentModuleObject, 3, state.dataSize->ZoneSizingInput(ZoneSizIndex).CoolDesTemp, true, ErrorsFound);
                    state.dataSize->ZoneSizingInput(ZoneSizIndex).HeatDesTemp = state.dataIPShortCut->rNumericArgs(3);


This is also been tested to working using a defect file that contains a -20C supply temperature in a Sizing:Zone object.

In a quick summary it is working as expected and fixed the problem well---and actually in this defect file case, even though the Sizing:Zone has an input value of -20C for supply temperature, it should not cause a warning in the defect file as the defect file as the object actually specifies a "temperature difference" method---and this should also add to the point when it is a bug before. After the fix the problem is fixed; and no warning for the defect file on this zone (because it uses the temperature difference method):

A little bit complication about this test file is that with the difference method, the user specified a 11.11C temperature difference in Sizing:Zone and the zone thermostat is set to 2C---this means that the supply is around -9.11. I original thought that this is the reason why the warning is flagged.--but actually it should not be. According to the code's original intention, it should only flag the below zero supply temperature with SupplyAirTemperature method. Therefore from this point it is also a bug.

I think the new fix nice fixed this and it is now clear that the warning is given only when a below-zero supply air temperature is inputted together with the "SupplyAirTemperature" method. If a user selects the "temperature difference", then even though a negative supply air temperature is inferred from the inputs, it would not (and also probably should not) give a warning.

jcyuan2020 · 2021-08-23T21:34:27Z

@mjwitte I will try a few cases on the space-related new transition changes next.

jcyuan2020 · 2021-08-23T22:42:38Z

For the space-related transitions, I tried a few more cases with 9.4 version and it is confirmed working. Without the fix, it will have a long list of warnings complaining the geometry fields in the transition audit file when going to the point from 9.5->9.6---for example, like this:

Conversion 9.5 => 9.6
 Starting new Transition.audit
 Will create new full IDFs
 Will create new IDF lines with units where applicable
 Will create new IDF lines leaving blank incoming fields as blank (no default fill)
 Processing IDF -- C:\EnergyPlus\_Code_Review\PR_8995_Transition_Fixes\Upgrade_Test_develop\5ZoneAirCooled.idf
   ** Warning ** Invalid Number in Numeric Field#4 (Vertex 1 Y-coordinate), value=BuildingSurface:Detailed, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#5 (Vertex 1 Z-coordinate), value=WALL-1PR, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#6 (Vertex 2 X-coordinate), value=WALL, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#7 (Vertex 2 Y-coordinate), value=WALL-1, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#8 (Vertex 2 Z-coordinate), value=PLENUM-1, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#9 (Vertex 3 X-coordinate), value=Outdoors, in BuildingSurface:Detailed=WALL-1PF
   ** Severe  ** Error detected in Object=BuildingSurface:Detailed, name=WALL-1PF
   **   ~~~   ** Field [Vertex 3 Y-coordinate] is required but was blank
   ** Warning ** Invalid Number in Numeric Field#11 (Vertex 3 Z-coordinate), value=SunExposed, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#12 (Vertex 4 X-coordinate), value=WindExposed, in BuildingSurface:Detailed=WALL-1PF
   ** Warning ** Invalid Number in Numeric Field#4 (Vertex 1 Y-coordinate), value=BuildingSurface:Detailed, in BuildingSurface:Detailed=WALL-1PL
   ** Warning ** Invalid Number in Numeric Field#5 (Vertex 1 Z-coordinate), value=TOP-1, in BuildingSurface:Detailed=WALL-1PL
   ** Warning ** Invalid Number in Numeric Field#6 (Vertex 2 X-coordinate), value=ROOF, in BuildingSurface:Detailed=WALL-1PL
   ** Warning ** Invalid Number in Numeric Field#7 (Vertex 2 Y-coordinate), value=ROOF-1, in BuildingSurface:Detailed=WALL-1PL
   ** Warning ** Invalid Number in Numeric Field#8 (Vertex 2 Z-coordinate), value=PLENUM-1, in BuildingSurface:Detailed=WALL-1PL
   ** Warning ** Invalid Number in Numeric Field#9 (Vertex 3 X-coordinate), value=Outdoors, in BuildingSurface:Detailed=WALL-1PL
   ** Severe  ** Error detected in Object=BuildingSurface:Detailed, name=WALL-1PL
   **   ~~~   ** Field [Vertex 3 Y-coordinate] is required but was blank
   ** Warning ** Invalid Number in Numeric Field#11 (Vertex 3 Z-coordinate), value=SunExposed, in BuildingSurface:Detailed=WALL-1PL
   ** Warning ** Invalid Number in Numeric Field#12 (Vertex 4 X-coordinate), value=WindExposed, in BuildingSurface:Detailed=WALL-1PL
...

Now with the fix, the transition results and audit reports will just come out clean:

Conversion 9.5 => 9.6
 Starting new Transition.audit
 Will create new full IDFs
 Will create new IDF lines with units where applicable
 Will create new IDF lines leaving blank incoming fields as blank (no default fill)
 Processing IDF -- C:\Users\jinch\Dropbox\Wk_GARD\EnergyPlus_Compile\_Code_Review\PR_8995_Transition_Fixes\Upgrade_Tests_MJW_8995\5ZoneAirCooled.idf
 Processing rvi -- C:\Users\jinch\Dropbox\Wk_GARD\EnergyPlus_Compile\_Code_Review\PR_8995_Transition_Fixes\Upgrade_Tests_MJW_8995\5ZoneAirCooled.rvi

Myoldmopar · 2021-08-31T19:21:45Z

I haven't looked at the diffs, but generally this seems fine. Anything else needed here outside of a bit more testing?

mjwitte · 2021-08-31T19:23:06Z

@Myoldmopar Nothing more that I know of.

mjwitte · 2021-08-31T19:25:02Z

I wouldn't expect any diffs here, so this may need a fresh regression to be sure.

…ionAirLoopHVACOutdoorAirSystem

Myoldmopar · 2021-08-31T19:28:23Z

Pulled develop in, we'll see how it does

rraustad · 2021-09-01T19:14:02Z

@mjwitte this PR is independent of #9019 and can be merged as is? with no other changes expected? Once reviewed and tested that is.

mjwitte · 2021-09-01T22:38:47Z

@rraustad Yes, this is separate from #9019.

Myoldmopar · 2021-09-02T00:34:01Z

Obviously CI looks much happier now. I will try to run a test this evening or in the morning and get this in.

rraustad · 2021-09-02T13:13:00Z

src/Transition/VCompareGlobalRoutines.f90

@@ -799,31 +808,32 @@ SUBROUTINE CheckSpecialObjects(DifUnit,ObjectName,CurArgs,OutArgs,FieldNames,Fie
          CALL WriteOutIDFLinesAsSingleLine(DifUnit,ObjectName,CurArgs,OutArgs,FieldNames,FieldUnits)

        CASE('BUILDINGSURFACE:DETAILED')
+          NVertFieldNum = 10 + SurfaceSpaceFieldShift
          IF (MakeUPPERCase(OutArgs(11)) == 'AUTOCALCULATE') THEN


What about this line? "11" has been replaced with NVertFieldNum and line 812 has OutArgs(11). For input files > V9.2 and < V9.6, this would be OutArgs(10), right? So OutArgs(NVertFieldNum) here as well.

Well, this does convert a V9.4 file to V9.5 so maybe this is OK.

@rraustad Good catch. I can fix shortly, or feel free to push a fix yourself.

Oh, it's when the number of vertices field is AutoCalculate when there is a problem. So this does need to change.

Try that same file with Number of Vertices = autocalculate.

I'm not sure, I was just updating an UnmetHours file and saw that. I actually think it was an ill formed input file. I guess I should also open the transitioned file in IDFEditor for this test as well. Can't hurt to check everything we can think of.

Transition worked but opening transitioned file in IDF Editor shows an error.

Here is the transitioned OA Sys object. This is V9.4 -> V9.5.

AirLoopHVAC:OutdoorAirSystem,
Outside Air System, !- Name
Outside Air System Controllers, !- Controller List Name
Outside Air System Equipment, !- Outdoor Air Equipment List Name
Evap Cooler Availability List; !- Availability Manager List Name

Sorry, was using V9.6 IDF Editor.

OK, the above issue is resolved using V9.5 IDF Editor and the transition above for V9.5 AirLoopHVAC:OutdoorAirSystem is correct.

I can't think of anything else to test so am committing this change.

…5CleanUpTransitionAirLoopHVACOutdoorAirSystem

mjwitte · 2021-09-03T16:31:31Z

@rraustad Anything left to do here?

rraustad · 2021-09-03T16:46:03Z

Well, I still see the ConvectionAdaptiveSmallOffice issue on this branch. I guess we could assume that is not related and get this merged.

mjwitte · 2021-09-03T16:52:22Z

Hmm, the changes in this branch only show transition now. @RKStrand Looks like those changes already merged with #9026 ? Please confirm.
those changes --> other Energyplus changes

RKStrand · 2021-09-03T16:58:01Z

@mjwitte Yes, the changes to EnergyPlus were already merged with #9026. This PR is only the correction of the transition program.

RKStrand added 2 commits August 23, 2021 10:33

Transition Cleanup with Other Bug Fix

51a36d0

Correction to a problem in the transition that caused input files to be truncated inappropriately. Also, included another minor fix from a different defect resolution that got merged into develop already.

Transition Fix Attempt 2

cdf84fa

RKStrand added the Defect Includes code to repair a defect in EnergyPlus label Aug 23, 2021

RKStrand requested review from Myoldmopar, mjwitte and jcyuan2020 August 23, 2021 16:51

RKStrand self-assigned this Aug 23, 2021

jcyuan2020 reviewed Aug 23, 2021

View reviewed changes

mjwitte approved these changes Aug 23, 2021

View reviewed changes

mjwitte added 2 commits August 23, 2021 15:58

Fix transition blunder for versions <9.6

36030a6

Merge remote-tracking branch 'remotes/origin/develop' into 8995CleanU…

8f03a59

…pTransitionAirLoopHVACOutdoorAirSystem

jcyuan2020 reviewed Aug 23, 2021

View reviewed changes

mjwitte added this to the EnergyPlus 9.6 Release milestone Aug 27, 2021

Merge remote-tracking branch 'origin/develop' into 8995CleanUpTransit…

bae9d01

…ionAirLoopHVACOutdoorAirSystem

rraustad mentioned this pull request Sep 1, 2021

Fix various HVACTemplate issues #9019

Merged

20 tasks

rraustad reviewed Sep 2, 2021

View reviewed changes

rraustad added 2 commits September 2, 2021 10:14

One other update to transition program.

c1cc6d2

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 899…

f9c7262

…5CleanUpTransitionAirLoopHVACOutdoorAirSystem

mjwitte merged commit 1d31b0b into develop Sep 3, 2021

mjwitte deleted the 8995CleanUpTransitionAirLoopHVACOutdoorAirSystem branch September 3, 2021 16:59

hongyuanjia mentioned this pull request Nov 7, 2021

PR #8997 breaks transitions for Wall:Detailed, etc for version < 9.6 #9172

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correction of Defect in Transition Program and Recent Bug Fix #8997

Correction of Defect in Transition Program and Recent Bug Fix #8997

RKStrand commented Aug 23, 2021 •

edited by mjwitte

Loading

jcyuan2020 Aug 23, 2021

mjwitte left a comment

mjwitte commented Aug 23, 2021

jcyuan2020 Aug 23, 2021

jcyuan2020 commented Aug 23, 2021

jcyuan2020 commented Aug 23, 2021 •

edited

Loading

Myoldmopar commented Aug 31, 2021

mjwitte commented Aug 31, 2021

mjwitte commented Aug 31, 2021

Myoldmopar commented Aug 31, 2021

rraustad commented Sep 1, 2021

mjwitte commented Sep 1, 2021

Myoldmopar commented Sep 2, 2021

rraustad Sep 2, 2021

rraustad Sep 2, 2021

mjwitte Sep 2, 2021

rraustad Sep 2, 2021

mjwitte Sep 2, 2021

rraustad Sep 2, 2021

rraustad Sep 2, 2021

rraustad Sep 2, 2021

rraustad Sep 2, 2021

rraustad Sep 2, 2021

mjwitte commented Sep 3, 2021

rraustad commented Sep 3, 2021 •

edited

Loading

mjwitte commented Sep 3, 2021 •

edited

Loading

RKStrand commented Sep 3, 2021

Correction of Defect in Transition Program and Recent Bug Fix #8997

Correction of Defect in Transition Program and Recent Bug Fix #8997

Conversation

RKStrand commented Aug 23, 2021 • edited by mjwitte Loading

Pull request overview

Pull Request Author

Reviewer

Choose a reason for hiding this comment

mjwitte left a comment

Choose a reason for hiding this comment

mjwitte commented Aug 23, 2021

Choose a reason for hiding this comment

jcyuan2020 commented Aug 23, 2021

jcyuan2020 commented Aug 23, 2021 • edited Loading

Myoldmopar commented Aug 31, 2021

mjwitte commented Aug 31, 2021

mjwitte commented Aug 31, 2021

Myoldmopar commented Aug 31, 2021

rraustad commented Sep 1, 2021

mjwitte commented Sep 1, 2021

Myoldmopar commented Sep 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjwitte commented Sep 3, 2021

rraustad commented Sep 3, 2021 • edited Loading

mjwitte commented Sep 3, 2021 • edited Loading

RKStrand commented Sep 3, 2021

RKStrand commented Aug 23, 2021 •

edited by mjwitte

Loading

jcyuan2020 commented Aug 23, 2021 •

edited

Loading

rraustad commented Sep 3, 2021 •

edited

Loading

mjwitte commented Sep 3, 2021 •

edited

Loading