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

Support gbXML translation where user-input <Name> is different from the id #4457

Closed
chriswmackey opened this issue Oct 4, 2021 · 18 comments · Fixed by #4513
Closed

Support gbXML translation where user-input <Name> is different from the id #4457

chriswmackey opened this issue Oct 4, 2021 · 18 comments · Fixed by #4513

Comments

@chriswmackey
Copy link

chriswmackey commented Oct 4, 2021

Enhancement Request

For the majority of objects in gbXML, there is both an id that is used to reference the object throughout the XML but there is also a human-readable <Name> tag for the object. Here is an example with Space:

gbXML_Name_and_id

Currently, there is no way to produce gbXML files with the OpenStudio SDK where objects have a <Name> that is different than their id so both fields suffer from the same restrictions. This also means that the id of the object is lost when importing from gbXML to OpenStudio and the resulting gbXML gets imported with the user-input <Name> set as the EnergyPlus Name, meaning the imported object name often contains characters that are not appropriate for EnergyPlus.

Detailed Description

In many workflows, it makes sense for the Name of the OpenStudio workspace object to become gbXML id (and vice versa on import) since OpenStudio Names are ultimately what identify the objects within IDF and in the EnergyPlus results. Both gbXML ids and OpenStudio/EnergyPlus Names need to follow typical character restrictions and they need to be unique since they are used to identify objects across a model. So it would be helpful if the gbXML importer (aka. ReverseTranslator) could have an option to import the gbXML ids as OpenStudio/EnergyPlus Names instead of always trying to make the gbXML <Name> act like an identifier.

Conversely, given that the <Name> tags in gbXML are often specified by end users, such <Name> tags can often be duplicated in the objects across a gbXML model or they can contain characters that aren't legal for EnergyPlus. It would be nice to have some way to support this usage of the gbXML <Name> tag in the OpenStudio SDK since, right now, such usage can result in imported gbXML models that are not simulate-able in EnergyPlus. Or, if they are simulate-able, the EnergyPlus results cannot be easily matched back to the original gbXML because OpenStudio escapes special characters in the gbXML <Names> when it translates them to IDF.

Possible Implementation

Working with the features currently supported in the OpenStudio SDK, we can see that all ModelObjects in OpenStudio have an addiditonalProperties attribute that can be used to store custom types of user inputs (like user-input names).

Within the Ladybug Tools software that I support, we are currently planning to store the user-input names of objects under these AdditionalProperties. We plan to call this user-input name under AdditionalProperties a DisplayName and we'll use it to ensure that these user-input names can survive the translation from OpenStudio and back to our software, while still ensuring that the IDF and E+ Results use ids that are appropriate for E+.

Perhaps, if we could agree that that DisplayName (or some other property) could be a standard additionalProperty that was available on OpenStudio Model Objects, we can use it to support the the current intended behavior of the <Name> tag in gbXML. This would free up the gbXML id to act as an identifier across the gbXML > OSM > IDF > E+ Results translation process.

Note

I realize that this request is probably not that technically difficult to implement but I'm sure that many of us will have strong opinions. So let's all be open-minded that there may be a few good possible implementations to making this type of interoperability with gbXML work.

CC: @tijcolem , @tanushree04 , @macumber

@chriswmackey chriswmackey added Enhancement Request Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Oct 4, 2021
@macumber
Copy link
Contributor

macumber commented Oct 6, 2021

I support this enhancement request. I wanted to point out there is already some support for "CADObjectId" as a special additional property:

https://github.com/NREL/OpenStudio/blob/master/src/gbxml/ForwardTranslator.cpp#L1337

I think reserved or supported additional properties names should have an API method to access them or at least a list of the supported property names some where.

I also think it would be good to have a table showing the mappings between various names, ids, etc between gbXML, E+, and OSM. The table could include which properties are optional and which properties are the fall-back if not set.

@macumber
Copy link
Contributor

macumber commented Oct 15, 2021

FYI, here is some code that could be used as a forward compatible patch in measures or other code until this method is available in C++:

if !OpenStudio::Model::ModelObject.method_defined?(:displayName)
  OpenStudio::Model::ModelObject.class_eval do
    define_method(:displayName) do
      additionalProperties.getFeatureAsString("DisplayName")
    end
    define_method(:setDisplayName) do |value|
      additionalProperties.setFeature("DisplayName", value)
    end
  end
