lbl-srg / modelica-buildings

Modelica Buildings library
254 stars 158 forks source link

Segmentation fault in EnergyPlus is output variable does not exist #2003

Closed mwetter closed 4 years ago

mwetter commented 4 years ago

On branch issue1633_schedulesActuators, commit ccf60dcad7e664f2e6272974e36c0c28cf45a1a4 the command

simulateModel("Buildings.ThermalZones.EnergyPlus.Validation.Actuator.ShadeControl(shaAbsSol(key=\"xxx\"))");

gives a segmentation fault in EnergyPlus. The expected behavior is that EnergyPlus returns with an error.

kbenne commented 4 years ago

@mwetter

The title of this issue says output variable, but after running this model, I think the principle issue is the actuator.

I think I've isolated the problem. The code in BuildingInstantiate.c is writing the following data into json for actuators:

      buildJSONKeyValue(buffer, 4, "name", inpVars[i]->name, true, size);                                 
      buildJSONKeyValue(buffer, 4, "componentName", inpVars[i]->componentName, true, size);               
      buildJSONKeyValue(buffer, 4, "componentType", inpVars[i]->componentType, true, size);
      buildJSONKeyValue(buffer, 4, "controlType", inpVars[i]->controlType, true, size);
      buildJSONKeyValue(buffer, 4, "unit", inpVars[i]->unit, true, size);                                                
      buildJSONKeyValue(buffer, 4, "fmiName", inpVars[i]->inputs->fmiNames[0], false, size); 

Refer to here: https://github.com/lbl-srg/modelica-buildings/blob/issue1633_schedulesActuators/Buildings/Resources/C-Sources/EnergyPlus/BuildingInstantiate.c#L137

The issue is that the documentation https://lbl-srg.github.io/soep/softwareArchitecture.html#ems-actuators says actuators are defined this way:

"model": {
   "emsActuators": [
     {
       "name"          : "Zn001_Wall001_Win001_Shading_Deploy_Status",
       "variableName"  : "Zn001:Wall001:Win001",
       "componentType" : "Window Shading Control",
       "controlType"   : "Control Status",
       "unit"          : "1", // Unit string as shown in column "EnergyPlus Unit String" in the above table
       "fmiName"       : "yShade"
     }
   ]
}

Specifically notice that the documentation uses the term variableName while the BuildingInstantiate.c function is using the term componentName. I have written the FMU according to the documentation and used the term variableName. I'm going to go ahead and change the code in MBL for now and PR, but if you want to use the term componentName I can go back and change the EnergyPlus FMU code.

Also. I'm not sure what you had in mind for "name" but I'm not using it. type, name, controlType is all I need to nail down an actuator.

I'm also going to attach some updated documentation for this section. I think the phrase "Modelica will write to this EMS actuator" is false. We should not be using the term EMS anywhere in the Spawn documentation because I think EMS in the context of EnergyPlus refers to the EnergyPlus Runtime Language (ERL) which we are not using. "Actuator" seems like a safe word, I think "EMS Actuator" is confusing.

kbenne commented 4 years ago

Also, I know this is a prime example of where some logging from the FMU would be useful. I am going to provide that on a separate issue, and focus on the root cause for this one.

kbenne commented 4 years ago

There's the commit which I believe addresses this, however as I think about it, I do actually prefer the term "componentName" which pairs well with "componentType". Before accepting this fix, it might be better to change the example json and documentation, and then have me change the FMU side. Something to consider. For now this follows the docs and gets it working. *Disclaimer and did not verify the shade actually moving, but I did verify that EnergyPlus now accepts writing to this actuator.

kbenne commented 4 years ago

For the documentation I would use something other than a shade because shades have some peculiarities related to the correct component type, component name, and control type. Details documented here https://bigladdersoftware.com/epx/docs/9-3/ems-application-guide/thermal-envelope.html#window-shading-control.

The user-defined name for the WindowShadingControl is not used to identify unique window shading controls; rather, the window name is used to identify the actuator. This is because there could be multiple windows, all with shades, each of which is governed by a single WindowShadingControl input object.

A light is a nice run of the mill thing to use as an example. I suggest documentation something like this. Building on what you already have, but adjusting....

5.2.4.1.5. Actuators

To configure the data exchange for actuators, as described in Section 5.2.3.4.2, consider an example where one wants to actuate the electric power level for an EnergyPlus Lights object named "EAST ZONE Lights 1" and actuate the "Electric Power Level" which is documented by EnergyPlus https://bigladdersoftware.com/epx/docs/9-3/ems-application-guide/internal-gains-and-exterior-lights.html#lights

The corresponsding section in the ModelicaBuildingsEnergyPlus.json configuration file is

"model": {
   "emsActuators": [
     {
      "variableName"  : "EAST ZONE Lights 1",
       "componentType" : "Lights",
       "controlType"   : "Electric Power Level",
       "unit"          : "W", // Unit string as shown in column "EnergyPlus Unit String" in the above table
       "fmiName"       : "yLighting"
     }
   ]
}

The resulting FMU will then make available a variable named yLighting shown in emsActuators.fmiName and unit listed in emsActuators.unit. Modelica will write to this actuator with units shown in the column EnergyPlus Unit String in Table 5.1.
mwetter commented 4 years ago

@kbenne : ChangingcomponentName to variableName is fine if you think this is clearer on the E+ side. The value of emsActuators.[].name is the name that will be used in the modelDescription.xml file. I think you just need to pass this through, getting it from the json file and writing it to the xml file. As long as it is unique, it can be anything meaningful for those (very few) users who may want to look at the xml file. My code will search for this in the xml file to get the value reference. I plan to work on it on Tuesday and then also add the example with the light.

kbenne commented 4 years ago

If emsActuators.[].name is the name that will be used in the modelDescription.xml then what is emsActuators.[].fmiName? I currently use fmiName to populate the modelDescription because that is what the documentation says EnergyPlus will then declare in the modelDescription.xml file an input variable with name as shown in emsActuators.fmiName

https://lbl-srg.github.io/soep/softwareArchitecture.html#ems-actuators

mwetter commented 4 years ago

@kbenne Sorry, I confused the two. emsActuators.[].name should be what EnergyPlus needs, and emsActuators.[].fmiName is what it will be called in the xml file.

kbenne commented 4 years ago

I'm not using emsActuators.[].name and EnergyPlus does not require this name to make actuators work. I need the type of object to actuate (Lights), its name (EAST ZONE Lights 1), and the control type (Electric Power Level). See the suggested documentation revision in my comment above https://github.com/lbl-srg/modelica-buildings/issues/2003#issuecomment-646934886

mwetter commented 4 years ago

This should be fixed in new binary to be posted, needs testing.

mwetter commented 4 years ago

This now correctly reports

In ShadeControl.building: EnergyPlus Error: Attempt to get invalid variable using name 'SURFACE WINDOW SHADING DEVICE ABSORBED SOLAR RADIATION RATE', and key 'XXX'

on branch issue1633_schedulesActuators.