Skip to content
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

Round2 init state use #10662

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/EnergyPlus/CurveManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@

// EnergyPlus Headers
#include <EnergyPlus/CurveManager.hh>
#include <EnergyPlus/Data/EnergyPlusData.hh>
#include <EnergyPlus/DataBranchAirLoopPlant.hh>
#include <EnergyPlus/DataIPShortCuts.hh>
#include <EnergyPlus/DataLoopNode.hh>
Expand Down Expand Up @@ -572,7 +571,6 @@ namespace Curve {
bool GetInputErrorsFound = false;

GetCurveInputData(state, GetInputErrorsFound);
state.dataCurveManager->GetCurvesInputFlag = false;

if (GetInputErrorsFound) {
ShowFatalError(state, "GetCurveInput: Errors found in getting Curve Objects. Preceding condition(s) cause termination.");
Expand Down Expand Up @@ -611,6 +609,8 @@ namespace Curve {
int IOStatus; // Used in GetObjectItem
std::string CurrentModuleObject; // for ease in renaming.

if (!state.dataCurveManager->GetCurvesInputFlag) return;
state.dataCurveManager->GetCurvesInputFlag = false;
Copy link
Contributor Author

@rraustad rraustad Aug 16, 2024

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

😆 🤣 😄 This is excellent @rraustad !

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow.

// Find the number of each type of curve (note: Current Module object not used here, must rename manually)

int const NumBiQuad = state.dataInputProcessing->inputProcessor->getNumObjectsFound(state, "Curve:Biquadratic");
Expand Down Expand Up @@ -653,6 +653,7 @@ namespace Curve {
// initialize the array

int CurveNum = 0; // keep track of the current curve index in the main curve array
state.dataCurveManager->UniqueCurveNames.clear();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only change needed to allow curve manager getInput to be called more than once. It will likely not be called more than once but if it does it will give the same answer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably better to set a flag and make sure the body is not called twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the outer getInput call does protect from the second call. If a unit test calls the root function multiple times it would need to give the same answer. I think I did this before removing init_state from the unit test fixture (last thing I did) so maybe I can fix this and not have to change any unit tests. I'll just move that flag down into GetCurveInputData.

    if (state.dataCurveManager->GetCurvesInputFlag) {
        GetCurveInput(state);
        GetPressureSystemInput(state);
        state.dataCurveManager->GetCurvesInputFlag = false;
    }

void GetCurveInput(EnergyPlusData &state)
{
    // wrapper for GetInput to allow unit testing when fatal inputs are detected - follow pattern from GetSetPointManagerInputs()
    bool GetInputErrorsFound = false;

    GetCurveInputData(state, GetInputErrorsFound);
    state.dataCurveManager->GetCurvesInputFlag = false;

    if (GetInputErrorsFound) {
        ShowFatalError(state, "GetCurveInput: Errors found in getting Curve Objects.  Preceding condition(s) cause termination.");
    }
}

void GetCurveInputData(EnergyPlusData &state, bool &ErrorsFound)

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a general practice in EnergyPlus of "protecting functions from bad inputs or bad calling contexts". This is actually counter-productive because it hides bad uses of the function. It's better programming to assert good inputs and good context (to the extent possible) within a function and then use those to progressively clean up the calling contexts.


// Loop over biquadratic curves and load data
CurrentModuleObject = "Curve:Biquadratic";
Expand Down
6 changes: 5 additions & 1 deletion src/EnergyPlus/CurveManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@

// EnergyPlus Headers
#include <EnergyPlus/Data/BaseData.hh>
#include <EnergyPlus/Data/EnergyPlusData.hh>
#include <EnergyPlus/DataBranchAirLoopPlant.hh>
#include <EnergyPlus/DataGlobals.hh>
#include <EnergyPlus/EPVector.hh>
Expand Down Expand Up @@ -469,8 +470,11 @@ struct CurveManagerData : BaseGlobalStruct
this->PerfCurve.push_back(new EnergyPlus::Curve::Curve);
}

void init_state([[maybe_unused]] EnergyPlusData &state) override
void init_state(EnergyPlusData &state) override
{
Curve::GetCurveInput(state);
Curve::GetPressureSystemInput(state);
state.dataCurveManager->GetCurvesInputFlag = false;
}

void clear_state() override
Expand Down
10 changes: 7 additions & 3 deletions src/EnergyPlus/Data/EnergyPlusData.cc
Original file line number Diff line number Diff line change
Expand Up @@ -572,14 +572,18 @@ void EnergyPlusData::clear_state()
void EnergyPlusData::init_state(EnergyPlusData &state)
{
if (this->init_state_called) return;

state.dataGlobal->InputsFlag = true;
this->init_state_called = true;
// The order in which we do this matters. We're going to try to
// do this in "topological" order meaning the first to go are the
// objects that do not reference any other objects, like fluids,
// schedules, curves, etc.
this->dataSimulationManager->init_state(state); // GetProjectData
this->dataSimulationManager->init_state(state); // OpenOutputFiles, GetProjectData, SetPreConstructionInputParameters
this->dataFluidProps->init_state(state); // GetFluidPropertiesData
this->dataPsychrometrics->init_state(state); // InitializePsychRoutines
this->dataScheduleMgr->init_state(state); // ProcessScheduleInput - requires NumOfTimeStepInHour and AnyEnergyManagementSystemInModel
this->dataCurveManager->init_state(state); // GetCurveInput, GetPressureSystemInput
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added schedules and curves to init_state


this->dataAirLoop->init_state(state);
this->dataAirLoopHVACDOAS->init_state(state);
Expand Down Expand Up @@ -614,7 +618,6 @@ void EnergyPlusData::init_state(EnergyPlusData &state)
this->dataCoolTower->init_state(state);
this->dataCostEstimateManager->init_state(state);
this->dataCrossVentMgr->init_state(state);
this->dataCurveManager->init_state(state);
this->dataDXCoils->init_state(state);
this->dataDXFEarClipping->init_state(state);
this->dataDaylightingDevices->init_state(state);
Expand Down Expand Up @@ -758,7 +761,6 @@ void EnergyPlusData::init_state(EnergyPlusData &state)
this->dataRuntimeLang->init_state(state);
this->dataRuntimeLangProcessor->init_state(state);
this->dataSQLiteProcedures->init_state(state);
this->dataScheduleMgr->init_state(state);
this->dataSetPointManager->init_state(state);
this->dataShadowComb->init_state(state);
this->dataSimAirServingZones->init_state(state);
Expand Down Expand Up @@ -826,6 +828,8 @@ void EnergyPlusData::init_state(EnergyPlusData &state)
this->dataZoneEquipmentManager->init_state(state);
this->dataZonePlenum->init_state(state);
this->dataZoneTempPredictorCorrector->init_state(state);

state.dataGlobal->InputsFlag = false;
}

} // namespace EnergyPlus
2 changes: 2 additions & 0 deletions src/EnergyPlus/DataErrorTracking.hh
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ struct ErrorTrackingData : BaseGlobalStruct
int NumRecurringErrors = 0; // Number of stored recurring error messages
int TotalSevereErrors = 0; // Counter
int TotalWarningErrors = 0; // Counter
int TotalSevereErrorsDuringInputs = 0; // Counter
int TotalWarningErrorsDuringInputs = 0; // Counter
int TotalSevereErrorsDuringWarmup = 0; // Counter
int TotalWarningErrorsDuringWarmup = 0; // Counter
int TotalSevereErrorsDuringSizing = 0; // Counter
Expand Down
1 change: 1 addition & 0 deletions src/EnergyPlus/DataGlobals.hh
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ struct DataGlobal : BaseGlobalStruct
int numSpaceTypes = 0; // Number of unique space types
int TimeStep = 0; // Counter for time steps (fractional hours)
Real64 TimeStepZone = 0.0; // Zone time step in fractional hours
bool InputsFlag = false; // True during the init_state portion of a simulation
bool WarmupFlag = false; // True during the warmup portion of a simulation
Int64 StdOutputRecordCount = 0; // Count of Standard output records
Int64 StdMeterRecordCount = 0; // Count of Meter output records
Expand Down
4 changes: 3 additions & 1 deletion src/EnergyPlus/ScheduleManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,10 @@ struct ScheduleManagerData : BaseGlobalStruct
Array1D<ScheduleManager::WeekScheduleData> WeekSchedule; // Week Schedule Storage
Array1D<ScheduleManager::ScheduleData> Schedule; // Schedule Storage

void init_state([[maybe_unused]] EnergyPlusData &state) override
void init_state(EnergyPlusData &state) override
{
ScheduleManager::ProcessScheduleInput(state);
ScheduleInputProcessed = true;
}

void clear_state() override
Expand Down
6 changes: 2 additions & 4 deletions src/EnergyPlus/SimulationManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ extern "C" {
#include <EnergyPlus/DemandManager.hh>
#include <EnergyPlus/DisplayRoutines.hh>
#include <EnergyPlus/DualDuct.hh>
#include <EnergyPlus/EMSManager.hh>
#include <EnergyPlus/EconomicLifeCycleCost.hh>
#include <EnergyPlus/EconomicTariff.hh>
#include <EnergyPlus/ElectricPowerServiceManager.hh>
Expand All @@ -102,7 +101,6 @@ extern "C" {
#include <EnergyPlus/HVACSizingSimulationManager.hh>
#include <EnergyPlus/HeatBalanceAirManager.hh>
#include <EnergyPlus/HeatBalanceIntRadExchange.hh>
#include <EnergyPlus/HeatBalanceManager.hh>
#include <EnergyPlus/HeatBalanceSurfaceManager.hh>
#include <EnergyPlus/InputProcessing/InputProcessor.hh>
#include <EnergyPlus/MixedAir.hh>
Expand Down Expand Up @@ -211,10 +209,11 @@ namespace SimulationManager {

state.init_state(state);

// these checks report warnings for SimulationControl setup, no actual inputs are read in
CheckForMisMatchedEnvironmentSpecifications(state);
CheckForRequestedReporting(state);
// set up pre-defined reports, no actual inputs are read in
OutputReportPredefined::SetPredefinedTables(state);
SetPreConstructionInputParameters(state); // establish array bounds for constructions early

OutputProcessor::SetupTimePointers(
state, OutputProcessor::TimeStepType::Zone, state.dataGlobal->TimeStepZone); // Set up Time pointer for HB/Zone Simulation
Expand Down Expand Up @@ -2635,7 +2634,6 @@ namespace SimulationManager {
// there is no need to process that data if there are no DOAS used in the simulation
state.dataGlobal->AirLoopHVACDOASUsedInSim =
state.dataInputProcessing->inputProcessor->getNumObjectsFound(state, "AirLoopHVAC:DedicatedOutdoorAirSystem") > 0;
EMSManager::CheckIfAnyEMS(state);
PlantManager::CheckIfAnyPlant(state);
PlantPipingSystemsManager::CheckIfAnySlabs(state);
PlantPipingSystemsManager::CheckIfAnyBasements(state);
Expand Down
4 changes: 4 additions & 0 deletions src/EnergyPlus/SimulationManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@

// EnergyPlus Headers
#include <EnergyPlus/Data/BaseData.hh>
#include <EnergyPlus/EMSManager.hh>
#include <EnergyPlus/EnergyPlus.hh>
#include <EnergyPlus/FileSystem.hh>
#include <EnergyPlus/HeatBalanceManager.hh>

namespace EnergyPlus {

Expand Down Expand Up @@ -103,6 +105,8 @@ struct SimulationManagerData : BaseGlobalStruct
{
SimulationManager::OpenOutputFiles(state);
SimulationManager::GetProjectData(state);
EMSManager::CheckIfAnyEMS(state); // required prior to ProcessScheduleInput
HeatBalanceManager::SetPreConstructionInputParameters(state); // establish array bounds for constructions early
}

void clear_state() override
Expand Down
8 changes: 6 additions & 2 deletions src/EnergyPlus/UtilityRoutines.cc
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,10 @@ int AbortEnergyPlus(EnergyPlusData &state)
CloseMiscOpenFiles(state);
NumWarnings = fmt::to_string(state.dataErrTracking->TotalWarningErrors);
NumSevere = fmt::to_string(state.dataErrTracking->TotalSevereErrors);
NumWarningsDuringWarmup = fmt::to_string(state.dataErrTracking->TotalWarningErrorsDuringWarmup);
NumSevereDuringWarmup = fmt::to_string(state.dataErrTracking->TotalSevereErrorsDuringWarmup);
NumWarningsDuringWarmup =
fmt::to_string(state.dataErrTracking->TotalWarningErrorsDuringWarmup + state.dataErrTracking->TotalWarningErrorsDuringInputs);
NumSevereDuringWarmup =
fmt::to_string(state.dataErrTracking->TotalSevereErrorsDuringWarmup + state.dataErrTracking->TotalSevereErrorsDuringInputs);
NumWarningsDuringSizing = fmt::to_string(state.dataErrTracking->TotalWarningErrorsDuringSizing);
NumSevereDuringSizing = fmt::to_string(state.dataErrTracking->TotalSevereErrorsDuringSizing);

Expand Down Expand Up @@ -928,6 +930,7 @@ void ShowSevereError(EnergyPlusData &state, std::string const &ErrorMessage, Opt
}

++state.dataErrTracking->TotalSevereErrors;
if (state.dataGlobal->InputsFlag) ++state.dataErrTracking->TotalSevereErrorsDuringInputs;
if (state.dataGlobal->WarmupFlag && !state.dataGlobal->DoingSizing && !state.dataGlobal->KickOffSimulation &&
!state.dataErrTracking->AbortProcessing)
++state.dataErrTracking->TotalSevereErrorsDuringWarmup;
Expand Down Expand Up @@ -1129,6 +1132,7 @@ void ShowWarningError(EnergyPlusData &state, std::string const &ErrorMessage, Op
}

++state.dataErrTracking->TotalWarningErrors;
if (state.dataGlobal->InputsFlag) ++state.dataErrTracking->TotalWarningErrorsDuringInputs;
if (state.dataGlobal->WarmupFlag && !state.dataGlobal->DoingSizing && !state.dataGlobal->KickOffSimulation &&
!state.dataErrTracking->AbortProcessing)
++state.dataErrTracking->TotalWarningErrorsDuringWarmup;
Expand Down
2 changes: 1 addition & 1 deletion tst/EnergyPlus/unit/DXCoils.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4633,7 +4633,7 @@ TEST_F(EnergyPlusFixture, TestMultiSpeedCoilsAutoSizingOutput)
DX Cooling Coil AHRI 2023 Standard Rating Information, Coil:Cooling:DX:MultiSpeed, ASHP CLG COIL, 31156.1, 3.73, 12.72, 15.17, 15.98, 15.0
)EIO";
replace_pipes_with_spaces(clg_coil_eio_output);
EXPECT_TRUE(compare_eio_stream_substring(clg_coil_eio_output, true));
EXPECT_TRUE(compare_eio_stream(clg_coil_eio_output, true));

// check multi-speed DX heating coil
EXPECT_EQ("ASHP HTG COIL", state->dataDXCoils->DXCoil(2).Name);
Expand Down
11 changes: 1 addition & 10 deletions tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,6 @@ bool EnergyPlusFixture::compare_eio_stream(std::string const &expected_string, b
return are_equal;
}

bool EnergyPlusFixture::compare_eio_stream_substring(std::string const &search_string, bool reset_stream)
{
auto const stream_str = state->files.eio.get_output();
bool const found = stream_str.find(search_string) != std::string::npos;
EXPECT_TRUE(found);
if (reset_stream) state->files.eio.open_as_stringstream();
return found;
}

bool EnergyPlusFixture::compare_mtr_stream(std::string const &expected_string, bool reset_stream)
{
auto const stream_str = state->files.mtr.get_output();
Expand Down Expand Up @@ -360,7 +351,7 @@ bool EnergyPlusFixture::process_idf(std::string_view const idf_snippet, bool use
inputProcessor->initializeMaps();
SimulationManager::PostIPProcessing(*state);

state->init_state(*state);
FluidProperties::GetFluidPropertiesData(*state);

if (state->dataSQLiteProcedures->sqlite) {
bool writeOutputToSQLite = false;
Expand Down
7 changes: 0 additions & 7 deletions tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.hh
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,6 @@ protected:
// Will return true if string matches the stream and false if it does not
bool compare_eio_stream(std::string const &expected_string, bool reset_stream = true);

// Check if EIO string contains a substring. The default is to reset the EIO stream after every call.
// It is easier to test successive functions if the EIO stream is 'empty' before the next call.
// This calls EXPECT_* within the function as well as returns a boolean so you can call [ASSERT/EXPECT]_[TRUE/FALSE] depending
// if it makes sense for the unit test to continue after returning from function.
// Will return true if string matches the stream and false if it does not
bool compare_eio_stream_substring(std::string const &expected_string, bool reset_stream = true);

// Compare an expected string against the MTR stream. The default is to reset the MTR stream after every call.
// It is easier to test successive functions if the MTR stream is 'empty' before the next call.
// This calls EXPECT_* within the function as well as returns a boolean so you can call [ASSERT/EXPECT]_[TRUE/FALSE] depending
Expand Down
2 changes: 1 addition & 1 deletion tst/EnergyPlus/unit/Furnaces.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ namespace EnergyPlus {
TEST_F(EnergyPlusFixture, SetVSHPAirFlowTest_VSFurnaceFlowTest)
{

state->init_state(*state);
state->init_state(*state); // get FluidProperties
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, why did I have to leave this here when FluidProperties is called in the fixture? Maybe this unit test needed something else now in init_state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's right. If the unit test does not have an idf snippet then process_idf will not be called and default fluids will not be set up yet. So this is correct.

int FurnaceNum(1);
Real64 OnOffAirFlowRatio; // This is a return value
Real64 PartLoadRatio(1.0);
Expand Down
4 changes: 2 additions & 2 deletions tst/EnergyPlus/unit/MixedAir.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6563,10 +6563,10 @@ TEST_F(EnergyPlusFixture, CO2ControlDesignOARateTest)
" ** ~~~ ** This may be overriding desired ventilation controls. Check inputs for Minimum Outdoor Air Flow Rate, Minimum Outdoor Air "
"Schedule Name and Controller:MechanicalVentilation",
" ** ~~~ ** Minimum OA fraction = 2.9412E-003, Mech Vent OA fraction = 1.5603E-003",
" ** ~~~ ** Environment=, at Simulation time= 00:00 - 00:00",
" ** ~~~ ** Environment=, at Simulation time= 00:15 - 00:15",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that Timestep is processed in process_idf the time stamp is the first time step of the simulation. Before, NumTimeStepInHour = 0, now it is non-zero and Timestep defaults to 4.

});

EXPECT_TRUE(compare_err_stream_substring(error_string, true));
EXPECT_TRUE(compare_err_stream(error_string, true));

state->dataAirLoop->AirLoopControlInfo.deallocate();
state->dataSize->OARequirements.deallocate();
Expand Down
2 changes: 1 addition & 1 deletion tst/EnergyPlus/unit/OutdoorAirUnit.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ TEST_F(EnergyPlusFixture, OutdoorAirUnit_AutoSize)
" ** ~~~ ** Air mass balance is required by other outdoor air units: Fan:ZoneExhaust, ZoneMixing, ZoneCrossMixing, or other air flow "
"control inputs.",
" ** ~~~ ** The outdoor mass flow rate = 0.602 and the exhaust mass flow rate = 0.000.",
" ** ~~~ ** Environment=, at Simulation time= 00:00 - 00:00",
" ** ~~~ ** Environment=, at Simulation time= 00:15 - 00:15",
});

EXPECT_TRUE(compare_err_stream(error_string, true));
Expand Down
2 changes: 2 additions & 0 deletions tst/EnergyPlus/unit/OutputFiles.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ TEST_F(EnergyPlusFixture, OutputControlFiles)
" ** ~~~ ** See InputOutputReference document for more details.",
" ************* Object=Building=Bldg",
" ** ~~~ ** Object=GlobalGeometryRules",
" ** ~~~ ** Object=Timestep",
" ** ~~~ ** Object=Version",
});

compare_err_stream(expected_error);
Expand Down
Loading