Remove allowedObjects check from `energyPlusOutputRequests` #5358

eringold · 2025-03-03T17:11:05Z

Pull request overview

Fixes Remove IddObjectType limits from ReportingMeasure energyPlusOutputRequests #5357

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

@eringold (and @shorowit) I made changes to handle unique objects and wrote tests. Could you review/test it to see if this still matches your expectations please?

jmarrec · 2025-03-17T08:52:32Z

src/workflow/Util.cpp

+  // If already present, don't do it
+  for (const auto& wo : workspace.getObjectsByType(iddObjectType)) {
+    if (idfObject.dataFieldsEqual(wo)) {
+      return false;
    }
+  }

-    workspace.addObject(idfObject);
+  workspace.addObject(idfObject);


This can't work like this. We still have to merge the unique ones

jmarrec · 2025-03-17T10:13:06Z

src/workflow/Util.cpp


  for (const auto& newReport : reportsToAdd) {
-    if (std::find(reports.cbegin(), reports.cend(), newReport) != reports.cend()) {
+    if (std::find(reports.cbegin(), reports.cend(), newReport) == reports.cend()) {


This was an existing logic error. Found it because I wrote a test.

jmarrec · 2025-03-17T10:13:20Z

src/workflow/test/Util_GTest.cpp

+TEST_F(WorkflowFixture, Util_mergeOutputTableSummaryReports) {
+  IdfObject existingObject(IddObjectType::Output_Table_SummaryReports);
+  existingObject.pushExtensibleGroup({"InputVerificationandResultsSummary"});
+  existingObject.pushExtensibleGroup({"ClimaticDataSummary"});
+
+  IdfObject newObject(IddObjectType::Output_Table_SummaryReports);
+  newObject.pushExtensibleGroup({"ComponentSizingSummary"});
+  newObject.pushExtensibleGroup({"ClimaticDataSummary"});
+  newObject.pushExtensibleGroup({"EnvelopeSummary"});
+
+  EXPECT_TRUE(openstudio::workflow::util::mergeOutputTableSummaryReports(existingObject, newObject));
+  ASSERT_EQ(4, existingObject.numExtensibleGroups());
+  auto egs = existingObject.extensibleGroups();
+  EXPECT_EQ("InputVerificationandResultsSummary", egs.at(0).getString(0).get());
+  EXPECT_EQ("ClimaticDataSummary", egs.at(1).getString(0).get());
+  EXPECT_EQ("ComponentSizingSummary", egs.at(2).getString(0).get());
+  EXPECT_EQ("EnvelopeSummary", egs.at(3).getString(0).get());
+}


New test for mergeOutputTableSummaryReports

jmarrec · 2025-03-17T10:13:51Z

src/workflow/test/Util_GTest.cpp

+TEST_F(WorkflowFixture, Util_addEnergyPlusOutputRequest) {
+
+  Workspace w(StrictnessLevel::Draft, IddFileType::EnergyPlus);
+  std::vector<IdfObject> energyPlusOutputRequests;
+  {
+    // Unique with explicit merge
+    WorkspaceObject wo = w.addObject(IdfObject(IddObjectType::Output_Table_SummaryReports)).get();
+    wo.pushExtensibleGroup({"InputVerificationandResultsSummary"});
+    wo.pushExtensibleGroup({"ClimaticDataSummary"});
+
+    auto& newObject = energyPlusOutputRequests.emplace_back(IddObjectType::Output_Table_SummaryReports);
+    newObject.pushExtensibleGroup({"ComponentSizingSummary"});
+    newObject.pushExtensibleGroup({"ClimaticDataSummary"});
+    newObject.pushExtensibleGroup({"EnvelopeSummary"});
+
+    EXPECT_TRUE(openstudio::workflow::util::addEnergyPlusOutputRequest(w, newObject));
+    {
+      auto wos = w.getObjectsByType(IddObjectType::Output_Table_SummaryReports);
+      ASSERT_EQ(1, wos.size());
+      auto egs = wos.front().extensibleGroups();
+      EXPECT_EQ("InputVerificationandResultsSummary", egs.at(0).getString(0).get());
+      EXPECT_EQ("ClimaticDataSummary", egs.at(1).getString(0).get());
+      EXPECT_EQ("ComponentSizingSummary", egs.at(2).getString(0).get());
+      EXPECT_EQ("EnvelopeSummary", egs.at(3).getString(0).get());
+    }
+  }
+  {
+    // Unique and present already
+    WorkspaceObject wo = w.addObject(IdfObject(IddObjectType::Output_SQLite)).get();
+    wo.setString(0, "Simple");
+
+    auto& newObject = energyPlusOutputRequests.emplace_back(IddObjectType::Output_SQLite);
+    newObject.setString(0, "SimpleAndTabular");
+    newObject.setString(1, "JtoKWH");
+
+    EXPECT_TRUE(openstudio::workflow::util::addEnergyPlusOutputRequest(w, newObject));
+    {
+      auto wos = w.getObjectsByType(IddObjectType::Output_SQLite);
+      ASSERT_EQ(1, wos.size());
+      const auto& new_wo = wos.front();
+      EXPECT_EQ("SimpleAndTabular", new_wo.getString(0).get());
+      EXPECT_EQ("JtoKWH", new_wo.getString(1).get());
+    }
+  }
+  {
+    // Unique and not present already
+    auto& newObject = energyPlusOutputRequests.emplace_back(IddObjectType::Output_Diagnostics);
+    newObject.setString(0, "DisplayExtraWarnings");
+    newObject.setString(1, "DisplayUnusedSchedules");
+
+    EXPECT_EQ(0, w.getObjectsByType(IddObjectType::Output_Diagnostics).size());
+    EXPECT_TRUE(openstudio::workflow::util::addEnergyPlusOutputRequest(w, newObject));
+    {
+      auto wos = w.getObjectsByType(IddObjectType::Output_Diagnostics);
+      ASSERT_EQ(1, wos.size());
+      const auto& new_wo = wos.front();
+      EXPECT_EQ("DisplayExtraWarnings", new_wo.getString(0).get());
+      EXPECT_EQ("DisplayUnusedSchedules", new_wo.getString(1).get());
+    }
+  }
+  {
+    // Non unique, and present already
+    WorkspaceObject wo = w.addObject(IdfObject(IddObjectType::Schedule_Constant)).get();
+    wo.setString(0, "AlwaysOn");
+    wo.setDouble(2, 1.0);
+
+    auto& newObject = energyPlusOutputRequests.emplace_back(IddObjectType::Schedule_Constant);
+    newObject.setString(0, "AlwaysOn");
+    newObject.setDouble(2, 1.0);
+
+    EXPECT_TRUE(w.getObjectByTypeAndName(IddObjectType::Schedule_Constant, "AlwaysOn"));
+    EXPECT_FALSE(openstudio::workflow::util::addEnergyPlusOutputRequest(w, newObject));
+    {
+      EXPECT_EQ(1, w.getObjectsByType(IddObjectType::Schedule_Constant).size());
+      EXPECT_TRUE(w.getObjectByTypeAndName(IddObjectType::Schedule_Constant, "AlwaysOn"));
+    }
+  }
+  {
+    // Non unique, and not present already
+    auto& newObject = energyPlusOutputRequests.emplace_back(IddObjectType::Schedule_Constant);
+    newObject.setString(0, "A New Schedule");
+    newObject.setDouble(2, 18.0);
+    EXPECT_EQ(1, w.getObjectsByType(IddObjectType::Schedule_Constant).size());
+    EXPECT_TRUE(w.getObjectByTypeAndName(IddObjectType::Schedule_Constant, "AlwaysOn"));
+    EXPECT_TRUE(openstudio::workflow::util::addEnergyPlusOutputRequest(w, newObject));
+    {
+      auto wos = w.getObjectsByType(IddObjectType::Schedule_Constant);
+      ASSERT_EQ(2, wos.size());
+      EXPECT_TRUE(w.getObjectByTypeAndName(IddObjectType::Schedule_Constant, "AlwaysOn"));
+      auto new_wo_ = w.getObjectByTypeAndName(IddObjectType::Schedule_Constant, "A New Schedule");
+      ASSERT_TRUE(new_wo_);
+      EXPECT_EQ("A New Schedule", new_wo_->getString(0).get());
+      EXPECT_TRUE(new_wo_->isEmpty(1));
+      EXPECT_EQ(18.0, new_wo_->getDouble(2).get());
+    }
+  }
+}


New test for addEnergyPlusOutputRequest, specifically I'm testing what happens with unique objects. We should NOT end up with more than 1

jmarrec · 2025-03-17T10:14:45Z

src/workflow/Util.cpp

+  const auto iddObject = idfObject.iddObject();
+  const bool is_unique = iddObject.properties().unique;
+  const auto iddObjectType = iddObject.type();
+  if (!is_unique) {


If not unique, indeed just check if there is the same object via dataFieldsEqual and add it if not.

jmarrec · 2025-03-17T10:14:57Z

src/workflow/Util.cpp

-  //  };
-
-  if (iddObjectType == IddObjectType::Output_Table_SummaryReports) {
+  } else if (iddObjectType == IddObjectType::Output_Table_SummaryReports) {


If it's summaryReports, merge.

jmarrec · 2025-03-17T10:16:04Z

src/workflow/Util.cpp

+  } else {
+    // It's a unique one, for which we didn't write a merge, so remove it first
+    auto existingUniqueObjects = workspace.getObjectsByType(iddObjectType);
+    if (!existingUniqueObjects.empty()) {
+      // Technically it should be size == 1
+      if (std::ranges::any_of(existingUniqueObjects, [&idfObject](const auto& wo) { return idfObject.dataFieldsEqual(wo); })) {
+        return false;
+      }
+      for (auto& wo : existingUniqueObjects) {
+        wo.remove();
+      }
+    }
+    workspace.addObject(idfObject);
+    return true;


If it's a unique one, check if there's the same exact object in there already and return false if so.

Otherwise, remove it first before adding the one (=replace).

We shouldn't end up with 2 "Output:SQLite" objects for eg, or E+ will throw

jmarrec · 2025-03-17T10:18:54Z

src/workflow/Util.cpp

  return added;
 }

 bool addEnergyPlusOutputRequest(Workspace& workspace, IdfObject& idfObject) {


I was super tempted to add a LOG (Warn or Info) in there, something along the lines of

if iddObjectType.valueName does not start with "Output" or "Meter", or "EnergyManagement" or "PythonPlugin", then issue a warning like "Requested to add an object of type XXX, ensure you aren't going to modify the energy usage of the model".

But I'm guessing @eringold and @shorowit wouldn't like that too much either, right?

I'm not against log messages. Though technically objects starting with 'EnergyManagement' or 'PythonPlugin' could modify energy use. Maybe better would be to check if object type names contain just 'Output' or 'Meter'.

This is true but PythonPlugin (and EnergyManagement is the same) were some of the objects that you specifically wanted in your OP, and they could have a valid application.

I decided to add the Warn after all, but not being restrictive and letting the request be processed anyways.

cf #5358 (review)

jmarrec · 2025-03-17T11:15:06Z

@wenyikuang can you look into mac failing please?

CPack Error: Problem copying the package: /Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2/_CPack_Packages/Darwin/IFW/OpenStudio-3.10.0-alpha+bf5726e49d-Darwin-x86_64.dmg to /Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2/OpenStudio-3.10.0-alpha+bf5726e49d-Darwin-x86_64.dmg
CPack Error: Error when generating package: OpenStudio

…eSummaryReports

…xisting one before you add another

I aimed to not be restrictive, and it'll still allow it. I also made the list of prefix easy to change even for someone who isn't familiar with C++

jmarrec · 2025-03-19T11:24:56Z

src/workflow/Util.cpp

+  // If not marked safe, we Warn, but we let you do it anyways
+  if (isEnergyPlusOutputRequestPotentiallyUnsafe(iddObjectType)) {
+    LOG_FREE(Warn, "openstudio.worklow.Util",
+             "Requested to add an EnergyPlus Output Request of type '"
+               << iddObjectType.valueName() << "' which is not marked as safe: make sure you aren't going to modify the energy usage of the model.");
+  }


I added a Warn message when not safe, but still we allow it anyways.

jmarrec · 2025-03-19T11:26:56Z

src/workflow/Util.hpp

+    // Schedule can be used onto a meter/variable, the EMS/PythonPlugin **could** be used for custom reporting
+    // Also adding some cost/utility related objects, the goal is to be not too restrictive here
+    static constexpr std::array<std::string_view, 11> safe_idd_prefixes{
+      "Output",     "Meter",    "PythonPlugin", "EnergyManagementSystem", "Schedule", "LifeCycleCost", "UtilityCost", "ComponentCost",
+      "Compliance", "Currency", "FuelFactors",
+    };
+
+    // If the IddObjectType.valueName doesn't start with one of the allowed prefixes (see `safe_idd_prefixes`), it returns true.
+    bool isEnergyPlusOutputRequestPotentiallyUnsafe(const IddObjectType& iddObjectType);


I aimed to strike the right balance here: the list is not restrictive on purpose

Remember that this is just resulting in a Warn message, it'll still process your request.

Also, energyplusOutputRequests is somewhat moot now that we have a modelOutputRequests method

Finally, I made the list of prefix really easy to change: just add to that std::array and you're good to go, which would allow anyone to propose a change even without knowing much C++

jmarrec · 2025-03-19T11:27:04Z

src/workflow/test/Util_GTest.cpp

+TEST_F(WorkflowFixture, Util_isEnergyPlusOutputRequestPotentiallyUnsafe) {
+
+  std::vector<IddObjectType> should_be_allowed{
+    IddObjectType::Output_Table_Annual,
+    IddObjectType::Meter_Custom,
+    IddObjectType::Schedule_Compact,
+    IddObjectType::Output_Diagnostics,
+    IddObjectType::Output_Variable,
+    IddObjectType::EnergyManagementSystem_OutputVariable,
+    IddObjectType::PythonPlugin_OutputVariable,
+    IddObjectType::LifeCycleCost_Parameters,
+    IddObjectType::UtilityCost_Tariff,
+    IddObjectType::ComponentCost_Adjustments,
+    IddObjectType::Compliance_Building,
+    IddObjectType::CurrencyType,
+  };
+  for (const auto& iddObjectType : should_be_allowed) {
+    EXPECT_FALSE(openstudio::workflow::util::isEnergyPlusOutputRequestPotentiallyUnsafe(iddObjectType)) << "Failed for " << iddObjectType;
+  }
+
+  EXPECT_TRUE(openstudio::workflow::util::isEnergyPlusOutputRequestPotentiallyUnsafe(IddObjectType::People));
+}


kbenne

This is great. The compromise by logging a warning when certain types are added seems reasonable to me.

kbenne · 2025-03-19T16:37:55Z

@wnykuang it looks like the non linux builds are still failing. Can you have a look? I don't think it is related to this PR.

wenyikuang · 2025-03-19T16:40:38Z

Let me clean it up (again) and run it again. I think the "real" issue (due to NREL's network) should be gone. The issues should be gone after that.

wenyikuang · 2025-03-19T17:33:04Z

well it still broken in osx-incremental, let me dive deep.

wenyikuang · 2025-03-19T17:51:04Z

OK,

Actually looks like this alarm is reasonable:

[2906/9345] Building CXX object src/workflow/CMakeFiles/openstudio_workflow.dir/OSWorkflow.cpp.o

FAILED: src/workflow/CMakeFiles/openstudio_workflow.dir/OSWorkflow.cpp.o 

/usr/bin/clang++ -DBOOST_NO_CXX98_FUNCTION_BASE -DBOOST_STACKTRACE_ADDR2LINE_LOCATION=\"/usr/bin/addr2line\" -DBOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED -DBOOST_STACKTRACE_USE_ADDR2LINE -DBOOST_STACKTRACE_USE_BACKTRACE -DBOOST_STACKTRACE_USE_NOOP -DHAVE_UNISTD_H -DLIBXML_STATIC -DSHARED_OS_LIBS -D_HAS_AUTO_PTR_ETC=0 -D_NO_ASYNCRTIMP -D_NO_PPLXIMP -I/Users/jenkins/git/OpenStudioIncremental/Openstudio/src -I/Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2/src -I/Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2 -isystem /Users/jenkins/.pyenv/versions/3.12.2/include/python3.12 -isystem /Users/jenkins/.conan2/p/b/boosteebcdebc6a937/p/include -isystem /Users/jenkins/.conan2/p/b/libic9427c8c0c4edd/p/include -isystem /Users/jenkins/.conan2/p/b/bzip2f2fbb99ee84b2/p/include -isystem /Users/jenkins/.conan2/p/b/zlibf283119b64e9b/p/include -isystem /Users/jenkins/.conan2/p/b/libbaab4178e601ebe/p/include -isystem /Users/jenkins/.conan2/p/b/jsonc8a0968cd67dbb/p/include -isystem /Users/jenkins/.conan2/p/b/cppreb77e061427b00/p/include -isystem /Users/jenkins/.conan2/p/b/opens41941aff6e832/p/include -isystem /Users/jenkins/.conan2/p/webso7a47c7731495b/p/include -isystem /Users/jenkins/.conan2/p/b/pugixe450de9ab6ce0/p/include -isystem /Users/jenkins/.conan2/p/b/libxma880b5f6c35f3/p/include -isystem /Users/jenkins/.conan2/p/b/libxma880b5f6c35f3/p/include/libxml2 -isystem /Users/jenkins/.conan2/p/b/fmtc027927559c40/p/include -m64 -stdlib=libc++ -Wall -fPIC -fno-strict-aliasing -Winvalid-pch -Wnon-virtual-dtor -Wno-narrowing -Wno-narrowing -Werror -Wno-overloaded-virtual -ftemplate-depth=1024 -stdlib=libc++ -O3 -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -mmacosx-version-min=10.15 -std=c++20 -MD -MT src/workflow/CMakeFiles/openstudio_workflow.dir/OSWorkflow.cpp.o -MF src/workflow/CMakeFiles/openstudio_workflow.dir/OSWorkflow.cpp.o.d -o src/workflow/CMakeFiles/openstudio_workflow.dir/OSWorkflow.cpp.o -c /Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/OSWorkflow.cpp

In file included from /Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/OSWorkflow.cpp:7:

/Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/Util.hpp:22:1: error: class 'IddObjectType' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Werror,-Wmismatched-tags]

class IddObjectType;

^

/Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/../measure/../model/../utilities/idd/IddEnums.hpp:88:22: note: previous use is here

struct UTILITIES_API IddObjectType : public ::EnumBase<IddObjectType>

                     ^

/Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/Util.hpp:22:1: note: did you mean struct here?

class IddObjectType;

^~~~~

struct

1 error generated.

[2907/9345] Building CXX object src/workflow/CMakeFiles/openstudio_workflow.dir/RunInitialization.cpp.o

FAILED: src/workflow/CMakeFiles/openstudio_workflow.dir/RunInitialization.cpp.o 

/usr/bin/clang++ -DBOOST_NO_CXX98_FUNCTION_BASE -DBOOST_STACKTRACE_ADDR2LINE_LOCATION=\"/usr/bin/addr2line\" -DBOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED -DBOOST_STACKTRACE_USE_ADDR2LINE -DBOOST_STACKTRACE_USE_BACKTRACE -DBOOST_STACKTRACE_USE_NOOP -DHAVE_UNISTD_H -DLIBXML_STATIC -DSHARED_OS_LIBS -D_HAS_AUTO_PTR_ETC=0 -D_NO_ASYNCRTIMP -D_NO_PPLXIMP -I/Users/jenkins/git/OpenStudioIncremental/Openstudio/src -I/Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2/src -I/Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2 -isystem /Users/jenkins/.pyenv/versions/3.12.2/include/python3.12 -isystem /Users/jenkins/.conan2/p/b/boosteebcdebc6a937/p/include -isystem /Users/jenkins/.conan2/p/b/libic9427c8c0c4edd/p/include -isystem /Users/jenkins/.conan2/p/b/bzip2f2fbb99ee84b2/p/include -isystem /Users/jenkins/.conan2/p/b/zlibf283119b64e9b/p/include -isystem /Users/jenkins/.conan2/p/b/libbaab4178e601ebe/p/include -isystem /Users/jenkins/.conan2/p/b/jsonc8a0968cd67dbb/p/include -isystem /Users/jenkins/.conan2/p/b/cppreb77e061427b00/p/include -isystem /Users/jenkins/.conan2/p/b/opens41941aff6e832/p/include -isystem /Users/jenkins/.conan2/p/webso7a47c7731495b/p/include -isystem /Users/jenkins/.conan2/p/b/pugixe450de9ab6ce0/p/include -isystem /Users/jenkins/.conan2/p/b/libxma880b5f6c35f3/p/include -isystem /Users/jenkins/.conan2/p/b/libxma880b5f6c35f3/p/include/libxml2 -isystem /Users/jenkins/.conan2/p/b/fmtc027927559c40/p/include -m64 -stdlib=libc++ -Wall -fPIC -fno-strict-aliasing -Winvalid-pch -Wnon-virtual-dtor -Wno-narrowing -Wno-narrowing -Werror -Wno-overloaded-virtual -ftemplate-depth=1024 -stdlib=libc++ -O3 -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -mmacosx-version-min=10.15 -std=c++20 -MD -MT src/workflow/CMakeFiles/openstudio_workflow.dir/RunInitialization.cpp.o -MF src/workflow/CMakeFiles/openstudio_workflow.dir/RunInitialization.cpp.o.d -o src/workflow/CMakeFiles/openstudio_workflow.dir/RunInitialization.cpp.o -c /Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/RunInitialization.cpp

In file included from /Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/RunInitialization.cpp:8:

/Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/Util.hpp:22:1: error: class 'IddObjectType' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Werror,-Wmismatched-tags]

class IddObjectType;

^

/Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/../measure/../model/../utilities/idd/IddEnums.hpp:88:22: note: previous use is here

struct UTILITIES_API IddObjectType : public ::EnumBase<IddObjectType>

                     ^

/Users/jenkins/git/OpenStudioIncremental/Openstudio/src/workflow/Util.hpp:22:1: note: did you mean struct here?

class IddObjectType;

Looks like we do have mismatch tags.

in https://github.com/NREL/OpenStudio/blob/unlimit_energyplusoutputrequests/src/utilities/idd/IddEnums.hpp#L32
struct UTILITIES_API IddFileType : public ::EnumBase
and we have
class IddObjectType;
in https://github.com/NREL/OpenStudio/blob/unlimit_energyplusoutputrequests/src/workflow/Util.hpp#L22

I think that's something we may need update?
btw, could we pick the new conan.lock from the develop branch for this branch too? thanks.

src/workflow/Util.hpp

jmarrec · 2025-03-19T22:22:02Z

btw, could we pick the new conan.lock from the develop branch for this branch too? thanks

This branch was rebased onto develop today.

I just fixed the class -> struct (hopefully did it right, on my phone)

ci-commercialbuildings · 2025-03-19T22:23:55Z

CI Results for 5e9f22a:

Ubuntu 24.04:
- OpenStudio-3.10.0-alpha+5e9f22adda-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.10.0-alpha+5e9f22adda-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
Darwin:
Windows:
- OpenStudio-3.10.0-alpha+5e9f22adda-Windows.exe
- OpenStudio-3.10.0-alpha+5e9f22adda-Windows.tar.gz
Code Coverage:

jmarrec · 2025-03-20T07:58:02Z

OSX build still failing at CPack step.

CPack: - Generate package

CPack Error: Problem copying the package: /Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2/_CPack_Packages/Darwin/IFW/OpenStudio-3.10.0-alpha+5e9f22adda-Darwin-x86_64.dmg to /Users/jenkins/git/OpenStudioIncremental/OS-build-release-v2/OpenStudio-3.10.0-alpha+5e9f22adda-Darwin-x86_64.dmg

CPack Error: Error when generating package: OpenStudio

Wipe the build/_CPack_Packages and make sure there is enough space on the device? Maybe upgrade QtIFW too in the process.

Also, it'd be a good idea to install pytest...

CMake Warning (dev) at python/SetupPython.cmake:22 (message):
  Pytest isn't installed on your system python, so some tests won't be run.
  Run `pip install pytest`

jmarrec · 2025-03-20T07:58:58Z

Windows is happy now. Mac has been broken for ever, so just ignoring it. Merging.

eringold added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Mar 3, 2025

eringold requested a review from kbenne March 3, 2025 17:11

jmarrec force-pushed the unlimit_energyplusoutputrequests branch from ca84a21 to 9a162a8 Compare March 17, 2025 10:10

jmarrec reviewed Mar 17, 2025

View reviewed changes

jmarrec requested a review from joseph-robertson March 17, 2025 23:20

jmarrec added the component - Measures label Mar 18, 2025

eringold and others added 4 commits March 19, 2025 11:41

remove allowedObjects

6f852b3

Write a unit test for addEnergyplusOutputRequests and mergeOutputTabl…

5c65ad0

…eSummaryReports

Fix existing logic error in mergeOutputTableSummaryReports

1c65ed0

Fixup changes in addEnergyPlusOutputRequests: if unique, remove the e…

958020e

…xisting one before you add another

jmarrec force-pushed the unlimit_energyplusoutputrequests branch from bf5726e to 958020e Compare March 19, 2025 10:41

Add a Warn message when prefix isn't marked safe

1ea6309

I aimed to not be restrictive, and it'll still allow it. I also made the list of prefix easy to change even for someone who isn't familiar with C++

jmarrec reviewed Mar 19, 2025

View reviewed changes

kbenne approved these changes Mar 19, 2025

View reviewed changes

jmarrec reviewed Mar 19, 2025

View reviewed changes

src/workflow/Util.hpp Outdated Show resolved Hide resolved

Update src/workflow/Util.hpp

5e9f22a

jmarrec merged commit 85c3500 into develop Mar 20, 2025
5 of 6 checks passed

jmarrec deleted the unlimit_energyplusoutputrequests branch March 20, 2025 07:59

Remove allowedObjects check from energyPlusOutputRequests #5358

Remove allowedObjects check from energyPlusOutputRequests #5358

Uh oh!

Conversation

eringold commented Mar 3, 2025

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!

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!

jmarrec commented Mar 17, 2025

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!

kbenne left a comment

Choose a reason for hiding this comment

Uh oh!

kbenne commented Mar 19, 2025

Uh oh!

wenyikuang commented Mar 19, 2025

Uh oh!

wenyikuang commented Mar 19, 2025

Uh oh!

wenyikuang commented Mar 19, 2025

Uh oh!

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!

jmarrec commented Mar 20, 2025

Uh oh!

jmarrec commented Mar 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Remove allowedObjects check from `energyPlusOutputRequests` #5358

Remove allowedObjects check from `energyPlusOutputRequests` #5358

jmarrec Mar 19, 2025 •

edited

Loading

ci-commercialbuildings commented Mar 19, 2025 •

edited

Loading