#5368 - Wrap OutputTableAnnual and OutputTableMonthly #5369

jmarrec · 2025-03-17T20:20:19Z

Pull request overview

Fixes Wrap E+ Output:Table:Monthly #5368
Wrap OutputTableAnnual and OutputTableMonthly in the SDK

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: #5368 - Wrap OutputTableAnnual and OutputTableMonthly OpenStudio-resources#217
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp): N/A
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

```bash $os_build3/Products/openstudio GenerateClass.rb -c "OutputTableMonthly" -b "ModelObject" -i "OS:Output:Table:Monthly" -s model -o /Users/julien/Software/Others/OpenStudio3/src/model/ -p -f -r ` ``

…once. Technically speaking, there should be a non-advanced aggregation type in between both, but it's way overkilled to try to enforce that

jmarrec · 2025-03-17T21:02:47Z

src/model/OutputTableMonthly.cpp

+      boost::optional<unsigned> existingIndex_ = monthlyVariableGroupIndex(monthlyVariableGroup);
+      if (existingIndex_) {
+        LOG(Warn, "For " << briefDescription() << ", MonthlyVariableGroup already exists: " << monthlyVariableGroup);
+        return false;
+      }


AANNND I just realized that this is too restrictrive, because you have the so called "Advanced Aggregation" types.

One example of which: https://github.com/NREL/EnergyPlus/blob/116998ec7fcec5435df31d05505bd4b38837276b/datasets/StandardReports.idf#L69-L87

[openstudio.model.OutputTableMonthly] <0> For Object of type 'OS:Output:Table:Monthly' and named 'Zone Cooling Summary', MonthlyVariableGroup already exists: (Output Variable or Meter = 'Site Outdoor Air Drybulb Temperature', Aggregation Type = 'ValueWhenMaximumOrMinimum')
[openstudio.energyplus.ReverseTranslator] <0> Extensible group 6(0-indexed) is already present, not adding it twice
[openstudio.model.OutputTableMonthly] <0> For Object of type 'OS:Output:Table:Monthly' and named 'Zone Cooling Summary', MonthlyVariableGroup already exists: (Output Variable or Meter = 'Site Outdoor Air Wetbulb Temperature', Aggregation Type = 'ValueWhenMaximumOrMinimum')

I implemented a bypass check for that in 586445b

I thought about just removing it, but it was actually easier (or at least not more complicated) to handle it

…orts.idf

* NREL/OpenStudio#5368 * Companion PR: NREL/OpenStudio#5369

joseph-robertson

Looks good to me. Just a couple questions.

joseph-robertson · 2025-03-18T20:00:21Z

src/energyplus/ForwardTranslator/ForwardTranslateOutputTableAnnual.cpp

+    if (modelObject.numberofAnnualVariableGroups() == 0) {
+      return boost::none;
+    }


So at least 1 group must be present to FT this object. And the IDD requires at least 1 group. The ctor doesn't add any groups. No message is issued when there aren't any groups; the object is just skipped? (I'm not saying this is wrong. I'm just confirming.)