end

m = OpenStudio::Model::exampleModel
puts m.getBuilding.displayName
m.getBuilding.setDisplayName("英語")
puts m.getBuilding.displayName
m.getBuilding.setDisplayName("C,S,V")
puts m.getBuilding.displayName

@tijcolem tijcolem added component - gbXML and removed Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Nov 5, 2021
@jmarrec
Copy link
Collaborator

jmarrec commented Nov 15, 2021

Thanks for the detailed description. It is quite dense, so it would be helpful to try and turn it into action items / bullet points. Before we do that, I'll mention what happens currently:

Reverse Translator

The id attribute and <Name> tag are gotten. id is set as as an AdditionalProperties called gbXMLId on the object. The object is named like <Name> if specified, id otherwise, but it goes through escapeName anyways.

eg: Let's take Space as an example

<Space zoneIdRef="aim0043" [...] id="aim0046">
  [...]
  <Name>1 Space, somewhere</Name>
</Space>

boost::optional<model::ModelObject> ReverseTranslator::translateSpace(const pugi::xml_node& element, openstudio::model::Model& model) {
openstudio::model::Space space(model);
std::string id = element.attribute("id").value();
m_idToObjectMap.insert(std::make_pair(id, space));
space.additionalProperties().setFeature("gbXMLId", id);
std::string name = element.child("Name").text().as_string();
space.setName(escapeName(id, name));

std::string ReverseTranslator::escapeName(const std::string& id, const std::string& name) {
std::string value = id;
if (!name.empty()) {
value = name;
}
std::replace(value.begin(), value.end(), ',', '-');
std::replace(value.begin(), value.end(), ';', '-');
return value;
}

Result: a space named '1 Space- somewhere' (due to escapeName), and an additional property

[1] gbxml(main)> puts s
OS:Space,
  {7d699f2a-fb29-45a9-9845-81ad9465db09}, !- Handle
  1 Space- somewhere,                     !- Name
  ,                                       !- Space Type Name
  ,                                       !- Default Construction Set Name
  ,                                       !- Default Schedule Set Name
  ,                                       !- Direction of Relative North {deg}
  ,                                       !- X Origin {m}
  ,                                       !- Y Origin {m}
  ,                                       !- Z Origin {m}
  {8fd16fc7-50eb-472f-8938-874eb4cde4ee}, !- Building Story Name
  {2d98c7ea-05dd-421d-84aa-d19569f24aad}; !- Thermal Zone Name

[2] gbxml(main)> puts s.additionalProperties
OS:AdditionalProperties,
  {de9f86dc-5180-4a66-898f-c28c22771f78}, !- Handle
  {7d699f2a-fb29-45a9-9845-81ad9465db09}, !- Object Name
  gbXMLId,                                !- Feature Name 1
  String,                                 !- Feature Data Type 1
  aim0046,                                !- Feature Value 1
  CADObjectId,                            !- Feature Name 2
  String,                                 !- Feature Data Type 2
  4498;                                   !- Feature Value 2

ForwardTranslator

The name of the object is used:

  • Directly as is for the <Name> tag
  • id is set to escapeName(name)

std::string name = space.name().get();
result.append_attribute("id") = escapeName(name).c_str();

// name
auto nameElement = result.append_child("Name");
nameElement.text() = name.c_str();

std::string ForwardTranslator::escapeName(const std::string& name) {
std::string result;
if (std::regex_match(name, std::regex("^\\d.*"))) {
result = "id_" + name;
} else {
result = name;
}
std::replace(result.begin(), result.end(), ' ', '_');
std::replace(result.begin(), result.end(), '(', '_');
std::replace(result.begin(), result.end(), ')', '_');
std::replace(result.begin(), result.end(), '[', '_');
std::replace(result.begin(), result.end(), ']', '_');
std::replace(result.begin(), result.end(), '{', '_');
std::replace(result.begin(), result.end(), '}', '_');
std::replace(result.begin(), result.end(), '/', '_');
std::replace(result.begin(), result.end(), '\\', '_');
//std::replace(result.begin(), result.end(),'-', '_'); // ok
//std::replace(result.begin(), result.end(),'.', '_'); // ok
std::replace(result.begin(), result.end(), ':', '_');
std::replace(result.begin(), result.end(), ';', '_');
return result;
}

[1] gbxml(main)> m = Model.new
=> #<OpenStudio::Model::Model:0x000055ea2ad35148 @__swigtype__="_p_openstudio__model__Model">
[2] gbxml(main)> s = Space.new(m)
=> #<OpenStudio::Model::Space:0x000055ea2ae1d448 @__swigtype__="_p_openstudio__model__Space">
[3] gbxml(main)> s.setName("A space, somewhere")
=> #<OpenStudio::OptionalString:0x000055ea2ae4d1e8 @__swigtype__="_p_boost__optionalT_std__string_t">
[4] gbxml(main)> ft = OpenStudio::GbXML::GbXMLForwardTranslator.new
=> #<OpenStudio::GbXML::GbXMLForwardTranslator:0x000055ea2a838668 @__swigtype__="_p_openstudio__gbxml__ForwardTranslator">
[5] gbxml(main)> puts ft.modelToGbXMLString(m)
[...]
      <Space id="A_space,_somewhere">
        <Name>A space, somewhere</Name>
      </Space>
[...]
=> nil

@jmarrec
Copy link
Collaborator

jmarrec commented Nov 15, 2021

I'm having a hard time finding the actions items. I do see a couple at least:

  • When ForwardTranslating a model to gbxml, we should check if there is a gbXMLId AdditionalProperty already and use that as the id, so we can actually round-trip a model properly.
  • Helper methods for these standard additional properties should be added, at least ModelObject::gbXMLId() / ModelObject::setGbXMLId(), and probably cADObjectId() too

As far as adding <DisplayName>, that is not part of the gbxml schema and therefore shouldn't be supported IMHO.
The schema for Space for eg can be found here: https://www.gbxml.org/schema/6-01/GreenBuildingXML_Ver6.01.xsd#element_Space

The id attribute is a xsd:ID. That is a xsd:string with the NCName (non-colonized name) restriction. The practical restrictions of NCName are that it cannot contain several symbol characters like :, @, $, %, &, /, +, ,, ;, whitespace characters or different parenthesis. Furthermore an NCName cannot begin with a number, dot or minus character although they can appear later in an NCName.
But it does support plenty of non-ascii characters. Any listed here: https://www.w3.org/TR/2000/WD-xml-2e-20000814#NT-Letter, so that includes stuff like ਏ for eg.
Name is an xsd:string, so pretty much anything is fair game.

A third field could be of interest: <Description>.

Given the E+ restrictions on what is a valid name, versus what the gbxml accepts, I'm not sure how we deal with this, aside from educating users to use all ASCII chars for name (like they would do in E+ directly), and probably for id too to be safe. The <Description> tag is fair game for adding whatever special characters they want though.

Where you perhaps suggesting to use the id field as the resulting ModelObject's Name when ReverseTranslating?

@macumber
Copy link
Contributor

macumber commented Nov 15, 2021

I would suggest a table showing the mappings to help make sure we are all on the same page. The following is a rough start, probably needs some iterations (feel free to edit my comment):

OpenStudio::GbXML::GbXMLReverseTranslator followed by OpenStudio::EnergyPlus::ForwardTranslator:

gbXML OpenStudio EnergyPlus
id name name
name displayName comment
cadObjectId cadObjectId comment

OpenStudio::EnergyPlus::ReverseTranslator followed by OpenStudio::GbXML::GbXMLForwardTranslator:

Reverse translation (backup means what to use if displayName/cadObjectId don't exist):

EnergyPlus OpenStudio gbXML
name name id
- displayName (backup name) name
- cadObjectId (backup none) cadObjectId

@jmarrec
Copy link
Collaborator

jmarrec commented Nov 15, 2021

Ok, so you do want to use the gbxml id as the resulting ModelObject name. Question: whose workflow will we break if we suddenly change the current behavior?

@chriswmackey
Copy link
Author

chriswmackey commented Nov 17, 2021

Hi @jmarrec ,
I understand the desire to keep the current behavior and, if the only way that we can support importing/exporting the gbXML id is by adding it as an additional property for it, then so be it. I can switch the gbXMLId and the Name as part of the measure that imports/exports the gbXML for the Ladybug Tools workflows.

But, if you ask me, I think a lot of developers expect to use the gbXML id to identify a given model element in the EnergyPlus results and this matching of results becomes difficult when gbXML Names are not unique and/or OpenStudio removes illegal characters in the Name upon export to IDF. I realize that the character restrictions on EnergyPlus Names and gbXML ids are not perfectly equal to one another but there's more overlap between them than there is between EnergyPlus Names and gbXML <Names>. Furthermore, the way that EnergyPlus uses the Name is really like an id. It's how the objects reference each other in the IDF, just like how objects reference each other within a gbXML file.

So I feel like an ideal solution would be to have an option on the gbXML translators to either keep the existing behavior and import the ids as gbXMLId. OR the gbXML ids could be imported to the OpenStudio/EnergyPlus Name and the gbXML Names be imported to DisplayName. The default could be the option that keeps the current behavior so that the current workflows don't break.

I understand that it's more work to implement it this way with both options and, if it's ultimately too difficult, I can handle the switching of name/id in our translation measures as I mentioned above. We'll take whatever I can get here and the most important thing is that the data isn't lost upon translation.

@joseph-robertson joseph-robertson self-assigned this Dec 15, 2021
@MatthewSteen
Copy link
Member

MatthewSteen commented Dec 29, 2021

A fix for this issue might also benefit Revit's Systems Analysis feature, which uses the Reverse Translator. I've been using https://github.com/GreenBuildingXML/Sample_gbXML_Files for testing, which have several files where the XML element's Name is not unique. For example, the gbXMLStandard Office (ASHRAE HQ) 2016.xml has 63 zones, which all have the same <Name>Zone Default</Name>.

@joseph-robertson
Copy link
Collaborator

We'll try (1) implementing @macumber's table, (2) consider adding methods to the API (displayName and cadObjectID), and then (3) think about possibly having a switch option (and what the default behavior should be).

@kbenne
Copy link
Contributor

kbenne commented Jan 4, 2022

One minor nitpick. I feel like the termdisplayName is a bit confusing with the term name. I feel like OS name already represents the display name, so perhaps displayName would be better as cadName or something of the sort.

@joseph-robertson
Copy link
Collaborator

joseph-robertson commented Jan 5, 2022

Using the above tables as starting points, here they are updated with Default (existing behavior) and Option (new behavior).

ReverseTranslator:

  XML OSM Backup
Default id gbXMLId  
  name name  
  cadObjectId cadObjectId none
Option id name  
  name cadName  
  cadObjectId cadObjectId none

ForwardTranslator:

  OSM XML Backup
Default gbXMLId id name 
  name name  
  cadObjectId cadObjectId none
Option name id
  cadName name name
  cadObjectId cadObjectId none

Does everyone agree on this? Should this apply to ThermalZone, Space, others?

@chriswmackey @macumber @kbenne @jmarrec @MatthewSteen

@jmarrec
Copy link
Collaborator

jmarrec commented Jan 6, 2022

gbXMLId is completely lost in forward translation in the option case and that bothers me somehow. I don't know if it's a big deal or not. Perhaps a simple warning saying "You have an additional property gbXMLId that will be unused" or something would be useful?

Otherwise if people are fine with this table (as in it fulfills their need), so am I.

@joseph-robertson
Copy link
Collaborator

Maybe the option case for forward translation should map gbXMLId to id and use name as a backup (like the default case)?

@joseph-robertson
Copy link
Collaborator

Thinking about this more, maybe there shouldn't be a separate option case for forward translation. The default case should be updated to:

  OSM XML Backup
Default gbXMLId id name
  cadName name name
  cadObjectId cadObjectId none

@macumber
Copy link
Contributor

macumber commented Jan 9, 2022

Hey everyone, happy new year! I'm glad you are all discussing this before writing code, I think it is worth really thinking through this issue otherwise it's just going to continue to be a problem. I don't have the solution, but it seems to me that the point of an id is to be able to find an object in a given data model and the point of a name is to display the object's name in a given app native to a specific data model. I think you would want to try to support round trip translation of these id and name attributes for any object that can come from a CAD model: spaces, thermal zones, surfaces, constructions, materials, etc.

If you start a model in CAD, import to OpenStudio, and inspect the OpenStudio model in an OpenStudio application: you want the CAD names to come in to OpenStudio's displayName field so you can compare the objects in an OpenStudio application side by side with the CAD application. The OpenStudio Application and SketchUp Plug-in don't always show the displayName for OpenStudio objects, but it should. I wouldn't want to also add a cadName field to all the UIs, although something like this might catch all cases: openstudiocoalition/OpenStudioApplication#417. You can't import the CAD name (which can have unicode, spaces, commas, etc) into the OpenStudio name field because there are restrictions on the OpenStudio name field due to E+.

If you start a model in CAD, import to OpenStudio, and run a simulation: you probably want OpenStudio to export some file in a reporting measure that associates E+ results with cadObjectId so the CAD program can read the results in directly. I don't know that you want to re-export a new gbXML from OpenStudio for the CAD program to use in this case.

You might want to start a model in CAD, import to OpenStudio, make changes to the objects (maybe fill in thermal properties or something), and want to export a gbXML to reimport back into your CAD application. Do any CAD applications support this?

Alternatively, you might want to start a model in CAD, import to OpenStudio, make changes to the objects (maybe fill in thermal properties or something), update model geometry in CAD, and then merge the OpenStudio model with a new gbXML from the CAD application. I think merging based on cadObjectId would be the right thing in this case.

So maybe it is useful to add the actual CAD applications to these tables. We can't control what Revit does but we can at least document it. @chriswmackey could fill this out with Honeybee as the CAD tool?

Revit::RevitForwardTranslator (which we don't control) followed by OpenStudio::GbXML::GbXMLReverseTranslator followed by OpenStudio::EnergyPlus::ForwardTranslator:

Revit gbXML OpenStudio EnergyPlus
- id (new random id?) name (or could add gbXMLId) name (or could add gbXMLId)
name name (is this true?) displayName comment
id cadObjectId (is this true?) cadObjectId comment

OpenStudio::EnergyPlus::ReverseTranslator followed by OpenStudio::GbXML::GbXMLForwardTranslator:

Reverse translation (backup means what to use if displayName/cadObjectId don't exist):

EnergyPlus OpenStudio gbXML
name name id
- displayName (backup name) name
- cadObjectId (backup none) cadObjectId

@chriswmackey
Copy link
Author

chriswmackey commented Jan 11, 2022

I second everything that @macumber said and I am really glad that we're discussing this before pulling the trigger.

I like the proposal of adding at least two new optional fields (eg. displayName and gbXMLId) and having logic that says, "if these fields are available, use them. Otherwise, default back to the old behavior where the OpenStudio/EnergyPlus name did everything." If we do this, I agree we can get away without needing a 2nd option. So the whole import/export process could get a lot more elegant and standardized without having to break current behavior of the name. I'll take a stab at adding Honeybee into the table there. We already have a convention for these type of fields that we use in our UI and HBJSON schema. Our schema is not going to change but how we map this to OSM and IDF is definitely still able to change.

This is how I think I would integrate Honeybee into the table Dan put together:

Revit Honeybee gbXML OpenStudio EnergyPlus
- identifier id name (and/or gbXMLId) name (and/or gbXMLId)
name display_name <Name> displayName comment
id user_data['cad_object_id'] <CADObjectId> cadObjectId comment

I'm not terribly attached to calling the property displayName in OpenStudio if most of us think that cadName is better. Obviously, I am biased because we called it display_name in our Honeybee schema. But I know that naming things is one of the two hardest things in computer programming and you have to pick whatever you think is best from the perspective of the OpenStudio SDK.

joseph-robertson added a commit that referenced this issue Feb 19, 2022
Addresses #4457, support gbXML translation where user-input <Name> is different from the id
@Cymap-NZ
Copy link

Hi Just re-visiting this as it seem that the SDK has been updated and I have been able to obtain the tag in Code from the additionalProperties(). However the OS application itself still defaults to using the gbxml.ID. Which isn't overly friendly. So there needs to be a request for the application to default to using the name field when a model is set-up from a gbxml translation?

@jmarrec
Copy link
Collaborator

jmarrec commented Jun 26, 2023

Not sure I follow the question here @Cymap-NZ. Are you talking about https://github.com/openstudiocoalition/OpenStudioApplication ? If so, yes, please open a detailed issue there with enough information to clearly understand what's needed so the OpenStudio Application team can have a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants