Strange crash when removing Unitheater from thermalzone equipment list

jmarrec commented 4 months ago

Issue overview

I am extremely puzzled about what's happening but I can reproduce a crash originally reported at https://github.com/openstudiocoalition/OpenStudioApplication/issues/686 on windows (it works on Ubuntu).

Current Behavior

It seems that when the extensible group corresponding to the UH is deleted, something very strange happens.

In the following group, which points correctly to a ZoneVentilationDesignFlowRate object to begin with, the handle is changed to be the handle of that object's Schedule Name (?!)

Expected Behavior

Steps to Reproduce

I'll provide a shortened file asap: edit: Here is a mcve.osm:

mcve.osm.txt

C:\openstudio-3.7.0\bin\openstudio -e "m = OpenStudio::Model::Model::load('mcve.osm').get; uh = m.getZoneHVACUnitHeaters.first; uh.remove; puts 'ok'"
[BOOST_ASSERT] <2> Assertion eg failed on line 346 of bool __cdecl openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const class openstudio::model::ModelObject &) in file D:\OSN\src\model\ZoneHVACEquipmentList.cpp.
[BOOST_ASSERT] <2> Assertion eg failed on line 346 of bool __cdecl openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const class openstudio::model::ModelObject &) in file D:\OSN\src\model\ZoneHVACEquipmentList.cpp.
[BOOST_ASSERT] <2> Assertion eg failed on line 356 of bool __cdecl openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const class openstudio::model::ModelObject &) in file D:\OSN\src\model\ZoneHVACEquipmentList.cpp.
[BOOST_ASSERT] <2> Assertion eg failed on line 356 of bool __cdecl openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const class openstudio::model::ModelObject &) in file D:\OSN\src\model\ZoneHVACEquipmentList.cpp.
ok

m = OpenStudio::Model::Model::load('mcve.osm')
uh.remove

test2:

require 'openstudio'

include OpenStudio::Model

def getGroupForModelObject(eqlist, modelObject)
  eqlist.extensibleGroups.each do |eg|
    meg = eg.to_ModelExtensibleGroup.get
    if meg.getTarget(0).get.handle == modelObject.handle
      return meg
    end
  end
  return nil
end

m = OpenStudio::Model::Model::load('mcve.osm').get
eqlist = m.getZoneHVACEquipmentLists.first
uh = m.getZoneHVACUnitHeaters.first

puts "Eq List before"
pp eqlist.extensibleGroups.map{|eg| [eg.to_ModelExtensibleGroup.get.getTarget(0).get.nameString, eg.getUnsigned(1).get, eg.getUnsigned(2).get]}
puts eqlist

coolingVector = eqlist.equipmentInCoolingOrder
heatingVector = eqlist.equipmentInHeatingOrder

index = getGroupForModelObject(eqlist, uh).groupIndex
eqlist.eraseExtensibleGroup(index)
puts "Eq List After erasing extensible group #{index}"
pp eqlist.extensibleGroups.map{|eg| [eg.to_ModelExtensibleGroup.get.getTarget(0).get.nameString, eg.getUnsigned(1).get, eg.getUnsigned(2).get]}
puts eqlist
m.save('mcve_after_erase.osm', true)

heatingVector = heatingVector.reject{|x| x==uh}
coolingVector = coolingVector.reject{|x| x==uh}

priority = 1
coolingVector.each do |elem|
  eg = getGroupForModelObject(eqlist, elem)
  raise "Group is nil for #{elem.briefDescription}" if eg.nil?
  eg.setUnsigned(1, priority)
  priority += 1
end

priority = 1
heatingVector.each do |elem|
  eg = getGroupForModelObject(eqlist, elem)
  raise "Group is nil for #{elem.briefDescription}" if eg.nil?
  eg.setUnsigned(2, priority)
  priority += 1
end

puts "Eq List after"
pp eqlist.extensibleGroups.map{|eg| [eg.to_ModelExtensibleGroup.get.getTarget(0).get.nameString, eg.getUnsigned(1).get, eg.getUnsigned(2).get]}

