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

v370-rc1 polyhedron calcs #15

Closed
wants to merge 5 commits into from
Closed

v370-rc1 polyhedron calcs #15

wants to merge 5 commits into from

Conversation

brgix
Copy link
Member

@brgix brgix commented Oct 21, 2023

1x RSpec fails with OpenStudio v370-rc1: "checks for space/surface convexity"

@brgix brgix added the bug Something isn't working label Oct 21, 2023
@brgix brgix self-assigned this Oct 21, 2023
@@ -2937,7 +2937,7 @@ module M
vtx << mini_attic_vtx[2]
vtx << mini_attic_vtx[1]
vtx << mini_attic_vtx[0]
vtx << mini_attic_vtx[3]
vtx << mini_attic_vtx[3] # ?
Copy link
Member Author

@brgix brgix Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failed RSpec deals with holes cut in (larger) surfaces, to accommodate either other (smaller) surfaces or shafts (e.g. skylight wells). Roughly half of the comments/illustrations of this merged PR describe encountered pitfalls/workarounds for a handful of scenarios. The solution rests on properly winding extra vertices (of the larger surface) around a new hole.

The RSpec passes for OpenStudio versions 3.0.0 to 3.6.1, yet fails for 3.7.0-rc1 (/spec/osut_tests_spec.rb:2978):

Failure/Error: expect(attic.volume).to be_within(TOL).of(720.19) unless v < 350
       expected 798.4139451428566 to be within 0.01 of 720.19

Note that a fix to OpenStudio volume calculations was introduced in v3.5.0. Recent releases correctly calculate/report space/zone volume (in sync with EnergyPlus). For this reason, OpenStudio version checks are found in the RSpec (e.g. unless v < 350).

OpenStudio v3.7.0-rc1 reports the following messages:

[utilities.Polyhedron] <0> Polyhedron is not enclosed in original testing. Trying to add missing colinear points.
[utilities.Polyhedron] <0> Polyhedron is not enclosed.
[openstudio.model.Space] <0> Object of type 'OS:Space' and named 'Core_ZN' is not enclosed, there are 2 edges that aren't used exactly twice. Falling back to ceilingHeight * floorArea. Volume calculation will be potentially inaccurate.
[utilities.Polyhedron] <0> Polyhedron is not enclosed in original testing. Trying to add missing colinear points.
[utilities.Polyhedron] <0> Polyhedron is not enclosed.
[openstudio.model.Space] <0> Object of type 'OS:Space' and named 'Attic' is not enclosed, there are 1 edges that aren't used exactly twice. Falling back to ceilingHeight * floorArea. Volume calculation will be potentially inaccurate.

... and reverts to pre-3.5.0 volume calculations. However, a forward translated IDF (v23.2.0) of the same modified model (files/osms/out/miniX.idf) reports the right volume (720.19 m3 for the attic). This remains consistent with older EnergyPlus versions (tested for v9.5.0 up to v23.2.0).

mini1

There are no EnergyPlus-reported warnings or errors with IDFs generated using the solution, regardless of the version tested. OpenStudio v3.5.1 and v3.6.1 (with fixed volume calculations) did not report this as an issue either.

This seems to stem from revised checks discussed here. Is it possible OpenStudio has introduced additional checks (beyond what EnergyPlus checks for)? Maybe best to avoid having OpenStudio enforce more stringent tests than EnergyPlus?


If one were to comment out the final vertex addition of all 3 sets of extra wrapping vertices (e.g. vtx << mini_floor_vtx[3] # ?), then OpenStudio v3.7.0-rc1 will not log to stdout the aforementioned unenclosed polyhedron messages. However, neither OpenStudio nor EnergyPlus will then report the correct floor areas and volumes. As with previous EnergyPlus runs, there are no reported warnings or errors with this slightly-altered solution. The initial solution is obviously preferred.

file = File.join(__dir__, "files/osms/out/miniX.idf")
ft = OpenStudio::EnergyPlus::ForwardTranslator.new
idf = ft.translateModel(model)
idf.save(file, true)
Copy link
Member Author

@brgix brgix Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated v3.7.0-rc1 (OpenStudio PR #5003) runs just fine locally - nice! The PR is failing as GitHub Actions is still pulling the original v3.7.0-rc1. Keeping the test as is for now; will re-test against eventual v3.7.0 release.

@brgix brgix closed this Mar 11, 2024
@brgix brgix deleted the v370rc1 branch March 11, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant