Unexpected errors when curves used for multiple objects

[DavidGoldwasser]

Issue overview

Current Behavior

This shows up as error which is then caught by standards and makes some measures fail.
(msg.logMessage.include?('[openstudio.model.Curve] This Curve, Object of type ') && msg.logMessage.include?(' has multiple parents. Returning the first.'))

Expected Behavior

Either the API should not let you assign same curve to multiple objects or it should not reflect doing so as an Error.

Steps to Reproduce

Running measure test related to this issue in ext reproduces it
NREL/openstudio-extension-gem#95

Possible Solution

Decide if curve is resource or child and update all code to reflect that. Simpler fix it to keep it as a resource and just remove triggered errors.

[jmarrec]

OpenStudio/src/model/Curve.cpp

Lines 50 to 59 in 7252b1d

	boost::optional<ParentObject> Curve_Impl::parent() const {
	ParentObjectVector parents = getObject<Curve>().getModelObjectSources<ParentObject>();
	if (parents.size() > 1u) {
	LOG(Error, "This Curve, " << briefDescription() << " has multiple parents. Returning the first.");
	}
	if (!parents.empty()) {
	return parents[0];
	}
	return boost::none;
	}

As you can see, it's been there since the first commit on github 8 years ago:

OpenStudio/openstudiocore/src/model/Curve.cpp

Lines 47 to 56 in c3a44f5

	boost::optional<ParentObject> Curve_Impl::parent() const {
	ParentObjectVector parents = getObject<Curve>().getModelObjectSources<ParentObject>();
	if (parents.size() > 1u) {
	LOG(Error,"This Curve, " << briefDescription() << " has multiple parents. Returning the first.");
	}
	if (!parents.empty()) {
	return parents[0];
	}
	return boost::none;
	}

[jmarrec]

Lots more:

(py39)julien@model (encodings=)$ grep -Ri "is referenced by "
InteriorPartitionSurface.cpp:265:        LOG(Error, "InteriorPartitionSurface is referenced by more than one DaylightingDeviceShelf, returning first");
GeneratorFuelCellAirSupply.cpp:159:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
RefrigerationSubcoolerMechanical.cpp:138:          LOG(Error, briefDescription() << " is referenced by more than one RefrigerationSystem, returning the first");
RefrigerationCondenserAirCooled.cpp:335:          LOG(Error, briefDescription() << " is referenced by more than one RefrigerationSystem, returning the first");
GeneratorFuelCellAuxiliaryHeater.cpp:99:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
GeneratorFuelCellExhaustGasToWaterHeatExchanger.cpp:145:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
GeneratorFuelCellElectricalStorage.cpp:84:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
PhotovoltaicPerformance_Impl.hpp:60:      /// do not allow this object to be removed if it is referenced by a PhotovoltaicGenerator
GeneratorFuelCellInverter.cpp:104:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
CoilCoolingDXMultiSpeedStageData.cpp:438:        LOG(Error, briefDescription() << " is referenced by more than one CoilCoolingDXMultiSpeed, returning the first");
GeneratorFuelCellPowerModule.cpp:112:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
RefrigerationSubcoolerLiquidSuction.cpp:152:          LOG(Error, briefDescription() << " is referenced by more than one RefrigerationSystem, returning the first");
RefrigerationCondenserEvaporativeCooled.cpp:603:          LOG(Error, briefDescription() << " is referenced by more than one RefrigerationSystem, returning the first");
ThermalZone_Impl.hpp:336:      /// If this zone is referenced by more than one space, then geometry from all spaces is added to a single zone.
GeneratorFuelCellWaterSupply.cpp:123:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
ShadingSurface.cpp:281:        LOG(Error, "ShadingSurface is referenced by more than one DaylightingDeviceShelf, returning first");
AvailabilityManagerAssignmentList.cpp:275:          LOG(Error, briefDescription() << " is referenced by more than one plantLoop, returning the first");
AvailabilityManagerAssignmentList.cpp:316:          LOG(Error, briefDescription() << " is referenced by more than one zoneHVACFourPipeFanCoils, returning the first");
AvailabilityManagerAssignmentList.cpp:352: *        LOG(Error, briefDescription() << " is referenced by more than one ZoneHVACComponent, returning the first");
RefrigerationCondenserWaterCooled.cpp:404:          LOG(Error, briefDescription() << " is referenced by more than one RefrigerationSystem, returning the first");
GeneratorFuelSupply.cpp:154:          LOG(Error, briefDescription() << " is referenced by more than one GeneratorFuelCell, returning the first");
ThermalZone.hpp:324:    /// If this zone is referenced by more than one space, then geometry from all spaces is added to a single zone.
ThermalZone.cpp:1163:    /// If this zone is referenced by more than one space, then geometry from all spaces is added to a single zone.
ThermalZone.cpp:2603:          LOG(Error, briefDescription() << " is referenced by more than one ZonePropertyUserViewFactorsBySurfaceName, returning the first");

