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

OpenStudio fails to write SpaceType into the IDF if it has the same name as the Building object #5226

Closed
chriswmackey opened this issue Jun 27, 2024 · 11 comments · May be fixed by #5305
Closed

Comments

@chriswmackey
Copy link

Issue overview

The title basically says it all. This is quite possibly the strangest and most subtle bug that I have found in OpenStudio. It seems like the type of thing that you only discover when you have a community of thousands of people using the software. I'm very thankful this user in our community helped us find it since I never would have found something like this on my own.

Granted, I also recognize that naming the Building and the SpaceType the same thing is probably not the best practice. But I'm also 90% sure that you would agree with me that it is a bug.

Current Behavior

Here is a minimal sample OSM that recreates the issue:
https://drive.google.com/file/d/19s7PgStDRpLMk3Q7Qn1opbQQccpk4tJd/view?usp=sharing

You can see that there is one SpaceType in the file called "housing" which has the same name as the Building object. In its current state, you will find that the OSM does not translate the Space Type to IDF. However, if you change either the SpaceType name or the Building name, everything will translate correctly.

Expected Behavior

I would expect the Space Type to be translated to the IDF regardless of what it is named. It took me quite a while to realize that the problem was with the name.

Environment

Both the user who reported the issue and I were on Windows but I think this bug probably also affects the Mac/Linux side.

Context

I implemented a workaround for this in Ladybug Tools for now so it's not urgent for us:
ladybug-tools/honeybee-energy@e5d46df

But I imagine that you would want to get this fixed for the sake of other developers

@chriswmackey chriswmackey added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Jun 27, 2024
@jmarrec
Copy link
Collaborator

jmarrec commented Oct 30, 2024

@chriswmackey you're writting the OSM by hand or something?

m = Model.new
b = m.getBuilding
b.setName("Building")
sp = SpaceType.new(m)
sp.setName("Building").get
=> "Building 1"        /// <----- you see? it's renamed

@jmarrec jmarrec added resolution - Can't Reproduce and removed Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Oct 30, 2024
@jmarrec
Copy link
Collaborator

jmarrec commented Oct 30, 2024

OS:SpaceType,
       \min-fields 1
  A1, \field Handle
       \type handle
       \required-field
  A2, \field Name
       \type alpha
       \required-field
       \reference SpaceTypeNames
       \reference SpaceAndSpaceTypeNames
       \reference SpaceAndSpaceGroupNames

OS:Building,
       \unique-object
       \required-object
       \min-fields 8
  A1, \field Handle
       \type handle
       \required-field
  A2, \field Name
       \type alpha
       \required-field
       \reference BuildingNames
       \reference SpaceAndSpaceGroupNames

the SpaceAndSpaceGroupNames enforces the unicity of the name in the model namespace, so this can't happen under normal circumstances. Closing for now, happy to reopen if something else is provided.

@chriswmackey
Copy link
Author

Hey @jmarrec ,

Sorry for the late response. I am writing the OSM using OpenStudio SDK in a Ruby measure. I don't know how helpful it is to explain exactly what we do but it's all happening with this honeybee-openstudio-gem that we authored to translate from our native model schema in Ladybug Tools (called HBJSON) over to OSM.

Here is where we set the Building name and here is where we set the SpaceType name.

I understand if this situation can't happen from the OpenSutdio Application but I can clearly create this situation with the OpenStudio SDK in a measure. Do you think something could be implemented within the OpenStudio SDK to sense if a SpaceType has the same name as a Building and, if so, change it so that is does not match? Maybe using something similar to the logic which ensures unique EnergyPlus names for objects by default (appending an integer to the end of the name if it's not unique)?

@jmarrec
Copy link
Collaborator

jmarrec commented Nov 8, 2024

@chriswmackey you're writting the OSM by hand or something?

m = Model.new
b = m.getBuilding
b.setName("Building")
sp = SpaceType.new(m)
sp.setName("Building").get
=> "Building 1"        /// <----- you see? it's renamed

I did mean that the OS SDK enforces the unicity already. Can you provide a MCVE where it doesn't?

