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
Switch to E+ Space FT by default #4589
Conversation
CI Results for 62a43cc:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch the bool flag to use the space translation, and bump to 3.4.1
LGTM!
@@ -40,7 +40,7 @@ OS:Version, | |||
\required-field | |||
A2, \field Version Identifier | |||
\type alpha | |||
\default 3.3.1 | |||
\default 3.4.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch, that was wrong defore (which isn't too big of a deal)
@@ -120,7 +120,7 @@ namespace energyplus { | |||
m_excludeSQliteOutputReport = false; | |||
m_excludeHTMLOutputReport = false; | |||
m_excludeVariableDictionary = false; | |||
m_excludeSpaceTranslation = true; | |||
m_excludeSpaceTranslation = false; // At 3.4.1, this was changed to false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual change
I started a Release Notes template, and tried to address the Windows test that failed:
|
I'm merging, but will check on the Windows runner to see if the test passed. Can be addressed on develop if needed |
* [#4589](https://github.com/NREL/OpenStudio/pull/4589) - Switch to E+ Space FT by default | ||
* OpenStudio SDK 3.3.0 added support to the new EnergyPlus 9.6.0 [space concept](https://github.com/NREL/EnergyPlus/blob/develop/design/FY2021/NFP-Spaces.md). This was an opt-in feature, as it was disabled by default. | ||
* This is now enabled by default. You can opt-out by by manually calling `ForwardTranslator::setExcludeSpaceTranslation(true)` before calling `ForwardTranslator::translateModel(Model&)` or passing the CLI argument `--no-space-translation` to the `run` method (eg: `openstudio run --no-space-translation -w workflow.osw`) | ||
* As a reminder, a few limitations due to DaylightingControls/IlluminanceMap exist: in case one of these objects is found in a ThermalZone, spaces will be combined for that ThermalZone (historical behavior) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document the change in the Release Notes
@@ -242,7 +242,7 @@ TEST_F(EnergyPlusFixture, ForwardTranslator_SpaceInfiltrationDesignFlowRate_Spac | |||
} | |||
} | |||
|
|||
EXPECT_EQ(0.687, totalInfiltration); | |||
EXPECT_DOUBLE_EQ(0.687, totalInfiltration); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to address windows runner failed test
Pull request overview
Pull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.