-
Notifications
You must be signed in to change notification settings - Fork 461
Fix #10220 - LifeCycleCost objects are using "Steam" not "DistrictHeatingSteam" #10229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…getObjectSchemaProps`
…of std::all_of)
```
[ RUN ] EnergyPlusFixture.EconomicLifeCycleCost_GetInput_EnsureFuelTypesAllRecognized
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/EconomicLifeCycleCost.unit.cc:601: Failure
Expected equality of these values:
numResources
Which is: 18
enum_values.size()
Which is: 16
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/EconomicLifeCycleCost.unit.cc:648: Failure
Expected equality of these values:
numResources
Which is: 18
state->dataEconLifeCycleCost->numUsePriceEscalation
Which is: 16
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.hh:137: Failure
Expected: (static_cast<typename std::underlying_type_t<T>>(expected)) != (static_cast<typename std::underlying_type_t<T>>(actual)), actual: -1 vs -1
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/EconomicLifeCycleCost.unit.cc:650: Failure
Value of: compare_enums(lcc.resource, Constant::eResource::Invalid, false)
Actual: true
Expected: false
Failed for LCCUSEPRICEESCALATION STEAM
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/EconomicLifeCycleCost.unit.cc:652: Failure
Expected equality of these values:
numResources
Which is: 18
state->dataEconLifeCycleCost->numUseAdjustment
Which is: 16
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.hh:137: Failure
Expected: (static_cast<typename std::underlying_type_t<T>>(expected)) != (static_cast<typename std::underlying_type_t<T>>(actual)), actual: -1 vs -1
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/EconomicLifeCycleCost.unit.cc:654: Failure
Value of: compare_enums(lcc.resource, Constant::eResource::Invalid, false)
Actual: true
Expected: false
Failed for LCCUSEADJUSTMENT STEAM
```
| \key DistrictCooling | ||
| \key DistrictHeatingWater | ||
| \key DistrictHeatingSteam |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For LifeCycleCost:UsePriceEscalation and LifeCycleCost:UseAdjustment
Replace Steam with DistrictHeatingSteam like the rest of the objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, good catch.
| CASE('LIFECYCLECOST:USEPRICEESCALATION') | ||
| CALL GetNewObjectDefInIDD(ObjectName,NwNumArgs,NwAorN,NwReqFld,NwObjMinFlds,NwFldNames,NwFldDefaults,NwFldUnits) | ||
| nodiff=.false. | ||
| OutArgs(1:CurArgs)=InArgs(1:CurArgs) | ||
| IF (SameString(OutArgs(2), "STEAM")) THEN | ||
| OutArgs(2) = "DistrictHeatingSteam" | ||
| END IF | ||
|
|
||
| CASE('LIFECYCLECOST:USEADJUSTMENT') | ||
| CALL GetNewObjectDefInIDD(ObjectName,NwNumArgs,NwAorN,NwReqFld,NwObjMinFlds,NwFldNames,NwFldDefaults,NwFldUnits) | ||
| nodiff=.false. | ||
| OutArgs(1:CurArgs)=InArgs(1:CurArgs) | ||
| IF (SameString(OutArgs(2), "STEAM")) THEN | ||
| OutArgs(2) = "DistrictHeatingSteam" | ||
| END IF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transition. See test on DevSupport: https://github.com/NREL/EnergyPlusDevSupport/blob/master/DefectFiles/10000s/10220/transition.idf
./Transition-V23-1-0-to-V23-2-0 /path/to/EnergyPlusDevSupport/DefectFiles/10000s/10220/transition.idf
gvim -d -f transition.idfold transition.idfnew
| TEST_F(EnergyPlusFixture, EconomicLifeCycleCost_GetInput_EnsureFuelTypesAllRecognized) | ||
| { | ||
| using json = nlohmann::json; | ||
| const json &lcc_useprice_props = state->dataInputProcessing->inputProcessor->getObjectSchemaProps(*state, "LifeCycleCost:UsePriceEscalation"); | ||
| const json &resource_field = lcc_useprice_props.at("resource"); | ||
| const json &enum_values = resource_field.at("enum"); | ||
|
|
||
| // Should support all fuels + Water + ElectricityXXX (Purchased, Produced, SurplusSold, Net) | ||
| constexpr size_t numResources = static_cast<size_t>(Constant::eFuel::Num) + 5; | ||
| EXPECT_EQ(numResources, enum_values.size()); | ||
| std::string idf_objects = delimited_string({ | ||
| "LifeCycleCost:Parameters,", | ||
| " TypicalLCC, !- Name", | ||
| " EndOfYear, !- Discounting Convention", | ||
| " ConstantDollar, !- Inflation Approach", | ||
| " 0.03, !- Real Discount Rate", | ||
| " , !- Nominal Discount Rate", | ||
| " , !- Inflation", | ||
| " January, !- Base Date Month", | ||
| " 2012, !- Base Date Year", | ||
| " January, !- Service Date Month", | ||
| " 2014, !- Service Date Year", | ||
| " 100, !- Length of Study Period in Years", | ||
| " 0, !- Tax rate", | ||
| " ; !- Depreciation Method", | ||
| }); | ||
| // All should be valid resources | ||
| for (const auto &enum_value : enum_values) { | ||
| const std::string enum_string = enum_value.get<std::string>(); | ||
|
|
||
| const auto resource = static_cast<Constant::eResource>(getEnumValue(Constant::eResourceNamesUC, enum_string)); | ||
| EXPECT_TRUE(compare_enums(Constant::eResource::Invalid, resource)) << "Failed for " << enum_string; | ||
|
|
||
| idf_objects += fmt::format(R"idf( | ||
| LifeCycleCost:UsePriceEscalation, | ||
| LCCUsePriceEscalation {0}, !- Name | ||
| {0}, !- Resource | ||
| 2009, !- Escalation Start Year | ||
| January, !- Escalation Start Month | ||
| 1, !- Year Escalation 1 | ||
| 1.01, !- Year Escalation 2 | ||
| 1.02; !- Year Escalation 3 | ||
| LifeCycleCost:UseAdjustment, | ||
| LCCUseAdjustment {0}, !- Name | ||
| {0}, !- Resource | ||
| 1, !- Year Multiplier 1 | ||
| 1.005, !- Year Multiplier 2 | ||
| 1.01; !- Year Multiplier 3 | ||
| )idf", | ||
| enum_string); | ||
| } | ||
| ASSERT_TRUE(process_idf(idf_objects)); | ||
|
|
||
| GetInputForLifeCycleCost(*state); | ||
|
|
||
| EXPECT_EQ(numResources, state->dataEconLifeCycleCost->numUsePriceEscalation); | ||
| for (const auto &lcc : state->dataEconLifeCycleCost->UsePriceEscalation) { | ||
| EXPECT_FALSE(compare_enums(lcc.resource, Constant::eResource::Invalid, false)) << "Failed for " << lcc.name; | ||
| } | ||
| EXPECT_EQ(numResources, state->dataEconLifeCycleCost->numUseAdjustment); | ||
| for (const auto &lcc : state->dataEconLifeCycleCost->UseAdjustment) { | ||
| EXPECT_FALSE(compare_enums(lcc.resource, Constant::eResource::Invalid, false)) << "Failed for " << lcc.name; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(meta) testing to ensure this doesn't creep back in.
|
Using the MCVE files on DevSupport:
Ori:eplusout.err has a warning as you can see, DistrictHeatingSteam is not changed whatsoever. New:All fuels are properly affected by the LCC objects: |
| OutArgs(1:CurArgs)=InArgs(1:CurArgs) | ||
| IF (SameString(OutArgs(2), "STEAM")) THEN | ||
| OutArgs(2) = "DistrictHeatingSteam" | ||
| END IF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch, @jmarrec, thanks.
| \key DistrictCooling | ||
| \key DistrictHeatingWater | ||
| \key DistrictHeatingSteam |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, good catch.
|
This also tests happily, and the only diff is the spurious table diff. I am going to go ahead and merge this in, thanks @jmarrec |



Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.