@chriswmackey
Copy link
Author

chriswmackey commented Nov 8, 2024

I tested your sample and you're right this is how it works, @jmarrec. I think I understand what is happening now on my side. The issue on my end is happening when I go to assign the SpaceType to spaces in the model here. Because the name automatically changes when the space type is created, I don't get any space when I try to get the Space's Space Type by name to assign it to the Space. That's why the space type does not make it into the IDF.

Alright. I guess this is what it is and the workaround that I added will just have to be permanent.

To wrap this all up, maybe I would just want to know why OpenStudio SDK enforces unicity in this case? I didn't see this documented anywhere in the OpenStudio SDK docs and, from what I can tell, EnergyPlus does not have this limitation of space type names needing to be different from the building name. Maybe I just missed where this was documented and what the reason for it is. Could you point me to any such documentation, @jmarrec ? Or is there some explanation in the source code comments that I missed?

@jmarrec
Copy link
Collaborator

jmarrec commented Nov 12, 2024

We enforce unicity accross references.

They both use \reference SpaceAndSpaceGroupNames.

@chriswmackey
Copy link
Author

Thanks, @jmarrec . I guess my question is more "Why are both the Building and the SpaceType apart of SpaceAndSpaceGroupNames?" Putting them in the same group like this just seems like it creates more errors than it helps avoid but maybe I'm missing some important feature that relies on this unicity.

Also, is there a place where I can look up all of the references for each object? I don's see them in the docs and I sense that there may be more cases where I have to change the E+ names of things to avoid my measures breaking.

@shorowit
Copy link
Contributor

Also, is there a place where I can look up all of the references for each object?

They are all listed in the OpenStudio.idd.

@jmarrec
Copy link
Collaborator

jmarrec commented Nov 13, 2024

The question is why Building specifically is part of it.
I don't know, probably just historical, because it felt natural.

The others definitely need unicity since e+ 9.6.0: ThermalZone, Space, SpaceType and BuildingStory

TEST_F(ModelFixture, Ensure_Name_Unicity_SpaceAndSpaceGroupNames) {
// Starting in 9.6.0, Space and SpaceList are supported.
// Zone, ZoneList, Space, SpaceList all need to be unique names

It's not a real problem really, setName returns an optionalString that contains the name it has ended picking up, and as long as you don't manipulate the OSM outside of the SDK you can't break things.

But if you want to make a PR removing that reference from the Building object and all tests still pass we can consider it.

@chriswmackey
Copy link
Author

Thanks, @shorowit and @jmarrec ,

The OpenStudio.idd is very helpful and it's already helping me see some changes that I need to make in my software (eg. I didn't realize that I could also get a similar problem if a Story name matches the name of a Building, Space, Zone, or SpaceType).

I understand that it's not a problem for people who only work on OSMs through the OpenStudio SDK, @jmarrec. And, for that reason, I don't blame you for putting a low priority on documenting the unicity rules.

But the main thing that is driving sales of my company's software right now is translation of models between CAD. BIM and various BEM platforms, of which the OpenStudio translators to gbXML and IDF are a critical part. So this type of thing is a fairly big problem for me since the rules of unicity are not the same between different BEM platforms (or even between OSM and IDF as I have learned here). So I need to know these unicity rules if I'm going to try to bring names/IDs from different platforms through the OSM translators and if I'm going to use these Names/IDs to link things together. In any event, that .idd file gives me what I need for now.

I'll try to find comments like that one, @jmarrec , if I'm trying to explain to people why certain names/IDs for certain objects didn't come through. If there ends up being a more formal documentation for the reason behind these groups in the future, it's definitely something I would use. But, as I said, I understand if it's not a high priority since we're not the typical OpenStudio SDK user.

@jmarrec
Copy link
Collaborator

jmarrec commented Nov 15, 2024

But if you want to make a PR removing that reference from the Building object and all tests still pass we can consider it.

A one line PR, which I just did in 30 seconds directly from github:

let's see if the tests pass.

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

Successfully merging a pull request may close this issue.

3 participants