Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT #4813

jmarrec · 2023-03-07T17:20:04Z

Pull request overview

Fixes E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT #4802

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:
- Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT OpenStudio-resources#192
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

…ing to be wrapped just yet)

…dio.idd Just adding the Handle

…oltaicThermal:BIPVT

``` $os_build_rel/Products/openstudio GenerateClass.rb -c "SolarCollectorPerformancePhotovoltaicThermalBIPVT" -b "ModelObject" -i "OS:SolarCollectorPerformance:PhotovoltaicThermal:BIPVT" -s model -o /Users/julien/Software/Others/OpenStudio/src/model/ -p -f -r ```

…solarCollectorPerformance: it now retuerns a ModelObject and not a SolarCollectorPerformancePhotovoltaicThermalSimple

… change

Companion test for NREL/OpenStudio#4813

joseph-robertson · 2023-03-07T21:13:32Z

resources/energyplus/ProposedEnergy+.idd

@@ -5118,6 +5118,36 @@ MaterialProperty:VariableThermalConductivity,
       \note for Temperature-Thermal Conductivity function corresponding to temperature 1
       \units W/m-K

+MaterialProperty:VariableAbsorptance,


Sneaking this in (unrelated to this PR), right?

Yes. I realized between this PR and the ones you added so far, it was going to be the only missing one. See commit message of 90005f1

…atPlatePhotovoltaicThermal isn't RT'ed, neither is SurfacePropertyOtherSideConditionsModel)

…e RT for this + BIPVT

…, remove defaulted one)

…ertyOtherSideConditionsModel to avoid creating a default one

jmarrec · 2023-03-08T06:38:30Z

developer/doc/ReleaseNotes/OpenStudio_Release_Notes_3_6_0_TBD.md

+* [#4813](https://github.com/NREL/OpenStudio/pull/4813) - Wrap `SolarCollectorPerformance:PhotovoltaicThermal:BIPVT`
+    * `SolarCollectorFlatPlatePhotovoltaicThermal` has API-breaking changes in the `solarCollectorPerformance` getter due to the addition of this new object: it used to return a `SolarCollectorPerformancePhotovoltaicThermalSimple` (the only performance object at the time), now it's a `ModelObject`.


Document API break in SolarCollectorFlatPlatePhotovoltaicThermal

jmarrec · 2023-03-08T06:39:17Z

resources/energyplus/ProposedEnergy+.idd

+  A3 , \field Control Signal
+       \type choice
+       \key SurfaceTemperature
+       \key SurfaceReceivedSolarRadiation
+       \key SpaceHeatingCoolingMode
+       \key Scheduled
+       \default SurfaceTemperature


MaterialProperty:VariableAbsorptance: I removed the \required-field in E+ idd since it already has a \default

jmarrec · 2023-03-08T06:41:22Z

src/model/SolarCollectorFlatPlatePhotovoltaicThermal.hpp

@@ -70,7 +73,7 @@ namespace model {
    /** @name Getters */
    //@{

-    SolarCollectorPerformancePhotovoltaicThermalSimple solarCollectorPerformance() const;
+    ModelObject solarCollectorPerformance() const;


API break in SolarCollectorFlatPlatePhotovoltaicThermal. It is unavoidable, now that we have two potential Performance objects.

I am considering adding a base (shallow) class SolarCollectorPerformancePhotovoltaicThermal

jmarrec · 2023-03-08T07:06:08Z

src/model/SolarCollectorFlatPlatePhotovoltaicThermal.hpp

+    /// Deletes the current performance and clones the performance passed in
+    bool setSolarCollectorPerformance(const SolarCollectorPerformancePhotovoltaicThermalBIPVT& performance);
+    bool setSolarCollectorPerformance(const SolarCollectorPerformancePhotovoltaicThermalSimple& performance);


I don't really like changing the parameter type to the overly generic "ModelObject" one here. It would work fine if you passed any ModelObject and just return false but that's silent-ish. Whether constraining the Type would create a type error message with the prototype(s) accepted.

That is the reason I am considering a base class SolarCollectorPerformancePhotovoltaicThermalhere. But maybe it's too much complexity.
So so far I've used two specific overloads for the setter. The only annoyance is that you need to cast if you retrieve the performance from one object to set to another.

-pvt2.setPerformance(pvt.performance()) +pvt2.setperformance(pvt.performance().to_SolarCollectorPerformancePhotovoltaicThermalSimple)

Seems consistent with how we've been breaking API in the recent past...

jmarrec · 2023-03-08T07:06:57Z

src/model/Surface.hpp

@@ -195,7 +195,7 @@ namespace model {
    boost::optional<SurfacePropertyOtherSideCoefficients> surfacePropertyOtherSideCoefficients() const;

    /** Sets the SurfacePropertyOtherSideCoefficients. */
-    bool setSurfacePropertyOtherSideCoefficients(SurfacePropertyOtherSideCoefficients& otherSideCoefficients);
+    bool setSurfacePropertyOtherSideCoefficients(const SurfacePropertyOtherSideCoefficients& otherSideCoefficients);


This isn't exactly related to this PR but I noticed the const-correctness issue while creating a GTest (It was forcing me to create a temporary before I could use these)

setSurfacePropertyOtherSideCoefficients / setSurfacePropertyOtherSideConditionsModel in both Surface and SubSurface need to have a const.

jmarrec · 2023-03-08T07:50:17Z

src/model/test/SolarCollectorFlatPlatePhotovoltaicThermal_GTest.cpp

+  {
+    Model m;
+    SolarCollectorFlatPlatePhotovoltaicThermal testObject(m);
+    auto performance = testObject.solarCollectorPerformance();
+    EXPECT_TRUE(performance.optionalCast<SolarCollectorPerformancePhotovoltaicThermalSimple>());
+  }
+  {
+    Model m;
+    SolarCollectorPerformancePhotovoltaicThermalSimple performance(m);
+    SolarCollectorFlatPlatePhotovoltaicThermal testObject(performance);
+    EXPECT_TRUE(performance.optionalCast<SolarCollectorPerformancePhotovoltaicThermalSimple>());
+    EXPECT_EQ(testObject.solarCollectorPerformance(), performance);
+  }
+  {
+    Model m;
+    SolarCollectorPerformancePhotovoltaicThermalBIPVT performance(m);
+    SolarCollectorFlatPlatePhotovoltaicThermal testObject(performance);
+    EXPECT_TRUE(performance.optionalCast<SolarCollectorPerformancePhotovoltaicThermalBIPVT>());
+    EXPECT_EQ(testObject.solarCollectorPerformance(), performance);
+  }


test the new explicit ctors for the Solar Collector PVT

jmarrec · 2023-03-08T07:51:24Z

src/energyplus/Test/SolarCollectorPerformancePhotovoltaicThermalBIPVT_GTest.cpp

+ * that means correct node names in the ChillerElectricASHRAE205 but also
+ * on the Branches
+ */
+TEST_F(EnergyPlusFixture, ForwardTranslator_SolarCollectorPerformancePhotovoltaicThermalBIPVT) {


Full FT test

jmarrec · 2023-03-08T09:12:59Z

src/energyplus/ReverseTranslator/ReverseTranslateSurfacePropertyOtherSideConditionsModel.cpp

+
+namespace energyplus {
+
+  boost::optional<ModelObject> ReverseTranslator::translateSurfacePropertyOtherSideConditionsModel(const WorkspaceObject& workspaceObject) {


New RT for SurfacePropertyOtherSideConditionsModel, I needed this so the BIPVT one would not be too useless.

jmarrec · 2023-03-08T09:13:33Z

...plus/ReverseTranslator/ReverseTranslateSolarCollectorPerformancePhotovoltaicThermalBIPVT.cpp

+    // Boundary Conditions Model Name: Required Object
+    if (boost::optional<WorkspaceObject> wo_ =
+          workspaceObject.getTarget(SolarCollectorPerformance_PhotovoltaicThermal_BIPVTFields::BoundaryConditionsModelName)) {
+      if (boost::optional<ModelObject> mo_ = translateAndMapWorkspaceObject(wo_.get())) {
+        // TODO: check return types
+        if (boost::optional<SurfacePropertyOtherSideConditionsModel> boundaryConditionsModel_ =
+              mo_->optionalCast<SurfacePropertyOtherSideConditionsModel>()) {
+          // The Ctor creates a BoundaryConditionsModel for us, so we can remove it now that we found the right one
+          auto defaultOSCM = modelObject.boundaryConditionsModel();
+          modelObject.setBoundaryConditionsModel(boundaryConditionsModel_.get());
+          defaultOSCM.remove();
+        } else {
+          LOG(Warn, workspaceObject.briefDescription() << " has a wrong type for 'Boundary Conditions Model Name'");
+        }
+      } else {
+        LOG(Error, "For " << workspaceObject.briefDescription()
+                          << ", cannot reverse translate required object 'Boundary Conditions Model Name'. Using defaulted one.");
+      }
+      LOG(Warn,
+          "For " << workspaceObject.briefDescription() << ", cannot find required object 'Boundary Conditions Model Name'. Using defaulted one.");
+    }


BIPVT: just warn if the OSCM can't be found (and keep using the defaulted one).

jmarrec · 2023-03-08T09:14:32Z

src/energyplus/ReverseTranslator.cpp

+      case openstudio::IddObjectType::SolarCollectorPerformance_PhotovoltaicThermal_BIPVT: {
+        modelObject = translateSolarCollectorPerformancePhotovoltaicThermalBIPVT(workspaceObject);
+        break;
+      }


I'm adding the RT for the future mostly, since the SolarCollectorPVT isn't RT'ed (since PlantLoop isn't).

But this can still be useful if you want to grab the Perf data from an IDF to assign to a model-time created SolarCollectorPVT.

jmarrec

Final walkthrough comments. This is ready for review. Not sure who wants to review: @joseph-robertson @tijcolem

jmarrec · 2023-03-08T09:18:33Z

src/model/SolarCollectorPerformancePhotovoltaicThermalBIPVT.hpp

+  }  // namespace detail
+
+  /** SolarCollectorPerformancePhotovoltaicThermalBIPVT is a ModelObject that wraps the OpenStudio IDD object 'OS:SolarCollectorPerformance:PhotovoltaicThermal:BIPVT'. */
+  class MODEL_API SolarCollectorPerformancePhotovoltaicThermalBIPVT : public ModelObject


Note: I made this a ModelObject because the SolarCollectorPerformancePhotovoltaicThermalSimple already was. I kinda find it weird that these aren't ResourceObjects, and that the SolarCollectorPVT just clones whatever Perf object you pass, but that's the way it is, I didn't want to break API.

OpenStudio/src/model/SolarCollectorFlatPlatePhotovoltaicThermal.cpp

Lines 181 to 198 in 2c46784

bool SolarCollectorFlatPlatePhotovoltaicThermal_Impl::setSolarCollectorPerformance(

const SolarCollectorPerformancePhotovoltaicThermalSimple& performance) {

bool result(false);

SolarCollectorPerformancePhotovoltaicThermalSimple current = this->solarCollectorPerformance();

if (current.handle() == performance.handle()) {

return true; // no-op

}

ModelObject clone = performance.clone(this->model());

result = setSolarCollectorPerformanceNoClone(clone.cast<SolarCollectorPerformancePhotovoltaicThermalSimple>());

if (result) {

current.remove();

} else {

result = setSolarCollectorPerformanceNoClone(current);

OS_ASSERT(result);

return false;

}

return result;

}

ci-commercialbuildings · 2023-03-08T15:37:41Z

CI Results for 8361971:

Ubuntu 18.04:
- OpenStudio-3.6.0-alpha+83619716af-Ubuntu-18.04.deb
- OpenStudio-3.6.0-alpha+83619716af-Ubuntu-18.04.tar.gz
Ubuntu 20.04:
Darwin:
- OpenStudio-3.6.0-alpha+83619716af-Darwin-x86_64.dmg
- OpenStudio-3.6.0-alpha+83619716af-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.6.0-alpha+146561ef6b-Windows.exe
- OpenStudio-3.6.0-alpha+146561ef6b-Windows.tar.gz
Code Coverage:

joseph-robertson

Looks good to me. Just a few clarifying questions.

joseph-robertson · 2023-03-14T00:05:00Z

developer/doc/ReleaseNotes/OpenStudio_Release_Notes_3_6_0_TBD.md

+* [#4813](https://github.com/NREL/OpenStudio/pull/4813) - Wrap `SolarCollectorPerformance:PhotovoltaicThermal:BIPVT`
+    * `SolarCollectorFlatPlatePhotovoltaicThermal` has API-breaking changes in the `solarCollectorPerformance` getter due to the addition of this new object: it used to return a `SolarCollectorPerformancePhotovoltaicThermalSimple` (the only performance object at the time), now it's a `ModelObject`.


joseph-robertson · 2023-03-14T00:05:33Z

resources/model/OpenStudio.idd

+OS:SolarCollectorPerformance:PhotovoltaicThermal:BIPVT,
+       \memo Thermal performance parameters for Building-Integrated Photovoltaic-Thermal (BIPVT) solar collector.
+       \min-fields 17
+  A1,  \field Handle
+       \type handle
+       \required-field
+  A2 , \field Name
+       \required-field
+       \type alpha
+       \reference FlatPlatePVTParameters
+  A3,  \field Boundary Conditions Model Name
+       \required-field
+       \type object-list
+       \object-list OSCMNames
+       \note Enter the name of a SurfaceProperty:OtherSideConditionsModel object
+  A4 , \field Availability Schedule Name
+       \note Availability schedule name for this collector. Schedule value > 0 means it is available.
+       \note If this field is blank, the collector is always available.
+       \type object-list
+       \object-list ScheduleNames
+       \required-field
+  N1 , \field Effective Plenum Gap Thickness Behind PV Modules
+       \required-field
+       \type real
+       \units m
+       \minimum> 0.0
+  N2 , \field PV Cell Normal Transmittance-Absorptance Product
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N3 , \field Backing Material Normal Transmittance-Absorptance Product
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N4 , \field Cladding Normal Transmittance-Absorptance Product
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N5 , \field Fraction of Collector Gross Area Covered by PV Module
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N6 , \field Fraction of PV Cell Area to PV Module Area
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N7 , \field PV Module Top Thermal Resistance
+       \type real
+       \units m2-K/W
+       \required-field
+       \minimum> 0.0
+  N8 , \field PV Module Bottom Thermal Resistance
+       \type real
+       \units m2-K/W
+       \required-field
+       \minimum> 0.0
+  N9  ,\field PV Module Front Longwave Emissivity
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N10 ,\field PV Module Back Longwave Emissivity
+       \type real
+       \required-field
+       \minimum> 0.0
+       \maximum< 1.00
+  N11 ,\field Glass Thickness
+       \type real
+       \required-field
+       \units m
+       \minimum> 0.0
+       \maximum< 0.01
+  N12 ,\field Glass Refraction Index
+       \type real
+       \required-field
+       \minimum> 1.0
+       \maximum< 10.00
+  N13 ;\field Glass Extinction Coefficient
+       \type real
+       \required-field
+       \units 1/m
+       \minimum> 0.0
+       \maximum< 100.00


joseph-robertson · 2023-03-14T00:05:58Z

src/energyplus/CMakeLists.txt

@@ -394,12 +395,12 @@ set(${target_name}_src
  ForwardTranslator/ForwardTranslateSurfacePropertyConvectionCoefficients.cpp
  ForwardTranslator/ForwardTranslateSurfacePropertyConvectionCoefficientsMultipleSurface.cpp
  ForwardTranslator/ForwardTranslateSurfacePropertyExposedFoundationPerimeter.cpp
+  ForwardTranslator/ForwardTranslateSurfacePropertyGroundSurfaces.cpp
+  ForwardTranslator/ForwardTranslateSurfacePropertyIncidentSolarMultiplier.cpp


Alphabetizing.

joseph-robertson · 2023-03-14T00:06:41Z

src/energyplus/CMakeLists.txt

  ReverseTranslator/ReverseTranslateSurfacePropertyGroundSurfaces.cpp
  ReverseTranslator/ReverseTranslateSurfacePropertyIncidentSolarMultiplier.cpp
+  ReverseTranslator/ReverseTranslateSurfacePropertyLocalEnvironment.cpp
+  ReverseTranslator/ReverseTranslateSurfacePropertyOtherSideConditionsModel.cpp


Alphabetizing, and new RT for existing object.

joseph-robertson · 2023-03-14T00:08:52Z

src/energyplus/ReverseTranslator/ReverseTranslateSurfacePropertyOtherSideConditionsModel.cpp

+
+namespace energyplus {
+
+  boost::optional<ModelObject> ReverseTranslator::translateSurfacePropertyOtherSideConditionsModel(const WorkspaceObject& workspaceObject) {


joseph-robertson · 2023-03-14T00:13:42Z

src/model/SolarCollectorFlatPlatePhotovoltaicThermal.hpp

@@ -70,7 +73,7 @@ namespace model {
    /** @name Getters */
    //@{

-    SolarCollectorPerformancePhotovoltaicThermalSimple solarCollectorPerformance() const;
+    ModelObject solarCollectorPerformance() const;


joseph-robertson · 2023-03-14T00:14:26Z

src/model/SolarCollectorFlatPlatePhotovoltaicThermal.hpp

+    /// Deletes the current performance and clones the performance passed in
+    bool setSolarCollectorPerformance(const SolarCollectorPerformancePhotovoltaicThermalBIPVT& performance);
+    bool setSolarCollectorPerformance(const SolarCollectorPerformancePhotovoltaicThermalSimple& performance);


Seems consistent with how we've been breaking API in the recent past...

joseph-robertson · 2023-03-14T00:17:55Z

src/model/SolarCollectorFlatPlatePhotovoltaicThermal.hpp


-    /// Deletes the current parameters and constructs a new default set of parameters
+    /// Deletes the current performance and constructs a new default set of performance


Maybe I missed it, but where does it actually construct a new default set of performance?

I see it now.

joseph-robertson · 2023-03-14T00:19:59Z

src/model/test/SolarCollectorFlatPlatePhotovoltaicThermal_GTest.cpp

+  {
+    Model m;
+    SolarCollectorFlatPlatePhotovoltaicThermal testObject(m);
+    auto performance = testObject.solarCollectorPerformance();
+    EXPECT_TRUE(performance.optionalCast<SolarCollectorPerformancePhotovoltaicThermalSimple>());
+  }
+  {
+    Model m;
+    SolarCollectorPerformancePhotovoltaicThermalSimple performance(m);
+    SolarCollectorFlatPlatePhotovoltaicThermal testObject(performance);
+    EXPECT_TRUE(performance.optionalCast<SolarCollectorPerformancePhotovoltaicThermalSimple>());
+    EXPECT_EQ(testObject.solarCollectorPerformance(), performance);
+  }
+  {
+    Model m;
+    SolarCollectorPerformancePhotovoltaicThermalBIPVT performance(m);
+    SolarCollectorFlatPlatePhotovoltaicThermal testObject(performance);
+    EXPECT_TRUE(performance.optionalCast<SolarCollectorPerformancePhotovoltaicThermalBIPVT>());
+    EXPECT_EQ(testObject.solarCollectorPerformance(), performance);
+  }


joseph-robertson · 2023-03-14T00:22:56Z

src/model/test/SolarCollectorPerformancePhotovoltaicThermalBIPVT_GTest.cpp

+  // Bad Value
+  EXPECT_FALSE(solarCollectorPerformancePhotovoltaicThermalBIPVT.setGlassExtinctionCoefficient(-10.0));
+  EXPECT_EQ(94.444, solarCollectorPerformancePhotovoltaicThermalBIPVT.glassExtinctionCoefficient());
+}


We check for clone, remove, etc. in parent Thermal test?

jmarrec added 15 commits March 7, 2023 18:18

Add to E+ idd (+ MaterialProperty:VariableAbsorptance which is not go…

90005f1

…ing to be wrapped just yet)

Naive add SolarCollectorPerformance:PhotovoltaicThermal:BIPVT OpenStu…

ff36e1a

…dio.idd Just adding the Handle

Make everything required-field in OS:SolarCollectorPerformance:Photov…

cb61aa4

…oltaicThermal:BIPVT

Add min-fields

8b1eaec

Register model files

44f5a9e

clang format the stubs

69d3822

Adjust model files + harcoded IDD defaults into ctor

237fe11

New ctors + API break in SolarCollectorFlatPlatePhotovoltaicThermal::…

178d932

…solarCollectorPerformance: it now retuerns a ModelObject and not a SolarCollectorPerformancePhotovoltaicThermalSimple

Make SolarCollectorPerformancePhotovoltaicThermalSimple ctor public

9d6d532

Caught a const-correctness issue in Surface/SubSurface

cd41fbc

Adjust SolarCollectorFlatPlatePhotovoltaicThermal Gtest following API…

3353ee6

… change

Model test register + adjustment for BIPVT

a09684f

Implement FT for BIPVT

a1555b7

Full FT test for BIPVT

7e250a1

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. labels Mar 7, 2023

jmarrec self-assigned this Mar 7, 2023

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

E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT

f09eb32

Companion test for NREL/OpenStudio#4813

jmarrec mentioned this pull request Mar 7, 2023

Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT NREL/OpenStudio-resources#192

Merged

26 tasks

jmarrec added 2 commits March 7, 2023 18:48

Adjust PR number in release notes [ci skip]

913ed17

Fix ctest due to E+ idd having both a required-field + default

ea6e218

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

E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT

5b75a22

Companion test for NREL/OpenStudio#4813

joseph-robertson mentioned this pull request Mar 7, 2023

Update to EnergyPlus 23.1.0 #4789

Closed

5 tasks

joseph-robertson reviewed Mar 7, 2023

View reviewed changes

jmarrec added 3 commits March 8, 2023 09:00

Setup the ReverseTranslator, but disable it for now (SolarCollectorFl…

2b95705

…atPlatePhotovoltaicThermal isn't RT'ed, neither is SurfacePropertyOtherSideConditionsModel)

Setup a RT for SurfacePropertyOtherSideConditionsModel, and enable th…

10cf12a

…e RT for this + BIPVT

Downgrade not finding the OSCM to a Warn, use defaulted one (if found…

7d1fee1

…, remove defaulted one)

jmarrec added 3 commits March 8, 2023 09:56

Setup FT/RT test for SurfacePropertyOtherSideConditionsModel

0040107

RT test for SolarCollectorPerformancePhotovoltaicThermalBIPVT

3298e29

Completeness: Add an overload ctor for BIPVT that takes a SurfaceProp…

02f0cfb

…ertyOtherSideConditionsModel to avoid creating a default one

jmarrec commented Mar 8, 2023

View reviewed changes

jmarrec added this to the OpenStudio SDK 3.6.0 milestone Mar 8, 2023

jmarrec requested a review from joseph-robertson March 8, 2023 09:15

jmarrec commented Mar 8, 2023

View reviewed changes

copy paste error [ci skip]

8361971

joseph-robertson reviewed Mar 14, 2023

View reviewed changes

Merge branch 'develop' into 4802_SolarCollectorPerf_BIPVT

31d613f

jmarrec merged commit 518f3f9 into develop Mar 16, 2023

jmarrec deleted the 4802_SolarCollectorPerf_BIPVT branch March 16, 2023 10:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT #4813

Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT #4813

jmarrec commented Mar 7, 2023 •

edited

Loading

joseph-robertson Mar 7, 2023

jmarrec Mar 8, 2023 •

edited

Loading

jmarrec Mar 8, 2023

joseph-robertson Mar 14, 2023

jmarrec Mar 8, 2023

jmarrec Mar 8, 2023

joseph-robertson Mar 14, 2023

jmarrec Mar 8, 2023

joseph-robertson Mar 14, 2023

jmarrec Mar 8, 2023

jmarrec Mar 8, 2023

joseph-robertson Mar 14, 2023

jmarrec Mar 8, 2023

jmarrec Mar 8, 2023

joseph-robertson Mar 14, 2023

jmarrec Mar 8, 2023

jmarrec Mar 8, 2023

jmarrec left a comment

jmarrec Mar 8, 2023

ci-commercialbuildings commented Mar 8, 2023 •

edited

Loading

joseph-robertson left a comment

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

joseph-robertson Mar 14, 2023

jmarrec Mar 16, 2023

		* [#4813](https://github.com/NREL/OpenStudio/pull/4813) - Wrap `SolarCollectorPerformance:PhotovoltaicThermal:BIPVT`
		* `SolarCollectorFlatPlatePhotovoltaicThermal` has API-breaking changes in the `solarCollectorPerformance` getter due to the addition of this new object: it used to return a `SolarCollectorPerformancePhotovoltaicThermalSimple` (the only performance object at the time), now it's a `ModelObject`.


		namespace energyplus {

		boost::optional<ModelObject> ReverseTranslator::translateSurfacePropertyOtherSideConditionsModel(const WorkspaceObject& workspaceObject) {

	bool SolarCollectorFlatPlatePhotovoltaicThermal_Impl::setSolarCollectorPerformance(
	const SolarCollectorPerformancePhotovoltaicThermalSimple& performance) {
	bool result(false);
	SolarCollectorPerformancePhotovoltaicThermalSimple current = this->solarCollectorPerformance();
	if (current.handle() == performance.handle()) {
	return true; // no-op
	}
	ModelObject clone = performance.clone(this->model());
	result = setSolarCollectorPerformanceNoClone(clone.cast<SolarCollectorPerformancePhotovoltaicThermalSimple>());
	if (result) {
	current.remove();
	} else {
	result = setSolarCollectorPerformanceNoClone(current);
	OS_ASSERT(result);
	return false;
	}
	return result;
	}


		/// Deletes the current parameters and constructs a new default set of parameters
		/// Deletes the current performance and constructs a new default set of performance

Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT #4813

Fix #4802 - E+ 23.1.0: Wrap SolarCollectorPerformance:PhotovoltaicThermal:BIPVT #4813

Conversation

jmarrec commented Mar 7, 2023 • edited Loading

Pull request overview

Pull Request Author

Review Checklist

Choose a reason for hiding this comment

jmarrec Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-commercialbuildings commented Mar 8, 2023 • edited Loading

joseph-robertson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec commented Mar 7, 2023 •

edited

Loading

jmarrec Mar 8, 2023 •

edited

Loading

ci-commercialbuildings commented Mar 8, 2023 •

edited

Loading