Skip to content

Conversation

@mjwitte
Copy link
Contributor

@mjwitte mjwitte commented May 6, 2025

Pull request overview

  • Fixes DesignSpecification:OutdoorAir:SpaceList does not work with Controller:MechanicalVentilation #11018
  • Refactors Controller:MechanicalVentilation to use methods on OARequirements (DesignSpecification:OutdoorAir) instead of copying single values from OARequirements into the VentilationMechanical data structure. This makes support of DesignSpecification:OutdoorAir:SpaceList possible.
  • Modify table output for System Summary report, "Demand Controlled Ventilation using Controller:MechanicalVentilation" subtable to use ZoneName for zones with a simple DSOA reference, and ZoneName:SpaceName for the spaces in a DSOA:SpaceList.

Pull Request Author

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label May 6, 2025
@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 3761e8b

Regression Summary
  • Audit: 8
  • EIO: 8

@mjwitte mjwitte changed the title Fix Controller:MechanicalVentilation with DesingSpecification:OutdoorAir:SpaceList Fix Controller:MechanicalVentilation with DesignSpecification:OutdoorAir:SpaceList May 16, 2025
@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 41cf1a6

Regression Summary
  • Audit: 8
  • EIO: 8

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 27e1248

Regression Summary
  • Audit: 26

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 36d9666

Regression Summary
  • Audit: 26

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 8fdaeea

Regression Summary
  • Audit: 26

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 6a3309f

Regression Summary
  • Audit: 26