On Linux, everything is fine. Comparing the mcve.osm and the mcve_after_erase.osm, I get what's expected:

Adding the WIndows one to the right, we see something weird happened, the handle for former second group and now first group is actually pointing to a Schedule

Eq List before
[["CV Unit Heater Gas", 1, 1],
 ["Natural Ventilation", 2, 2],
 ["Exhaust Ventilation 1", 3, 3]]
OS:ZoneHVAC:EquipmentList,
  {1fb73cb8-af9e-498c-835a-6ba8e9fc462d}, !- Handle
  Zone HVAC Equipment List 1,             !- Name
  {fc4c588a-3ee2-43bb-831f-3cb4edd4a60b}, !- Thermal Zone
  ,                                       !- Load Distribution Scheme
  {9ff16c2d-faa2-4235-8cd3-8c11ee784945}, !- Zone Equipment 1
  1,                                      !- Zone Equipment Cooling Sequence 1
  1,                                      !- Zone Equipment Heating or No-Load Sequence 1
  ,                                       !- Zone Equipment Sequential Cooling Fraction Schedule Name 1
  ,                                       !- Zone Equipment Sequential Heating Fraction Schedule Name 1
  {462cece8-1aa2-43b2-9c2b-b7988df19dd6}, !- Zone Equipment 2
  2,                                      !- Zone Equipment Cooling Sequence 2
  2,                                      !- Zone Equipment Heating or No-Load Sequence 2
  ,                                       !- Zone Equipment Sequential Cooling Fraction Schedule Name 2
  ,                                       !- Zone Equipment Sequential Heating Fraction Schedule Name 2
  {99ce599d-315f-4bd8-add2-726fcb42aaee}, !- Zone Equipment 3
  3,                                      !- Zone Equipment Cooling Sequence 3
  3,                                      !- Zone Equipment Heating or No-Load Sequence 3
  ,                                       !- Zone Equipment Sequential Cooling Fraction Schedule Name 3
  ;                                       !- Zone Equipment Sequential Heating Fraction Schedule Name 3

Eq List After erasing extensible group 0
[["Natural ventilation", 2, 2], ["Exhaust Ventilation 1", 3, 3]]
OS:ZoneHVAC:EquipmentList,
  {1fb73cb8-af9e-498c-835a-6ba8e9fc462d}, !- Handle
  Zone HVAC Equipment List 1,             !- Name
  {fc4c588a-3ee2-43bb-831f-3cb4edd4a60b}, !- Thermal Zone
  ,                                       !- Load Distribution Scheme
  {cbe1f37f-5b90-460b-9f75-c45094d6fb9e}, !- Zone Equipment 1
  2,                                      !- Zone Equipment Cooling Sequence 1
  2,                                      !- Zone Equipment Heating or No-Load Sequence 1
  ,                                       !- Zone Equipment Sequential Cooling Fraction Schedule Name 1
  ,                                       !- Zone Equipment Sequential Heating Fraction Schedule Name 1
  {99ce599d-315f-4bd8-add2-726fcb42aaee}, !- Zone Equipment 2
  3,                                      !- Zone Equipment Cooling Sequence 2
  3,                                      !- Zone Equipment Heating or No-Load Sequence 2
  ,                                       !- Zone Equipment Sequential Cooling Fraction Schedule Name 2
  ;                                       !- Zone Equipment Sequential Heating Fraction Schedule Name 2

Error: Group is nil for Object of type 'OS:ZoneVentilation:DesignFlowRate' and named 'Natural Ventilation'
Backtrace:
        C:/src/OpenStudioApplication-issues/os_app/686/test.rb:56:in `block in <top (required)>'
        C:/src/OpenStudioApplication-issues/os_app/686/test.rb:54:in `each'
        C:/src/OpenStudioApplication-issues/os_app/686/test.rb:54:in `<top (required)>'
        eval:139:in `require'
        eval:139:in `require'
        :/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require'
        eval:4:in `<main>'