I'm very unclear about how to address this, and whether the parent() really needs to be implemented by this class (and whether it shouldn't have been parents() when defined in ModelObject_Impl (that's where the virtual method is first implemented)) :/

[eringold]

I gotta say, I'm all kinds of confused about how 'children/resources' are supposed to work, specifically regarding curve objects. I found my way here while investigating why curves shared by AirConditionerVRF objects were unassigned after one of the ACVRF units was removed (e.g.:

m = Model.new
a1 = AirConditionerVariableRefrigerantFlow.new(m)
c1 = a1.coolingCapacityRatioBoundaryCurve.get
a2 = AirConditionerVariableRefrigerantFlow.new(m)
a2.setCoolingCapacityRatioBoundaryCurve(c1)
# => true
a1.remove
a2.coolingCapacityRatioBoundaryCurve.is_initialized?
# => false

edit: note this is with v3.2.1. I see there was an attempt to fix this (aad243b), but it doesn't?

[jmarrec]

AirConditionerVariableRefrigerantFlow is doing something usual in its remove() method: it's force removing its curve. As far as I know, this is the only class that does that. I think it's wrong.

OpenStudio/src/model/AirConditionerVariableRefrigerantFlow.cpp

Lines 1775 to 1911 in c93fb58

	std::vector<openstudio::IdfObject> AirConditionerVariableRefrigerantFlow_Impl::remove() {
	vrfModelObjectList().remove();
	boost::optional<Curve> curve;
	curve = coolingCapacityRatioModifierFunctionofLowTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingCapacityRatioBoundaryCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingCapacityRatioModifierFunctionofHighTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingEnergyInputRatioModifierFunctionofLowTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingEnergyInputRatioBoundaryCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingEnergyInputRatioModifierFunctionofHighTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingEnergyInputRatioModifierFunctionofLowPartLoadRatioCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingEnergyInputRatioModifierFunctionofHighPartLoadRatioCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingCombinationRatioCorrectionFactorCurve();
	if (curve) {
	curve->remove();
	}
	curve = coolingPartLoadFractionCorrelationCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingCapacityRatioModifierFunctionofLowTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingCapacityRatioBoundaryCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingCapacityRatioModifierFunctionofHighTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingEnergyInputRatioModifierFunctionofLowTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingEnergyInputRatioBoundaryCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingEnergyInputRatioModifierFunctionofHighTemperatureCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingEnergyInputRatioModifierFunctionofLowPartLoadRatioCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingEnergyInputRatioModifierFunctionofHighPartLoadRatioCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingCombinationRatioCorrectionFactorCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatingPartLoadFractionCorrelationCurve();
	if (curve) {
	curve->remove();
	}
	curve = pipingCorrectionFactorforLengthinCoolingModeCurve();
	if (curve) {
	curve->remove();
	}
	curve = pipingCorrectionFactorforLengthinCoolingModeCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatRecoveryCoolingCapacityModifierCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatRecoveryCoolingEnergyModifierCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatRecoveryHeatingCapacityModifierCurve();
	if (curve) {
	curve->remove();
	}
	curve = heatRecoveryHeatingEnergyModifierCurve();
	if (curve) {
	curve->remove();
	}
	return StraightComponent_Impl::remove();
	}

@kbenne I think you did that 8 years ago, I don't expect you to remember it, but does that make sense to you?

#842 specifically b8f9594

[jmarrec]

The clone method is equally weird:

OpenStudio/src/model/AirConditionerVariableRefrigerantFlow.cpp

Lines 1647 to 1650 in c93fb58

	if (auto curve = coolingCapacityRatioBoundaryCurve()) {
	auto clone = curve->clone(model).cast<Curve>();
	airConditionerClone.setCoolingCapacityRatioBoundaryCurve(clone);
	}

The curves are listed in children() (I'm not sure they should: StraightComponent is a HVACComponent which is a ParentObject), and they are ResourceObject...