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
33 changes: 29 additions & 4 deletions src/EnergyPlus/CurveManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,13 @@

// EnergyPlus Headers
#include <EnergyPlus/CurveManager.hh>
#include <EnergyPlus/Data/EnergyPlusData.hh>
#include <EnergyPlus/DataBranchAirLoopPlant.hh>
#include <EnergyPlus/DataIPShortCuts.hh>
#include <EnergyPlus/DataLoopNode.hh>
#include <EnergyPlus/DataSystemVariables.hh>
#include <EnergyPlus/EMSManager.hh>
#include <EnergyPlus/FileSystem.hh>
#include <EnergyPlus/GlobalNames.hh>
#include <EnergyPlus/InputProcessing/InputProcessor.hh>
#include <EnergyPlus/OutputProcessor.hh>
#include <EnergyPlus/UtilityRoutines.hh>

Expand Down Expand Up @@ -572,7 +570,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 +608,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 +652,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 All @@ -675,6 +675,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getObjectItem removes an object from the unusedInputs list just because it was read in. This change puts that curve back in the list and waits for a call to getCurveIndex before removing the curve from the unusedInputs list. This change was prompted by IndoorLivingWall not showing the unused curve warning in this branch. Now curves shown as unused are actually not used in the simulation (i.e., if I have 10 curves in the input and only use 1 then 9 curves are unused).

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably change GetObjectItem to not do this at some point.

++CurveNum;

Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);
Expand Down Expand Up @@ -758,6 +759,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -824,6 +826,7 @@ namespace Curve {
_,
state.dataIPShortCut->cAlphaFieldNames,
state.dataIPShortCut->cNumericFieldNames);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
GlobalNames::VerifyUniqueInterObjectName(state,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't have to be part of this PR, but look at how I am using std::map to implement both VerifyUniqueObjectName and GetObjectIndex in one fast structure. There are examples in Fans, OutputProcessor and SetPointManager.

state.dataCurveManager->UniqueCurveNames,
Expand Down Expand Up @@ -894,6 +897,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -958,6 +962,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1022,6 +1027,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1103,6 +1109,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1184,6 +1191,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1248,6 +1256,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1329,6 +1338,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1451,6 +1461,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1530,6 +1541,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1610,6 +1622,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1663,6 +1676,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1731,6 +1745,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1797,6 +1812,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1863,6 +1879,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1929,6 +1946,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -1995,6 +2013,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -2061,6 +2080,7 @@ namespace Curve {
CurrentModuleObject,
state.dataIPShortCut->cAlphaFieldNames(1),
ErrorsFound);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
Curve *thisCurve = state.dataCurveManager->PerfCurve(CurveNum);

Expand Down Expand Up @@ -2121,6 +2141,7 @@ namespace Curve {
state.dataIPShortCut->cAlphaFieldNames,
state.dataIPShortCut->cNumericFieldNames);

state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
std::string wpcName = Alphas(1); // Name of CP array
int numWindDir = NumNumbers;
std::vector<Real64> windDirs(numWindDir);
Expand Down Expand Up @@ -2171,6 +2192,7 @@ namespace Curve {
_,
state.dataIPShortCut->cAlphaFieldNames,
state.dataIPShortCut->cNumericFieldNames);
state.dataInputProcessing->inputProcessor->unusedInputs.emplace(CurrentModuleObject, Alphas(1));
++CurveNum;
GlobalNames::VerifyUniqueInterObjectName(state,
state.dataCurveManager->UniqueCurveNames,
Expand Down Expand Up @@ -3071,7 +3093,10 @@ namespace Curve {

if (state.dataCurveManager->NumCurves > 0) {
for (int Count = 1; Count <= (int)state.dataCurveManager->PerfCurve.size(); ++Count) {
if (CurveName == state.dataCurveManager->PerfCurve(Count)->Name) return Count;
if (CurveName == state.dataCurveManager->PerfCurve(Count)->Name) {
state.dataCurveManager->PerfCurve(Count)->markUsed(state);
return Count;
}
}
return 0; // Not found
} else {
Expand Down
21 changes: 19 additions & 2 deletions src/EnergyPlus/CurveManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@

// EnergyPlus Headers
#include <EnergyPlus/Data/BaseData.hh>
#include <EnergyPlus/Data/EnergyPlusData.hh>
#include <EnergyPlus/DataBranchAirLoopPlant.hh>
#include <EnergyPlus/DataGlobals.hh>
#include <EnergyPlus/EPVector.hh>
#include <EnergyPlus/EnergyPlus.hh>
#include <EnergyPlus/EnergyPlusLogger.hh>
#include <EnergyPlus/FileSystem.hh>
#include <EnergyPlus/InputProcessing/InputProcessor.hh>
#include <EnergyPlus/UtilityRoutines.hh>

namespace EnergyPlus {
Expand Down Expand Up @@ -150,7 +152,8 @@ namespace Curve {
struct Curve
{
// Basic data
std::string Name; // Curve Name
std::string Name; // Curve Name
bool used = true;
CurveType curveType = CurveType::Invalid; // Curve type (see parameter definitions above)
// Table data stuff
InterpType interpolationType = InterpType::Invalid; // Table interpolation method
Expand Down Expand Up @@ -207,6 +210,17 @@ namespace Curve {
const Real64 Var4, // 4th independent variable
const Real64 Var5, // 5th independent variable
const Real64 Var6);

void markUsed(EnergyPlusData &state)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like this. Leave it for now but I'm going to look at how InputProcessor is doing this.

{
if (!this->used || this->curveType == CurveType::Invalid) return;
auto const find_unused =
state.dataInputProcessing->inputProcessor->unusedInputs.find({std::string(objectNames[int(this->curveType)]), this->Name});
if (find_unused != state.dataInputProcessing->inputProcessor->unusedInputs.end()) {
state.dataInputProcessing->inputProcessor->unusedInputs.erase(find_unused);
}
this->used = false;
};
};

// Table file objects
Expand Down Expand Up @@ -469,8 +483,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
3 changes: 2 additions & 1 deletion src/EnergyPlus/InputProcessing/InputProcessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,8 @@ bool InputProcessor::checkVersionMatch(EnergyPlusData &state)
Which = static_cast<int>(index(v, MatchVersion));
}
if (Which != 0) {
ShowWarningError(state, "Version: in IDF=\"" + v + "\" not the same as expected=\"" + MatchVersion + "\"");
// this is reported in GetProjectData
// ShowWarningError(state, "Version: in IDF=\"" + v + "\" not the same as expected=\"" + MatchVersion + "\"");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am seeing duplicate messages so either the version issue is reported here or where the version object is read. I chose to comment this out here but maybe there is a reason to do the opposite (e.g., API calls).

** Warning ** Version: in IDF="24.1" not the same as expected="24.2"
** Warning ** Version: in IDF="24.1" not the same as expected="24.2"

return false;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/EnergyPlus/InputProcessing/InputProcessor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,11 @@ private:
UnorderedObjectCacheMap objectCacheMap;

// ObjectType to vector of ObjectName
UnusedObjectSet unusedInputs;
char s[129] = {0};

public:
UnusedObjectSet unusedInputs;

}; // InputProcessor

struct DataInputProcessing : BaseGlobalStruct
Expand Down
Loading
Loading