Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Space load-based actuator for spaces are named based on thermal zone name #4786

Closed
lymereJ opened this issue Jan 23, 2023 · 18 comments · Fixed by #4828
Closed

Space load-based actuator for spaces are named based on thermal zone name #4786

lymereJ opened this issue Jan 23, 2023 · 18 comments · Fixed by #4828

Comments

@lymereJ
Copy link

lymereJ commented Jan 23, 2023

Issue overview

The actuated component unique name of EMS:Actuator objects in EnergyPlus is determined by OS during the forward translation process. The name appears to be based on the zone name and load object (Lights in the example below) name, even for spaces. However, when spaces are used in a model, EnergyPlus expected the name of the actuator to refer to the space name, not the zone name

Current Behavior

The code snippet below creates the following actuator ....

EnergyManagementSystem:Actuator,
    Energy_Management_System_Actuator_1,  !- Name
    Thermal Zone 1 Lights 1, !- Actuated Component Unique Name
    Lights,                  !- Actuated Component Type
    Electricity Rate;        !- Actuated Component Control Type

Expected Behavior

... but it should be:

EnergyManagementSystem:Actuator,
    Energy_Management_System_Actuator_1,  !- Name
    Space 1 Lights 1, !- Actuated Component Unique Name
    Lights,                  !- Actuated Component Type
    Electricity Rate;        !- Actuated Component Control Type

Steps to Reproduce

require 'openstudio'
include OpenStudio::Model

model = Model.new()
spaceType1 = SpaceType.new(model)
space1 = Space.new(model)
tz1 = ThermalZone.new(model)

space1.setThermalZone(tz1)
space1.setSpaceType(spaceType1)

spaceType1.setLightingPowerPerFloorArea(0.5)

light = space1.spaceType.get.lights[0]

actuator = OpenStudio::Model::EnergyManagementSystemActuator.new(light, 'Lights', 'Electricity Rate', space1)

ft = OpenStudio::EnergyPlus::ForwardTranslator.new
w = ft.translateModel(model)
puts w.getObjectsByType("EnergyManagementSystem:Actuator")
EnergyManagementSystem:Actuator,
  Energy_Management_System_Actuator_1,    !- Name
  Thermal Zone 1 Lights 1,                !- Actuated Component Unique Name
  Lights,                                 !- Actuated Component Type
  Electricity Rate;                       !- Actuated Component Control Type

Possible Solution

Do not use the thermal zone name by default here.

Environment

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

  • Platform: Windows
  • Version of OpenStudio: Tested in 3.5.1

Context

Affects NREL/openstudio-standards#1395.

@lymereJ lymereJ added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Jan 23, 2023
@tijcolem tijcolem added severity - Normal Bug component - Model and removed Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Jan 27, 2023
@tijcolem tijcolem added this to the OpenStudio SDK 3.6.0 milestone Jan 27, 2023
@joseph-robertson
Copy link
Collaborator

Should use getSpaceLoadParent?

