Remove the deprecated AirWallMaterial class

[rwadhwa910]

There are two separate construction for air wall which when used gives different results for lighting load,

As you can see in the screenshot I have assumed the Lighting density to be 1 W/ft2 for atrium, Using both of them gives different results, It looks the air wall material taken form library gives accurate result as the summation of the lighting loads results to 1W/ft2 unlike the construction material which gives more than 4W/ft2. Not sure about the radiance though.

Current Behavior

Different Lighting load while using different set of air wall available.

Expected Behavior

There should be only 1 air wall and it should be the one that is available in library as air boundary construction as it gives more options like zone grouping and air mixing.

Steps to Reproduce

1.Make an OSM model. 2.Assign a lighting density to a space (multifloor) 3.Use air wall from construction for ceiling and floor. 4.Use air boundary from library and compare the results

Possible Solution

Have only 1 air wall construction.

Details

Windows, 10 Open studio app 1,0,1, 3.0.1

[DavidGoldwasser]

@kbenne we can talk about this in iteration meeting on Friday. This relates to when spaces are stacked in a zone for large space like atrium no floor or partial floor on upper stories. There is logic in (or I think prior to) forward translation when space types are expanded. When an air wall (old air wall material in construction) is used it deducts that floor area from the load calculations that are per floor area from objects like lights, equipment, people, ventilation, and infiltration.

In adding the new air wall EnergyPlus capability it was added as a new object, when it would have been better to have it replace the old air wall so there are not two different air wall workflows. More importantly it didn't tie to the same logic. The fix could be to tie the new air wall and old air wall to same logic, but I would lean toward deprecating and removing the old one. Version translation from 3.0 to 3.1 would have to manage that.

Note: while we have identified this space type issue it is likely the same issue exists in Radiance workflows.

Maybe it will be easy and an update to the isAirWall method for planer surface will fix this, if the other code relies on this check. I do know a number of measures use this check. https://openstudio-sdk-documentation.s3.amazonaws.com/cpp/OpenStudio-3.0.0-doc/model/html/classopenstudio_1_1model_1_1_planar_surface.html#a2a4dac9c598d7b40237d6fd71a0a17b0

There could be changes that OpenStudio Coalition may want to make to the SketchUp Plugin related to this, not sure if any impact on OpenStudio Application.

[jmarrec]

Moved to https://github.com/openstudiocoalition/OpenStudioApplication/issues/323

[DavidGoldwasser]

@jmarrec there is an issue in the OpenStudio SDK, about isAirWall and also how forward translate treats both kinds of air walls, (specifically impacts stacked spaces). So while there may be change in the app, the issue isn't limited to OS app.

[jmarrec]

Given this: https://github.com/NREL/OpenStudio/blob/7cec475621233fcd772de2c6715ef9252bcbb7a6/src/model/PlanarSurface.cpp#L214-L245

Is anything else really needed @DavidGoldwasser? because I can't see anything that would be missing personally. If you're satisfied, please close.

[DavidGoldwasser]

@jmarrec Before EnergyPlus had air wall, OpenStudios version of an air wall was just a layered construction with a specific material type as shown in this screenshot.

When EnergyPlus added an air wall and that was supported in OpenStudio it was added as a new construction type, and not using a layered construction object at all as shown below.

It looks like the code above is both checking for ConstructionAirBoundary and a layered construction with AirWallMaterial. Not sure what the third check is of layered construction with no layers, but I would not want to assume that is an air wall, and it should fail when a simulation runs.

I think a mistake was made when we supported energy plus air wall to make a a parallel path to the existing air wall. We should have either kept it as a material and added these fields or made the new ConstructionAirBoundary object, and version translate OSM files to use that and kill the air wall material. Having both is just confusing to a user. If we want to keep the old one in the model I suppose we can, but maybe interfaces can only show the new approach, which provides the option for airflow across the air wall, which doesn't happen with the old approach (which is just plywood like material).

[DavidGoldwasser]

@jmarrec I guess unless you want to undertake deprecating the old object, or merging these, then it is fine to close this

but if the code below was always in then we should test it and see why results were not as expected.

     boost::optional<ConstructionAirBoundary> constructionAirBoundary = 
       oConstruction->optionalCast<ConstructionAirBoundary>();
      if (constructionAirBoundary) {    
        return true;
      } 

Maybe forward translation is not using isAirWall method and is using a different approach, which also needs to be updated. Should be able to test with an OSM with two stacked spaces included as part of a single zone. make the ceiling/floor between them an air wall and look and EUI and building square footage. This was not working as described at the beginning of this issue. But I don't know if the code above was added after the issue was filed, or if it already existing.

The test might need to use two different space types to trigger the blending of space types that OpenStudio does prior to forward translation which may convert area normalized loads to watts.

[jmarrec]

It is confusing indeed to have both. I found the commit where this happened: https://github.com/NREL/OpenStudio/commit/bf17b66549ddc5c7a5badb7f0a84d12b93b44e81

It seems instead of doing VT to actually change the AirWallMaterial to ConstructionAirBoundary and remove AirWallMaterial class altogether, the AirWallMaterial model class was marked deprecated, and the replacement to CosntructionAirBoundary is actually done in the ForwardTranslator. This was done to avoid API breakage: if people had a measure that used the AirWallMaterial, suddenly it would have been broken. Makes sense...

https://github.com/NREL/OpenStudio/blob/70a5549c439eda69d6c514a7275254f71f7e3d2b/src/energyplus/ForwardTranslator.cpp#L208

This happened in 2.9. I think we do need to remove AirWallMaterial altogether, but this would be perhaps better done for the next major version? @kbenne Dear protector of the API realm, thoughts please? Personally I think having almost two years of this message https://github.com/NREL/OpenStudio/blob/70a5549c439eda69d6c514a7275254f71f7e3d2b/src/model/AirWallMaterial.cpp#L70 is enough to remove it in the next release...

I'm going to retitle the issue to be more clear and mark it "APIChange" so that next time we consider breaking API to remove deprecated stuff, we find it.