Yes. I figured issuing a LOG(Warn or Info would be kinda pointless and I know we have people in the ecosystem who are very sensitive to not having useless warnings.

I feel like having a blank OutputTable is quite harmless.

Maybe I should issue a log anyways?

The ctor doesnt push an extensible group because it'd be quite annoying to have to clear the extensible groups before adding yours.

This isn't that unsual to have the minFields set to indicate that there should be at least one extensible group, but the ctor doesn't add it:

factory = OpenStudio::IddFileAndFactoryWrapper.new("OpenStudio".to_IddFileType) factory.objects.select {|o| o.numFields < o.properties.minFields}.map(&:name) => ["OS:ComponentData", "OS:WindowMaterial:GlazingGroup:Thermochromic", "OS:Construction", "OS:Construction:InternalSource", "OS:Schedule:Compact", "OS:ShadingSurface", "OS:Surface", "OS:SubSurface", "OS:ShadingControl", "OS:ZoneProperty:UserViewFactors:BySurfaceName", "OS:SurfaceProperty:SurroundingSurfaces", "OS:SurfaceProperty:GroundSurfaces", "OS:AirLoopHVAC:DedicatedOutdoorAirSystem", "OS:GroundHeatExchanger:Vertical", "OS:Coil:Cooling:DX:CurveFit:OperatingMode", "OS:Connector:Mixer", "OS:Connector:Splitter", "OS:WaterUse:Connections", "OS:UtilityCost:Charge:Block", "OS:UtilityCost:Computation", "OS:EnergyManagementSystem:ProgramCallingManager", "OS:EnergyManagementSystem:Program", "OS:EnergyManagementSystem:Subroutine", "OS:PythonPlugin:Variable", "OS:AirflowNetworkDetailedOpening"]

m = Model.new ems = OpenStudio::Model::EnergyManagementSystemProgram.new(m) puts ems OS:EnergyManagementSystem:Program, {b988fdde-8d6b-4776-97fd-5e28e3e48316}, !- Handle Energy_Management_System_Program_1; !- Name puts ems.iddObject OS:EnergyManagementSystem:Program, \memo This input defines an Erl program \memo Each field after the name is a line of EMS Runtime Language \extensible:1 \min-fields 3 A1, \field Handle \type handle \required-field A2, \field Name \note No spaces allowed in names for ErlProgramNames \type alpha \required-field \reference ErlProgramNames \reference ErlProgramSubroutineNames A3; \field Program Line \type alpha \required-field \begin-extensible

joseph-robertson · 2025-03-18T20:00:59Z

src/model/OutputTableAnnual.hpp

+  class MODEL_API AnnualVariableGroup
+  {
+   public:
+    AnnualVariableGroup(std::string variableorMeterorEMSVariableorField, std::string aggregationType = "SumOrAverage", int digitsAfterDecimal = 2);


Name is required, but aggregation type and digits have default values.

"SumOrAverage" is accoridng to the I/O reference, the most common option.

Digits after decimal = 2 is the E+ IDD default

https://bigladdersoftware.com/epx/docs/24-2/input-output-reference/output-table-annual.html#field-aggregation-type-for-variable-or-meter

SumOrAverage is probably the most common choice for aggregation type.

factory = OpenStudio::IddFileAndFactoryWrapper.new("EnergyPlus".to_IddFileType) puts factory.getObject("Output:Table:Annual").get

truncated output:

Output:Table:Annual, \extensible:3 A1, \field Name A2, \field Filter A3, \field Schedule Name A4, \field Variable or Meter or EMS Variable or Field Name \type external-list + \begin-extensible \external-list autoRDDvariableMeter A5, \field Aggregation Type for Variable or Meter \type choice \key SumOrAverage \key Maximum [... other keys but no default ...] N1; \field Digits After Decimal \type integer \minimum 0 \maximum 10 + \default 2

joseph-robertson · 2025-03-18T20:01:49Z

src/model/OutputTableMonthly.hpp

+  class MODEL_API MonthlyVariableGroup
+  {
+   public:
+    MonthlyVariableGroup(std::string variableOrMeterName, std::string aggregationType = "SumOrAverage");


Monthly doesn't have a default value for digits because this field doesn't exist like it does for Annual.

Edit: Digits doesn't exist at the group level. It does exist at the object level.

Yes, it is an object one here, not per extensible group (one of the weirdness of E+...), and that one is set in the ctor

OpenStudio/src/model/OutputTableMonthly.cpp

Lines 822 to 827 in ea0e2a7

OutputTableMonthly::OutputTableMonthly(const Model& model) : ModelObject(OutputTableMonthly::iddObjectType(), model) {

OS_ASSERT(getImpl<detail::OutputTableMonthly_Impl>());

bool ok = true;

ok = setDigitsAfterDecimal(2);

OS_ASSERT(ok);

}

cf NREL/OpenStudio-resources#217 (comment)

jmarrec · 2025-03-19T09:24:37Z

I'm going to merge this after CI completes, we have too many open PRs that are related. Happy to hotfix anything if my answers weren't satisfying to you @joseph-robertson though. And thanks for the review.

ci-commercialbuildings · 2025-03-19T09:35:50Z

CI Results for 1445362:

Ubuntu 24.04:
- OpenStudio-3.10.0-alpha+1445362e6e-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.10.0-alpha+1445362e6e-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
Darwin:
Windows:
Code Coverage:

jmarrec added 10 commits March 17, 2025 14:04

Naive add Output:Table:Monthly and Annual

6cc528b

GenerateClass.rb

a0d7948

```bash $os_build3/Products/openstudio GenerateClass.rb -c "OutputTableMonthly" -b "ModelObject" -i "OS:Output:Table:Monthly" -s model -o /Users/julien/Software/Others/OpenStudio3/src/model/ -p -f -r ` ``

Modify IDD

f0bf1c1

model API for OutputTableMonthly

28f2560

Model tests for OutputTableMonthly

ae929d8

SWIG it in ModelCore

c6df0c0

FT + RT + tests for OutputTableMonthly

acc2a5a

Model Implementation for OutputTableAnnual

1429817

FT and RT + Tests for OutputTableAnnual

f48fccf

Swig the helper classes

74d0e06

jmarrec added Enhancement Request component - Model component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. IDDChange labels Mar 17, 2025

jmarrec self-assigned this Mar 17, 2025

jmarrec added 5 commits March 17, 2025 21:25

Update TDB release notes a bit

d125bc4

missing ' in ostream.

c0d7e8d

clang-format

958b921

cppcheck

e1475f3

Handle advanced aggregation types, which should be allowed more than …

586445b

…once. Technically speaking, there should be a non-advanced aggregation type in between both, but it's way overkilled to try to enforce that

jmarrec commented Mar 17, 2025

View reviewed changes

jmarrec added 3 commits March 17, 2025 23:22

Add a Factory method to create the reports in E+ datasets/StandardRep…

9fe3714

…orts.idf

Minor tweaks

6c57fa2

Remove TODOs

ea0e2a7

jmarrec added a commit to NREL/OpenStudio-resources that referenced this pull request Mar 17, 2025

#5368 - Wrap OutputTableAnnual and OutputTableMonthly

7bb6f5e

* NREL/OpenStudio#5368 * Companion PR: NREL/OpenStudio#5369

jmarrec mentioned this pull request Mar 17, 2025

#5368 - Wrap OutputTableAnnual and OutputTableMonthly NREL/OpenStudio-resources#217

Merged

19 tasks

jmarrec requested review from joseph-robertson and shorowit March 17, 2025 23:14

jmarrec added the component - Measures label Mar 18, 2025

joseph-robertson approved these changes Mar 18, 2025

View reviewed changes

Ensure ScheduleTypeRegistry is working correctly.

1445362

cf NREL/OpenStudio-resources#217 (comment)

jmarrec merged commit 5777550 into develop Mar 19, 2025
3 of 6 checks passed

jmarrec deleted the 5368-OutputTable branch March 19, 2025 09:37

	OutputTableMonthly::OutputTableMonthly(const Model& model) : ModelObject(OutputTableMonthly::iddObjectType(), model) {
	OS_ASSERT(getImpl<detail::OutputTableMonthly_Impl>());
	bool ok = true;
	ok = setDigitsAfterDecimal(2);
	OS_ASSERT(ok);
	}

#5368 - Wrap OutputTableAnnual and OutputTableMonthly #5369

#5368 - Wrap OutputTableAnnual and OutputTableMonthly #5369

Uh oh!

Conversation

jmarrec commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-robertson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarrec Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jmarrec Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jmarrec commented Mar 19, 2025

Uh oh!

ci-commercialbuildings commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmarrec commented Mar 17, 2025 •

edited

Loading

jmarrec Mar 19, 2025 •

edited

Loading

jmarrec Mar 19, 2025 •

edited

Loading

ci-commercialbuildings commented Mar 19, 2025 •

edited

Loading