IdfObject ForwardTranslator::getSpaceLoadParent(const model::SpaceLoad& sp, bool allowSpaceType) {

@brianlball
Copy link
Contributor

brianlball commented Mar 6, 2023

This was tricky back in the day because OS didnt use ZoneLists, but E+ did, etc. The constructors were built with the following

/**These constructors below are for SpaceloadInstances that are defined in SpaceTypes that are used in Spaces.
* Upon translation, the SpaceLoadInstances use ZoneLists which are not avail in OS
* The ZoneListName is the SpaceType name
* The Zone's are the Space->ThermalZone names
* So to attach to a future zone, use the TZ or the Space that the SpaceLoadInstance will operate on
**/
explicit EnergyManagementSystemActuator(const ModelObject& modelObject, const std::string& actuatedComponentType,
const std::string& actuatedComponentControlType, const ThermalZone& thermalZone);
explicit EnergyManagementSystemActuator(const ModelObject& modelObject, const std::string& actuatedComponentType,
const std::string& actuatedComponentControlType, const Space& space);

@brianlball
Copy link
Contributor

I would look at the constructor first to make sure the assumptions are still valid, given the space changes in E+, etc

@joseph-robertson
Copy link
Collaborator

@lymereJ Note that in your "Steps to Reproduce" section above, the light variable is unused. Should it be passed into the first position of EnergyManagementSystemActuator.new(...)?

@lymereJ
Copy link
Author

lymereJ commented Mar 8, 2023

@joseph-robertson - You're correct, may bad. The code that I have in "Steps to Reproduce" generates the following actuator (incorrect as well), not the one that's in "Current Behavior". Not sure why I put space1 instead of light.

EnergyManagementSystem:Actuator,
  Energy_Management_System_Actuator_1,    !- Name
  Space 1,                                !- Actuated Component Unique Name
  Lights,                                 !- Actuated Component Type
  Electricity Rate;                       !- Actuated Component Control Type

I'll correct that line in "Step to Reproduce". Thanks!

@jmarrec
Copy link
Collaborator

jmarrec commented Mar 14, 2023

@lymereJ is an option for you now to just disable the space feature when forward translating?

@jmarrec
Copy link
Collaborator

jmarrec commented Mar 14, 2023

This was tricky back in the day because OS didnt use ZoneLists, but E+ did, etc. The constructors were built with the following

/**These constructors below are for SpaceloadInstances that are defined in SpaceTypes that are used in Spaces.
* Upon translation, the SpaceLoadInstances use ZoneLists which are not avail in OS
* The ZoneListName is the SpaceType name
* The Zone's are the Space->ThermalZone names
* So to attach to a future zone, use the TZ or the Space that the SpaceLoadInstance will operate on
**/
explicit EnergyManagementSystemActuator(const ModelObject& modelObject, const std::string& actuatedComponentType,
const std::string& actuatedComponentControlType, const ThermalZone& thermalZone);
explicit EnergyManagementSystemActuator(const ModelObject& modelObject, const std::string& actuatedComponentType,
const std::string& actuatedComponentControlType, const Space& space);

Shouldn't these take in a SpaceLoadInstance instead of a ModelObject given the comment? I could see it going really wrong...

@lymereJ
Copy link
Author

lymereJ commented Mar 14, 2023

@lymereJ is an option for you now to just disable the space feature when forward translating?

@jmarrec, I believe that we don't want to do that, this is for OpenStudio-Standards. I'll let @mdahlhausen confirm.

@mdahlhausen
Copy link
Collaborator

@lymereJ is an option for you now to just disable the space feature when forward translating?

@jmarrec, I believe that we don't want to do that, this is for OpenStudio-Standards. I'll let @mdahlhausen confirm.

yeah, that's not a workable solution. People are build models using openstudio-standards, often in third-party software. I don't want to require third-party developers to have to disable the space feature when forward translating. Especially for end-users who might not know how to disable it.

@jmarrec
Copy link
Collaborator

jmarrec commented Mar 14, 2023

OK. I'm asking because this is marked as blocker, we're close to release date, and we have the existing API to retain.
I'll try to figure something out in the next couple of days that aims to support this without breaking api, I'll likely ask for thoughts/decisions as I dig down.

@brianlball
Copy link
Contributor

not sure if this helps with the debugging, but here is a measure that reproduces the EMS Demand Management example#9 and uses the Actuator on the lights object.
https://github.com/NREL/OpenStudio-EMS-Measures/blob/master/Measures/m_9_example_9_demand_management/measure.rb#L301

@mdahlhausen
Copy link
Collaborator

I'm going to try to see if there is a workaround in openstudio-standards.

@brianlball
Copy link
Contributor

i just checked and this does work in OS <=3.2 but not in OS 3.5.1

require 'openstudio'
model = OpenStudio::Model.exampleModel()
spaceType1 = model.getSpaceTypes()[0]
space1 = model.getSpaces()[0]
tz1 = model.getThermalZones()[0]

light = space1.spaceType.get.lights[0]
actuator = OpenStudio::Model::EnergyManagementSystemActuator.new(light, 'Lights', 'Electricity Rate', space1)

ft = OpenStudio::EnergyPlus::ForwardTranslator.new
w = ft.translateModel(model)

my bet it has something to do with no more ZoneLists or the new spaces

image

@brianlball
Copy link
Contributor

looks like the actuators changed to use Spaces instead of Thermal Zones according to the .EDD

image

@brianlball
Copy link
Contributor

@mdahlhausen
Copy link
Collaborator

Some more context:
Running test_small_office.rb for 90.1-2016 and ASHRAE 169-2013-5A

In OS 3.2.1:

OS:EnergyManagementSystem:Sensor,
  {9d59be4e-d960-47c1-aa96-6783eb166770}, !- Handle
  Core_ZNZN_LSr,                          !- Name
  Core_ZN ZN,                             !- Output Variable or Output Meter Index Key Name
  Zone Lights Electricity Rate;           !- Output Variable or Output Meter Name

OS:EnergyManagementSystem:InternalVariable,
  {496b4e8d-6dbc-49c2-8fd6-f0a0292d0d74}, !- Handle
  Core_ZNZN_Area,                         !- Name
  Core_ZN ZN,                             !- Internal Data Index Key Name
  Zone Floor Area;                        !- Internal Data Type

OS:EnergyManagementSystem:Actuator,
  {4936ef85-4857-41ae-b64f-5308c174d49f}, !- Handle
  Core_ZNZN_Light1_Actuator,              !- Name
  {a5e82b87-ce0f-4588-9715-85b90f56135a}, !- Actuated Component Name
  Lights,                                 !- Actuated Component Type
  Electricity Rate;                       !- Actuated Component Control Type

OS:EnergyManagementSystem:Actuator,
  {69231cf4-fc89-48f6-a304-29cadfe9f63c}, !- Handle
  Perimeter_ZN_1ZN_Light1_Actuator,       !- Name
  {a5e82b87-ce0f-4588-9715-85b90f56135a}, !- Actuated Component Name
  Lights,                                 !- Actuated Component Type
  Electricity Rate;                       !- Actuated Component Control Type

actuated component is:

OS:Lights,
  {a5e82b87-ce0f-4588-9715-85b90f56135a}, !- Handle
  Office WholeBuilding - Sm Office Lights, !- Name
  {72e4e8c6-2310-41ff-856b-20a2cebb0674}, !- Lights Definition Name
  {629a271e-7460-4c51-87d3-d2d3125cfac1}, !- Space or SpaceType Name
  ,                                       !- Schedule Name
  1,                                      !- Fraction Replaceable
  ,                                       !- Multiplier
  General;                                !- End-Use Subcategory

which is used by this space type object:

OS:SpaceType,
  {629a271e-7460-4c51-87d3-d2d3125cfac1}, !- Handle
  Office WholeBuilding - Sm Office,       !- Name
  ,                                       !- Default Construction Set Name
  {09f83373-2543-4efe-ad90-7facebecbcbb}, !- Default Schedule Set Name
  {c9f3a3a1-7407-4eab-bb64-27cdfc8e7a35}, !- Group Rendering Name
  {58b7ecf6-9d4d-4d56-95e8-b61a0fed9557}, !- Design Specification Outdoor Air Object Name
  ,                                       !- Standards Template
  Office,                                 !- Standards Building Type
  WholeBuilding - Sm Office;              !- Standards Space Type

The test passes but generates this warning from the forward translator:
[openstudio.energyplus.ForwardTranslator] <0> Actuator 'Core_ZNZN_Light1_Actuator' references a SpaceLoad 'Office WholeBuilding - Sm Office Lights' attached to SpaceType 'Office WholeBuilding - Sm Office', with multiple spaces. Check your EMS programs to make sure your actuators are correct.

What appears in the .idf:

EnergyManagementSystem:Actuator,
  Core_ZNZN_Light1_Actuator,              !- Name
  Core_ZN ZN Office WholeBuilding - Sm Office Lights, !- Actuated Component Unique Name
  Lights,                                 !- Actuated Component Type
  Electricity Rate;                       !- Actuated Component Control Type

EnergyManagementSystem:Actuator,
  Perimeter_ZN_1ZN_Light1_Actuator,       !- Name
  Core_ZN ZN Office WholeBuilding - Sm Office Lights, !- Actuated Component Unique Name
  Lights,                                 !- Actuated Component Type
  Electricity Rate;                       !- Actuated Component Control Type

All of the actuators point to the same shared lights object. That's a bug.

And it errors out now in OS 3.5.1:

   ** Severe  ** Invalid Actuated Component Unique Name =CORE_ZN ZN OFFICE WHOLEBUILDING - SM OFFICE LIGHTS
   **   ~~~   ** Entered in EnergyManagementSystem:Actuator=CORE_ZNZN_LIGHT1_ACTUATOR

It seems there are two issues here:

  1. There is a bug in openstudio-standards; the lights object needs to be broken out by zone or space before applying EMS code
  2. The forward translator simply can't handle shared space/zone lights objects required by EMS. Ideally, it would take a shared space type resource like the lights object and make zone individually for each space if shared by EMS. Or we can say you can't use shared space type resources with EMS.

I think I can fix openstudio-standards by creating individual space light objects in OS 3.5.1, which means this issue is no longer a blocker. @lymereJ correct me here if you don't think that's possible.

@mdahlhausen
Copy link
Collaborator

@lymereJ @jmarrec Update: I've got a decent workaround implemented in openstudio-standards. I clone the space type lights object to each individual space. Works with both OSv3.2.1 and OSv3.5.1.

Given the workaround, this issue is no longer an OpenStudio blocker for openstudio-standards.

jmarrec added a commit that referenced this issue Mar 15, 2023
jmarrec added a commit that referenced this issue Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment