V26.2.0-IOFreeze: new ZoneMRTCalculation object by joseph-robertson · Pull Request #5649 · NatLabRockies/OpenStudio

joseph-robertson · 2026-09-01T21:36:29Z

Pull request overview

Fixes #ISSUENUMBERHERE (IF THIS IS A DEFECT)

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

github-actions · 2026-09-01T21:36:56Z

🧪 Test Results Dashboard

Summary

Metric	Value
Total Tests	0
Passed	0
Failed	0
Errors	0
Skipped	0
Success Rate	0.0%
Generated	2026-09-02 04:15:31 UTC

❌ Significant Test Failures

📊 Test Run Information

Run	XML File	Status
-	No XML files found	❌ Missing

Copilot

🟡 Changes recommended

Weight-sum validation, stale references, non-finite values, and partial-add failure reporting can currently produce invalid behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Model API and EnergyPlus translation support for ZoneMRTCalculation.

Changes:

Adds the model object, MRT weighting-factor API, and ThermalZone integration.
Adds forward/reverse translators and binding support.
Updates IDDs, build registration, and tests.

File summaries

File	Description
`src/model/ZoneMRTCalculation.hpp`	Declares the new API.
`src/model/ZoneMRTCalculation.cpp`	Implements weighting-factor behavior.
`src/model/ZoneMRTCalculation_Impl.hpp`	Declares implementation details.
`src/model/ThermalZone.hpp`	Exposes MRT calculation access.
`src/model/ThermalZone.cpp`	Implements creation and lookup.
`src/model/ThermalZone_Impl.hpp`	Adds implementation declaration.
`src/model/test/ZoneMRTCalculation_GTest.cpp`	Tests the model API.
`src/model/ModelHVAC.i`	Adds C# ThermalZone access.
`src/model/ModelGeometry.i`	Registers binding templates.
`src/model/Model.cpp`	Registers constructors.
`src/model/ConcreteModelObjects.hpp`	Includes the new object.
`src/model/CMakeLists.txt`	Adds model sources and tests.
`src/energyplus/Test/ZoneMRTCalculation_GTest.cpp`	Tests both translators.
`src/energyplus/ReverseTranslator/ReverseTranslateZoneMRTCalculation.cpp`	Implements reverse translation.
`src/energyplus/ReverseTranslator.hpp`	Declares reverse translation.
`src/energyplus/ReverseTranslator.cpp`	Dispatches reverse translation.
`src/energyplus/ForwardTranslator/ForwardTranslateZoneMRTCalculation.cpp`	Implements forward translation.
`src/energyplus/ForwardTranslator.hpp`	Declares forward translation.
`src/energyplus/ForwardTranslator.cpp`	Registers forward translation.
`src/energyplus/CMakeLists.txt`	Adds translator sources and tests.
`resources/model/OpenStudio.idd`	Defines the OpenStudio object.
`resources/energyplus/ProposedEnergy+.idd`	Converts EnergyPlus fields to extensible groups.

Review details

Suppressed comments (1)

src/model/ZoneMRTCalculation.cpp:107

Removing a referenced People object clears this pointer but leaves the extensible group. A subsequent call to addMRTWeightingFactor reaches this predicate and calls .get() on an empty optional, throwing instead of adding the new factor. Treat a missing field as a non-match (and add a removal regression test).

      auto it = std::find_if(egs.begin(), egs.end(), [&](const WorkspaceExtensibleGroup& eg) {
        return eg.getField(OS_ZoneMRTCalculationExtensibleFields::PeopleName).get() == peopleHandle;
      });

Files reviewed: 22/22 changed files
Comments generated: 4
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

joseph-robertson · 2026-09-02T04:16:12Z

+    for (const MRTWeightingFactor& mrtWeightingFactor : mrtWeightingFactors) {
+      People people = mrtWeightingFactor.people();
+      boost::optional<IdfObject> _people = translateAndMapModelObject(people);
+
+      if (_people) {


joseph-robertson · 2026-09-02T04:16:39Z

+        return result;
+      }
+
+      boost::optional<unsigned> existingIndex = mrtWeightingFactorIndex(mrtWeightingFactor);


…m, per review.

…tWeightingFactorIndex to use just People, update model tests.

joseph-robertson · 2026-09-02T04:19:59Z

+    // The hooks below would remove ZoneMRTCalculation extensible groups as soon as a referenced People object
+    // becomes invalid by being moved to another Space, reset from its Space, assigned to a SpaceType, or losing
+    // its thermal comfort model types. That is more proactive than most existing extensible-reference patterns in
+    // the model, which generally clean references only from the referenced object's remove() path and otherwise
+    // rely on explicit remove APIs or read/translation-time filtering.


Not sure if we'd want this -- it may be overkill.

Copilot

🟡 Changes recommended

Thermal-zone deletion can leave an invalid ZoneMRTCalculation object that later causes assertions during access or translation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

resources/model/OpenStudio.idd:6830

This field declares two conflicting \type values; all other OpenStudio object-list fields use only \type object-list (for example, lines 6808-6809 and 6837-6838). Remove the redundant alpha declaration so the IDD metadata has one unambiguous field type.
src/model/ZoneMRTCalculation.cpp:85
These adjacent string literals concatenate as andthat, making the thrown clone error malformed. Preserve a space at the literal boundary.

Files reviewed: 25/25 changed files
Comments generated: 1
Review effort level: Balanced

joseph-robertson · 2026-09-02T05:15:59Z

+    ZoneMRTCalculation ThermalZone_Impl::getZoneMRTCalculation() const {
+      auto thisThermalZone = getObject<ThermalZone>();
+      std::vector<ZoneMRTCalculation> zoneMRTCalculations =
+        thisThermalZone.getModelObjectSources<ZoneMRTCalculation>(ZoneMRTCalculation::iddObjectType());


I think we'd have the same issue when deleting a Space.

Removing a Space bulk-removes its child People through ParentObject_Impl::remove(), but it does not call People_Impl::remove(). So cleanup would require Space_Impl::remove() to inspect all child People, then find and prune any ZoneMRTCalculation groups for each one. That reaches across more ownership boundaries and is less consistent with the existing bulk child-removal mechanism.

So: zone removal is direct source-object cleanup; space removal is indirect child/reference cleanup.

joseph-robertson added 5 commits September 1, 2026 14:07

Update both IDDs for new ZoneMRTCalculation object.

afefd66

Add model files for new ZoneMRTCalculation object.

03627f1

Add ft and rt for new ZoneMRTCalculation object.

23d26d2

Add model and translation tests for new ZoneMRTCalculation object.

0c8684f

Expand new tests to cover more throw/error scenarios.

5e92cbc

joseph-robertson self-assigned this Sep 1, 2026

joseph-robertson added Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. IDDChange labels Sep 1, 2026

joseph-robertson added 2 commits September 1, 2026 14:44

Another pass through model and tests with additions and cleanups.

222ac39

Address updates in ft and test, and model cpp.

0f9a938

joseph-robertson requested a balanced review from Copilot September 1, 2026 22:20

joseph-robertson marked this pull request as ready for review September 1, 2026 22:20

Copilot started reviewing on behalf of joseph-robertson September 1, 2026 22:20 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

joseph-robertson added 3 commits September 1, 2026 15:52

Updates related to stale people references, nan, weighting factors su…

d56eff7

…m, per review.

Override people remove so we can modify zone mrt extensible groups.

c5bc900

Add optional proactive extensible group removal for People, update mr…

445c0f6

…tWeightingFactorIndex to use just People, update model tests.

joseph-robertson commented Sep 2, 2026

View reviewed changes

joseph-robertson requested a balanced review from Copilot September 2, 2026 04:32

Copilot started reviewing on behalf of joseph-robertson September 2, 2026 04:32 View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Navigation Menu

Uh oh!

V26.2.0-IOFreeze: new ZoneMRTCalculation object - #5649

V26.2.0-IOFreeze: new ZoneMRTCalculation object#5649
joseph-robertson wants to merge 10 commits into
developfrom
v26.2.0-IOFreeze-ZoneMRTCalc

joseph-robertson commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

joseph-robertson Sep 2, 2026

Uh oh!

Uh oh!

joseph-robertson Sep 2, 2026

Uh oh!

Uh oh!

joseph-robertson Sep 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

joseph-robertson Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

joseph-robertson commented Sep 1, 2026

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 Test Results Dashboard

Summary

❌ Significant Test Failures

📊 Test Run Information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

joseph-robertson Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joseph-robertson Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joseph-robertson Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

joseph-robertson Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 1, 2026 •

edited

Loading