Address #4630, wrap Output:Schedules and Output:Constructions objects by joseph-robertson · Pull Request #4685 · NatLabRockies/OpenStudio

joseph-robertson · 2022-09-16T21:09:23Z

Pull request overview

Fixes Wrap Output:Schedules and Output:Constructions objects #4630

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

jmarrec

A straightforward addition. Use a const string&, test that having a blank line in the Output:Constructions object is fine, and it should be good

jmarrec · 2022-09-26T09:33:54Z

src/energyplus/ForwardTranslator.cpp

+    result.push_back(IddObjectType::OS_Output_Schedules);
+    result.push_back(IddObjectType::OS_Output_Constructions);


Yes, need to always FT them

jmarrec · 2022-09-26T09:39:46Z

resources/model/OpenStudio.idd

+OS:Output:Constructions,
+       \memo Adds a report to the eio output file which shows details for each construction,
+       \memo including overall properties, a list of material layers, and calculated results
+       \memo related to conduction transfer functions.
+       \format singleLine
+       \unique-object
+  A1,  \field Handle
+       \type handle
+       \required-field
+  A2,  \field Constructions
+       \type choice
+       \required-field
+       \key No
+       \key Yes
+  A3;  \field Materials
+       \type choice
+       \required-field
+       \key No
+       \key Yes


Thanks a lot for using a sane IDD object unlike the E+ object which is rather weird.

For reference the E+ object is like this:

Output:Constructions, \memo Adds a report to the eio output file which shows details for each construction, \memo including overall properties, a list of material layers, and calculated results \memo related to conduction transfer functions. \format singleLine A1, \field Details Typte 1 \type choice \key Constructions \key Materials A2; \field Details Type 2 \type choice \key Constructions \key Materials

Maybe Report For Constructions would be clearer. But I don't really care.

jmarrec · 2022-09-26T09:41:56Z

src/energyplus/ForwardTranslator/ForwardTranslateOutputConstructions.cpp

+    if (modelObject.constructions()) {
+      idfObject.setString(Output_ConstructionsFields::DetailsType1, "Constructions");
+    }
+
+    if (modelObject.materials()) {
+      idfObject.setString(Output_ConstructionsFields::DetailsType2, "Materials");
+    }


Have you tested whether this works when you have Constructions = No and Materials = yes? This leads to this IDF object:

Output:Constructions, , !- Details Type 1 Materials; !- Details Type 2

jmarrec · 2022-09-26T09:43:41Z

src/energyplus/ForwardTranslator/ForwardTranslateOutputConstructions.cpp

+    // If nothing to write, don't
+    bool constructions = modelObject.constructions();
+    bool materials = modelObject.materials();
+    if (!constructions && !materials) {
+      return boost::none;
+    }


I assume E+ is unhappy if you end up with a blank object?

jmarrec · 2022-09-26T09:46:03Z

src/model/OutputSchedules.cpp

+      if (result.empty()) {
+      }


Suggested change

if (result.empty()) {

}

jmarrec · 2022-09-26T09:46:35Z

src/model/OutputSchedules.hpp

+    /** @name Setters */
+    //@{
+
+    bool setKeyField(std::string keyField);


Suggested change

bool setKeyField(std::string keyField);

bool setKeyField(const std::string& keyField);

jmarrec · 2022-09-26T09:50:05Z

src/model/OutputConstructions.cpp

+    bool ok = setConstructions(true);
+    OS_ASSERT(ok);
+    ok = setMaterials(false);
+    OS_ASSERT(ok);


Makes sense to me to at least enable one

jmarrec · 2022-09-26T09:53:37Z

src/energyplus/ReverseTranslator/ReverseTranslateOutputConstructions.cpp

+    if (boost::optional<std::string> _detailsType1 = workspaceObject.getString(Output_ConstructionsFields::DetailsType1, true)) {
+      if (istringEqual("Constructions", _detailsType1.get())) {
+        modelObject.setConstructions(true);
+      } else if (istringEqual("Materials", _detailsType1.get()) {


build error here, missing a closing )

…ut_objects This will NOT affect EUI in any way so it's fine to add it there. Companion PR: NatLabRockies/OpenStudio#4685

ci-commercialbuildings · 2022-09-28T12:46:23Z

CI Results for eab1c24:

Ubuntu 18.04:
- OpenStudio-3.5.0-alpha+eab1c244f6-Ubuntu-18.04.deb
- OpenStudio-3.5.0-alpha+eab1c244f6-Ubuntu-18.04.tar.gz
Ubuntu 20.04:
Darwin:
Windows:
Code Coverage:

jmarrec · 2022-09-28T12:51:08Z

I wrapped up the PR, added a test on NatLabRockies/OpenStudio-resources#183. Ubuntu is happy, so is my extensive local testing. merging

joseph-robertson added Enhancement Request component - Model Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Sep 16, 2022

joseph-robertson self-assigned this Sep 16, 2022

joseph-robertson mentioned this pull request Sep 16, 2022

Update to EnergyPlus 22.2.0 #4649

Closed

17 tasks

jmarrec requested changes Sep 26, 2022

View reviewed changes

joseph-robertson and others added 13 commits September 28, 2022 13:29

Add new objects to idd.

8fbfd59

Stub new model source files.

4452020

Update all model resource files.

faff2db

Formatting.

44579a8

Changes to the idd.

b242000

Updates to model and tests.

78d104e

Add ft, rt, and test files.

6817163

Formatting.

d2ef114

Fix a few typos.

0fb6d8c

Declares in ft and rt files.

8db35b3

Fix RT and use a loop

9d9901f

FT: avoid a blank

93d5207

Modernize code in model for Output:Schedules and Output:Constructions

c30da65

jmarrec marked this pull request as ready for review September 28, 2022 11:41

jmarrec force-pushed the output-schs-constrs branch from a5d984f to c30da65 Compare September 28, 2022 11:41

RT/FT adjustments and testing

070f427

Adjust names of OutputConstruction fields (and methods) for clarity

eab1c24

NatLabRockies deleted a comment from ci-commercialbuildings Sep 28, 2022

jmarrec mentioned this pull request Sep 28, 2022

Add Output:Constructions and Output:Schedules NatLabRockies/OpenStudio-resources#183

Merged

23 tasks

jmarrec merged commit 515b977 into develop Sep 28, 2022

jmarrec deleted the output-schs-constrs branch September 28, 2022 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Address #4630, wrap Output:Schedules and Output:Constructions objects#4685

Address #4630, wrap Output:Schedules and Output:Constructions objects#4685
jmarrec merged 15 commits intodevelopfrom
output-schs-constrs

joseph-robertson commented Sep 16, 2022

Uh oh!

jmarrec left a comment

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

jmarrec Sep 26, 2022

Uh oh!

ci-commercialbuildings commented Sep 28, 2022 •

edited

Loading

Uh oh!

jmarrec commented Sep 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		result.push_back(IddObjectType::OS_Output_Schedules);
		result.push_back(IddObjectType::OS_Output_Constructions);

	bool setKeyField(std::string keyField);
	bool setKeyField(const std::string& keyField);

Comments

Conversation

joseph-robertson commented Sep 16, 2022

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

jmarrec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ci-commercialbuildings commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmarrec commented Sep 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ci-commercialbuildings commented Sep 28, 2022 •

edited

Loading