Comment on lines 2738 to 2739
for (int n = 0; n <= dsoa.numDSOA; ++n) {
std::string const zsName = dsoa.numDSOA > 0 ? format("{}:{}", zoneName, dsoa.dsoaSpaceNames[n]) : zoneName;
Copy link
Contributor

Choose a reason for hiding this comment

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

@mjwitte I'm trying this PR on a file (3.10-alpha-4219-DSOA.idf) that @mdahlhausen provided here: NREL/OpenStudio#5384 (comment)

Hitting an EPVector assert on this line here.

Initializing Simulation
terminate called after throwing an instance of 'std::out_of_range'
  what():  vector::_M_range_check: __n (which is 2) >= this->size() (which is 2)
Process 462089 stopped
* thread #1, name = 'energyplus', stop reason = signal SIGABRT
    frame #0: 0x00007fffe7a9eb2c libc.so.6`__GI___pthread_kill [inlined] __pthread_kill_implementation(no_tid=0, signo=6, threadid=<unavailable>) at pthread_kill.c:44:76


(lldb) backtrace
error: libenergyplusapi.so.25.2.0 0x04bae016: DW_TAG_member '_M_local_buf' refers to type 0x0000000004c9831f which extends beyond the bounds of 0x04bae00c
* thread #1, name = 'energyplus', stop reason = signal SIGABRT
  * frame #0: 0x00007fffe7a9eb2c libc.so.6`__GI___pthread_kill [inlined] __pthread_kill_implementation(no_tid=0, signo=6, threadid=<unavailable>) at pthread_kill.c:44:76
    frame #1: 0x00007fffe7a9eae8 libc.so.6`__GI___pthread_kill [inlined] __pthread_kill_internal(signo=6, threadid=<unavailable>) at pthread_kill.c:78:10
    frame #2: 0x00007fffe7a9eae8 libc.so.6`__GI___pthread_kill(threadid=<unavailable>, signo=6) at pthread_kill.c:89:10
    frame #3: 0x00007fffe7a4527e libc.so.6`__GI_raise(sig=6) at raise.c:26:13
    frame #4: 0x00007fffe7a288ff libc.so.6`__GI_abort at abort.c:79:7
    frame #5: 0x00007fffe7ea5ff5 libstdc++.so.6`__gnu_cxx::__verbose_terminate_handler() (.cold) at vterminate.cc:95:10
    frame #6: 0x00007fffe7ebb0da libstdc++.so.6`__cxxabiv1::__terminate(void (*)()) at eh_terminate.cc:48:15
    frame #7: 0x00007fffe7ea5a55 libstdc++.so.6`std::terminate() at eh_terminate.cc:58:27
    frame #8: 0x00007fffe7ebb391 libstdc++.so.6`__cxxabiv1::__cxa_throw(obj=0x000055555a5c3290, tinfo=0x00007fffe806eea0, dest=(libstdc++.so.6`std::out_of_range::~out_of_range() at stdexcept.cc:65:3)) at eh_throw.cc:98:18
    frame #9: 0x00007fffe7ea94a0 libstdc++.so.6`std::__throw_out_of_range_fmt(__fmt="vector::_M_range_check: __n (which is %zu) >= this->size() (which is %zu)") at functexcept.cc:101:27
    frame #10: 0x00005555555fe0bf energyplus`std::__cxx1998::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>::_M_range_check(this=0x000055555882b210 size=0, __n=2) const at stl_vector.h:1158:28
    frame #11: 0x00007fffecad941b libenergyplusapi.so.25.2.0`std::__cxx1998::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>::at(this=0x000055555882b210 size=0, __n=2) at stl_vector.h:1180:16
    frame #12: 0x00007fffecad47b1 libenergyplusapi.so.25.2.0`EnergyPlus::EPVector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>::operator[](this=0x000055555882b1f8, n=2) at EPVector.hh:79:34
    frame #13: 0x00007fffee51a925 libenergyplusapi.so.25.2.0`EnergyPlus::MixedAir::InitOAController(state=0x00007fffffffa880, OAControllerNum=1, FirstHVACIteration=true, AirLoopNum=1) at MixedAir.cc:2739:119
    frame #14: 0x00007fffee4f289e libenergyplusapi.so.25.2.0`EnergyPlus::MixedAir::SimOAController(state=0x00007fffffffa880, CtrlName="BUILDING STORY 1 PVAV_REHEAT (SYS5) OA CONTROLLER", CtrlIndex=0x000055555a6dc6f0, FirstHVACIteration=true, AirLoopNum=1) at MixedAir.cc:837:21
    frame #15: 0x00007fffee4ebd4b libenergyplusapi.so.25.2.0`EnergyPlus::MixedAir::SimOutsideAirSys(state=0x00007fffffffa880, OASysNum=1, FirstHVACIteration=true, AirLoopNum=1) at MixedAir.cc:355:24
    frame #16: 0x00007fffee4ead4a libenergyplusapi.so.25.2.0`EnergyPlus::MixedAir::ManageOutsideAirSystem(state=0x00007fffffffa880, OASysName="BUILDING STORY 1 PVAV_REHEAT (SYS5) OA SYSTEM", FirstHVACIteration=true, AirLoopNum=1, OASysNum=0x000055555a6f8604) at MixedAir.cc:272:21
    frame #17: 0x00007fffeeb2e31e libenergyplusapi.so.25.2.0`EnergyPlus::SimAirServingZones::SimAirLoopComponent(state=0x00007fffffffa880, CompName="BUILDING STORY 1 PVAV_REHEAT (SYS5) OA SYSTEM", CompType_Num=OAMixer_Num, FirstHVACIteration=true, AirLoopNum=1, CompIndex=0x000055555a6f8604, CompPointer=0x0000000000000000, airLoopNum=1, branchNum=1, compNum=1) at SimAirServingZones.cc:3437:31
    frame #18: 0x00007fffeeb2e0b7 libenergyplusapi.so.25.2.0`EnergyPlus::SimAirServingZones::SimAirLoopComponents(state=0x00007fffffffa880, AirLoopNum=1, FirstHVACIteration=true) at SimAirServingZones.cc:3360:32
    frame #19: 0x00007fffeeb2934c libenergyplusapi.so.25.2.0`EnergyPlus::SimAirServingZones::SolveAirLoopControllers(state=0x00007fffffffa880, FirstHVACIteration=true, AirLoopNum=1, AirLoopConvergedFlag=0x0000555556a4fcc8, IterMax=0x0000555556a4fcbc, IterTot=0x0000555556a4fcc0, NumCalls=0x0000555556a4fcc4) at SimAirServingZones.cc:2847:83
    frame #20: 0x00007fffeeb286ef libenergyplusapi.so.25.2.0`EnergyPlus::SimAirServingZones::SimAirLoop(state=0x00007fffffffa880, FirstHVACIteration=true, AirLoopNum=1, AirLoopPass=1, AirLoopIterMax=0x00007fffffff93dc, AirLoopIterTot=0x00007fffffff93e0, AirLoopNumCalls=0x00007fffffff93e4) at SimAirServingZones.cc:2699:32
    frame #21: 0x00007fffeeb2716e libenergyplusapi.so.25.2.0`EnergyPlus::SimAirServingZones::SimAirLoops(state=0x00007fffffffa880, FirstHVACIteration=true, SimZoneEquipment=0x0000555555710c3b) at SimAirServingZones.cc:2504:23
    frame #22: 0x00007fffeeaffa54 libenergyplusapi.so.25.2.0`EnergyPlus::SimAirServingZones::ManageAirLoops(state=0x00007fffffffa880, FirstHVACIteration=true, SimAir=0x0000555555710c38, SimZoneEquipment=0x0000555555710c3b) at SimAirServingZones.cc:174:20
    frame #23: 0x00007fffedd90ffd libenergyplusapi.so.25.2.0`EnergyPlus::HVACManager::SimSelectedEquipment(state=0x00007fffffffa880, SimAirLoops=0x0000555555710c38, SimZoneEquipment=0x0000555555710c3b, SimNonZoneEquipment=0x0000555555710c3c, SimPlantLoops=0x0000555555710c3a, SimElecCircuits=0x0000555555710c39, FirstHVACIteration=0x00007fffffff9645, LockPlantFlows=false) at HVACManager.cc:1786:43
    frame #24: 0x00007fffedd82a75 libenergyplusapi.so.25.2.0`EnergyPlus::HVACManager::SimHVAC(state=0x00007fffffffa880) at HVACManager.cc:826:25
    frame #25: 0x00007fffedd79e4f libenergyplusapi.so.25.2.0`EnergyPlus::HVACManager::ManageHVAC(state=0x00007fffffffa880) at HVACManager.cc:272:12
    frame #26: 0x00007fffedfd9615 libenergyplusapi.so.25.2.0`EnergyPlus::HeatBalanceAirManager::CalcHeatBalanceAir(state=0x00007fffffffa880) at HeatBalanceAirManager.cc:4491:32
    frame #27: 0x00007fffedf8888a libenergyplusapi.so.25.2.0`EnergyPlus::HeatBalanceAirManager::ManageAirHeatBalance(state=0x00007fffffffa880) at HeatBalanceAirManager.cc:158:23
    frame #28: 0x00007fffee11c502 libenergyplusapi.so.25.2.0`EnergyPlus::HeatBalanceSurfaceManager::ManageSurfaceHeatBalance(state=0x00007fffffffa880) at HeatBalanceSurfaceManager.cc:171:48
    frame #29: 0x00007fffee0475d6 libenergyplusapi.so.25.2.0`EnergyPlus::HeatBalanceManager::ManageHeatBalance(state=0x00007fffffffa880) at HeatBalanceManager.cc:206:33
    frame #30: 0x00007fffec845dc7 libenergyplusapi.so.25.2.0`EnergyPlus::SimulationManager::SetupSimulation(state=0x00007fffffffa880, ErrorsFound=0x00007fffffffa5d3) at SimulationManager.cc:1903:30
    frame #31: 0x00007fffec827c90 libenergyplusapi.so.25.2.0`EnergyPlus::SimulationManager::ManageSimulation(state=0x00007fffffffa880) at SimulationManager.cc:270:24
    frame #32: 0x00007fffeb7aa460 libenergyplusapi.so.25.2.0`RunEnergyPlus(state=0x00007fffffffa880, filepath="") at EnergyPlusPgm.cc:421:56
    frame #33: 0x00007fffeb7a831f libenergyplusapi.so.25.2.0`EnergyPlusPgm(args=size=6, filepath="") at EnergyPlusPgm.cc:242:25
    frame #34: 0x00005555555abfb2 energyplus`main(argc=6, argv=0x00007fffffffbfa8) at main.cc:60:25
    frame #35: 0x00007fffe7a2a1ca libc.so.6`__libc_start_call_main(main=(energyplus`main at main.cc:54:1), argc=6, argv=0x00007fffffffbfa8) at libc_start_call_main.h:58:16
    frame #36: 0x00007fffe7a2a28b libc.so.6`__libc_start_main_impl(main=(energyplus`main at main.cc:54:1), argc=6, argv=0x00007fffffffbfa8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffbf98) at libc-start.c:360:3
    frame #37: 0x00005555555abd95 energyplus`_start + 37


(lldb) f 13
frame #13: 0x00007fffee51a925 libenergyplusapi.so.25.2.0`EnergyPlus::MixedAir::InitOAController(state=0x00007fffffffa880, OAControllerNum=1, FirstHVACIteration=true, AirLoopNum=1) at MixedAir.cc:2739:119
   2736	                auto &dsoa = state.dataSize->OARequirements(thisMechVentZone.ZoneDesignSpecOAObjIndex);
   2737	                // Loop through spaces if DesignSpecification:OutdoorAir:Spacelist, or just once for simple DSOA
   2738	                for (int n = 0; n <= dsoa.numDSOA; ++n) {
-> 2739	                    std::string const zsName = dsoa.numDSOA > 0 ? format("{}:{}", zoneName, dsoa.dsoaSpaceNames[n]) : zoneName;
   2740	                    auto &dsoa2 = dsoa.numDSOA > 0 ? state.dataSize->OARequirements(dsoa.dsoaIndexes[n]) : dsoa;
   2741	                    OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVventMechName, zsName, vent_mech.Name);
   2742	                    OutputReportPredefined::PreDefTableEntry(

Copy link
Contributor

@jmarrec jmarrec May 22, 2025

Choose a reason for hiding this comment

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

(lldb) p dsoa.numDSOA
(int) 2
(lldb) p dsoa.dsoaSpaceNames
(EnergyPlus::EPVector<std::basic_string<char> >) {
  std::__debug::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > = size=2 {
    [0] = "101N CORRIDOR"
    [1] = "118 CUSTODIAL"
  }
  m_allocated = false
}
(lldb) p n
(int) 2

Obviously this fails, dsoa.dsoaSpaceNames[n] is past the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is a MCVE I created:

  • develop: does not use the OutdoorAir:SpaceList, and runs
  • dsoa: uses the OutdoorAirSpaceList, and exhibits that EPVector issue

mcve_develop.idf.txt
mcve_dsoa.idf.txt

This is a shoebox model, I created 5 spaces in 3 thermal zones

{
  'Zone1_2': ['Space1', 'Space2'],
  'Zone3': ['Space3'],
  'Zone4_5': ['Space4', 'Space5'],
 }

Each Space is assigned a specific DesignSpecificationOutdoorAir

image

Zone1 is assigned to a PSZ-AC (Sys3)

image

Zone2 and Zone3 are assigned to PVAV (Sys5):

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I can reproduce with this even smaller example that keeps only the Zone1 (PSZ), and uses the same construction and no loads (800 lines)

super_mcve_develop.idf.txt
super_mcve_dsoa.idf.txt

Copy link
Contributor

Choose a reason for hiding this comment

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

I find it a bit scary that none of our unit tests or integration tests crash in debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find it a bit scary that none of our unit tests or integration tests crash in debug?

  • The defect file crashes with debug. It helps to run that.
  • The new unit test doesn't touch this code. I'll see if I can modify an existing one or move this code.
  • And none of the existing integration tests use MechVent with DSOA:Spacelist yet, working on that.

@Myoldmopar
Copy link
Member

Pulled in develop to make sure it's still happy with formatting changes.

@Myoldmopar
Copy link
Member

image

?? :) I'll take a look

@Myoldmopar
Copy link
Member

It's completely happy here. I may need to give the CI machine a reboot.

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 638af94

Regression Summary
  • Table Big Diffs: 4
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table String Diffs: 1

jmarrec pushed a commit to jmarrec/EnergyPlus that referenced this pull request May 26, 2025
…pecification:OutdoorAir:SpaceList

Delete unused MechVent totals

Add calcIAQMethods argument to calcDesignSpecificationOutdoorAir

Use calcDesignSpecificationOutdoorAir in MechVent - Part 1

Split up desFlow OA functions

Use calcDesignSpecificationOutdoorAir in MechVent - Part 2

Add new OARequirements functions

Trim copied values in VentilationMechanicalZoneProps - use DSOA directly

Fix diffs and other cleanup

Fix diffs again

Simplify OA functions and fix missing table rows

Fix unit test failures and other cleanup

Address comments

Add DSOA:SpaceList unit test

Fix DSOA:SpaceList unit test

Add DSOA:SpaceList to unit test

Use DSOA:SpaceList in Controller:MechVent

Fix EPVector out of range issue: good candidate for a lambda!

Output rules
@Myoldmopar
Copy link
Member

This will still have a few spurious table diffs, but hopefully Decent is clean and this can be ready to go in.

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit dcde949

Regression Summary
  • Table Big Diffs: 4
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table String Diffs: 1

@nrel-bot-2c
Copy link

@mjwitte @Myoldmopar it has been 34 days since this pull request was last updated.

@Myoldmopar
Copy link
Member

My local regressions are showing more table diffs than before. I'm going to push it up and let CI confirm my findings before investigating any further.

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 6d77e12

Regression Summary
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 1
  • Table String Diffs: 1

@mjwitte
Copy link
Contributor Author

mjwitte commented Jul 23, 2025

@Myoldmopar The diffs here look like they are back to normal.

Regression Summary
Audit: 27
EIO: 1
ERR: 1
RDD: 1
ESO Big Diffs: 1
MTR Big Diffs: 1
Table Big Diffs: 1
Table String Diffs: 1

@nrel-bot-2
Copy link

@mjwitte @Myoldmopar it has been 30 days since this pull request was last updated.

@nrel-bot-2
Copy link

@mjwitte @Myoldmopar it has been 33 days since this pull request was last updated.

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 2b16c3c

Regression Summary
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 1
  • Table String Diffs: 1

@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit 2b16c3c

Regression Summary
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 1
  • Table String Diffs: 1

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit ddb7e70

Regression Summary
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 1
  • Table String Diffs: 1

@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit ddb7e70

Regression Summary
  • Audit: 27
  • EIO: 1
  • ERR: 1
  • RDD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 1
  • Table String Diffs: 1

@mjwitte
Copy link
Contributor Author

mjwitte commented Sep 26, 2025

@mitchute Conflicts resolved, CI looks good so far. Ready for review.

Comment on lines -1152 to +1373
// TODO MJW: this looks like it's double-counting the multipliers
// TODO MJW: this looks like it's double-counting the multipliers - it *is* double counting for methods PCOccSch and PCDesOcc
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this worth cleaning up? May also make sure where it's being double counted is noted, and relevant reasoning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this worth cleaning up? May also make sure where it's being double counted is noted, and relevant reasoning.

How about a followup-PR? I see there's an open issue related to multipliers that I should address anyway.
See ##10799

Copy link
Collaborator

Choose a reason for hiding this comment

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

Works for me. Thanks!

@mitchute mitchute merged commit 1ece45c into develop Sep 26, 2025
11 checks passed
@mitchute mitchute deleted the DSOASpaceList branch September 26, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus OutputChange Code changes output in such a way that it cannot be merged after IO freeze Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DesignSpecification:OutdoorAir:SpaceList does not work with Controller:MechanicalVentilation

8 participants