Failed to execute 'C:/src/OpenStudioApplication-issues/os_app/686/test.rb'

Possible Solution

Details

Environment

Some additional details about your environment for this issue (if relevant):

Platform (Operating system, version): Windows
Version of OpenStudio (if using an intermediate build, include SHA): 3.7.0

Context

https://github.com/openstudiocoalition/OpenStudioApplication/issues/686

jmarrec commented 4 months ago

Oh well well well. The schedule is named exactly (casing aside) like the ZoneVentilationDesignFlowRate object... I found that's the precondition that makes me also reproduce on linux.

test_scratch.rb:

require 'openstudio'

include OpenStudio::Model

m = Model.new

z = ThermalZone.new(m)

uh = ZoneHVACUnitHeater.new(m, m.alwaysOnDiscreteSchedule, FanOnOff.new(m), CoilHeatingElectric.new(m))
nv = ZoneVentilationDesignFlowRate.new(m)
nv.setName("ZVDFR NaturalVentilation")
nv.setVentilationType("Natural")
nv_sch = ScheduleRuleset.new(m)
nv.setSchedule(nv_sch)
nv_sch.setName(nv.nameString)      # <-------- comment out this line and it works fine

exhaust = ZoneVentilationDesignFlowRate.new(m)
exhaust.setVentilationType("Exhaust")
exhaust_sch = ScheduleRuleset.new(m)
exhaust.setSchedule(exhaust_sch)
#exhaust_sch.setName(exhaust.nameString)

uh.addToThermalZone(z)
nv.addToThermalZone(z)
exhaust.addToThermalZone(z)

uh.remove

On ubuntu:

$ ruby test_scratch.rb 
[BOOST_ASSERT] <2> Assertion eg failed on line 346 of bool openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const openstudio::model::ModelObject&) in file /srv/jenkins/openstudio/git/nightly/ubuntu_2004/src/model/ZoneHVACEquipmentList.cpp.
[BOOST_ASSERT] <2> Assertion eg failed on line 346 of bool openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const openstudio::model::ModelObject&) in file /srv/jenkins/openstudio/git/nightly/ubuntu_2004/src/model/ZoneHVACEquipmentList.cpp.
[BOOST_ASSERT] <2> Assertion eg failed on line 356 of bool openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const openstudio::model::ModelObject&) in file /srv/jenkins/openstudio/git/nightly/ubuntu_2004/src/model/ZoneHVACEquipmentList.cpp.
[BOOST_ASSERT] <2> Assertion eg failed on line 356 of bool openstudio::model::detail::ZoneHVACEquipmentList_Impl::removeEquipment(const openstudio::model::ModelObject&) in file /srv/jenkins/openstudio/git/nightly/ubuntu_2004/src/model/ZoneHVACEquipmentList.cpp.

jmarrec commented 4 months ago

As long as grabbing by name returns the schedule first, I can reproduce the crash

m = Model.new

z = ThermalZone.new(m)

bb_delete = ZoneHVACBaseboardConvectiveElectric.new(m)

bb = ZoneHVACBaseboardConvectiveElectric.new(m)
bb_sch = ScheduleConstant.new(m)
bb.setName("Baseboard")
bb_sch.setName(bb.nameString)
bb.setAvailabilitySchedule(bb_sch)

bb_delete.addToThermalZone(z)
bb.addToThermalZone(z)

puts "Objects named #{bb.nameString}"
puts m.objects.select{|o| o.nameString == bb.nameString}.map(&:iddObject).map(&:type)
raise unless m.getObjectsByName(bb.nameString).first.iddObject.type == "OS_Schedule_Constant".to_IddObjectType

bb_delete.remove

jmarrec commented 4 months ago

The explanation of why this happens (along with the fix) is in https://github.com/NREL/OpenStudio/pull/5122#discussion_r1538747049

NREL / OpenStudio