Skip to content

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Jan 14, 2025

Pull request overview

Pull Request Author

  • Model API Changes / Additions: None
  • Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate): Corrected
  • Model API methods are tested (in src/model/test): No Change
  • EnergyPlus ForwardTranslator Tests (in src/energyplus/Test): Added a new unit test file
  • If a new object or method, added a test in NREL/OpenStudio-resources: N/A, but could cause regressions
    • Could cause regression in water_economizer.osm and coolingtowers.osm
  • 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 jmarrec added severity - Normal Bug component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Jan 14, 2025
@jmarrec jmarrec self-assigned this Jan 14, 2025
Test/Construction_GTest.cpp
Test/ConstructionWithInternalSource_GTest.cpp
Test/ControllerOutdoorAir_GTest.cpp
Test/CoolingTowerSingleSpeed_GTest.cpp
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no FT test for the object, so I started by adding one

Comment on lines +62 to +64
EXPECT_TRUE(ct.setDesignApproachTemperature(3.89));
// ct.autosizeDesignRangeTemperature();
EXPECT_TRUE(ct.setDesignRangeTemperature(5.55));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Comment on lines +114 to +115
EXPECT_EQ(3.89, idfObject.getDouble(CoolingTower_SingleSpeedFields::DesignApproachTemperature).get());
EXPECT_EQ(5.55, idfObject.getDouble(CoolingTower_SingleSpeedFields::DesignRangeTemperature).get());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ ninja && Products/openstudio_energyplus_tests -- --gtest_filter=*ForwardTranslator_CoolingTowerSingleSpeed*
[100%][2/2] Linking CXX executable Products/openstudio_energyplus_tests
Running main() from gmock_main.cc
Note: Google Test filter = *ForwardTranslator_CoolingTowerSingleSpeed*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from EnergyPlusFixture
[ RUN      ] EnergyPlusFixture.ForwardTranslator_CoolingTowerSingleSpeed
/media/DataExt4/Software/Others/OpenStudio3/src/energyplus/Test/CoolingTowerSingleSpeed_GTest.cpp:115: Failure
Expected equality of these values:
  5.55
  idfObject.getDouble(CoolingTower_SingleSpeedFields::DesignRangeTemperature).get()
    Which is: 3.89

[  FAILED  ] EnergyPlusFixture.ForwardTranslator_CoolingTowerSingleSpeed (217 ms)
[----------] 1 test from EnergyPlusFixture (217 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (218 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] EnergyPlusFixture.ForwardTranslator_CoolingTowerSingleSpeed

if (modelObject.isDesignRangeTemperatureAutosized()) {
idfObject.setString(openstudio::CoolingTower_SingleSpeedFields::DesignRangeTemperature, "Autosize");
} else if ((d = modelObject.designApproachTemperature())) {
} else if ((d = modelObject.designRangeTemperature())) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a copy paste error

@eringold
Copy link
Collaborator

also fixes (duplicate issue): #5338

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jan 14, 2025

Thanks @eringold I just took the issue list from the top down and didn't get there!

Copy link
Collaborator

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a copy-paste mistake of modelObject.designApproachTemperature()? New FT test 👍

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jan 15, 2025

Yeah, defiitely a copy paste issue. Strange that it took us (and I mean the entire community) 8 years to notice it though.

@jmarrec jmarrec merged commit 96731d1 into develop Jan 15, 2025
4 of 6 checks passed
@jmarrec jmarrec deleted the 5343_CT branch January 15, 2025 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. severity - Normal Bug

Projects

None yet

5 participants