Skip to content

Commit

Permalink
Merge pull request #9996 from NREL/ErrorHandling
Browse files Browse the repository at this point in the history
Modernized Error Reporting
  • Loading branch information
Myoldmopar authored Aug 20, 2024
2 parents 2fda843 + 472ae9a commit 9f0fc00
Show file tree
Hide file tree
Showing 29 changed files with 551 additions and 380 deletions.
89 changes: 89 additions & 0 deletions design/FY2023/ImprovedErrorHandling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Modern Error Reporting

One feature request pretty much summarizes the issue and solution for our error reporting:

> "The current solution of writing out the error file to a text file without establishing a standard procedure makes it difficult for third-party vendors to translate the content.
We would prefer to start establishing a standard template for warnings/error records with unique IDs being written to the JSON files. "

OK, so the big picture is we want an error file in JSON format with some form of ID associated with the error messages.
A nice side benefit of this would be that we could generate a bit of documentation that cross references the ID.
The documentation could be built up over time with additions like typical causes, tips and tricks for fixing or working around, etc.

## Detailed task list

To meet this need, I think development would follow something like this:

- Analyze existing structure, figuring out a feasible plan for refactoring error message calls
- Create a new error manager in EnergyPlus that tracks errors in a nice structure
- Refactor calls to error message routines to use a vector of strings rather than individual calls to *Continue*
- Refactor the error message routines to not just emit to the err file but also add them to the new error manager
- (at this point I should be able to get no diffs)
- Try to find "nearly" identical error messages and bring them together
- I would expect this to cause valid diffs in the err file as minor wording changes are collected together
- Create a set of ID categories, and modify the error emission to report the error ID
- Use the error manager to generate the JSON error file
- Create an auto-generated document for the different error message IDs

## Error File Contents

I would start creating the error file with something like this, but am totally open to feedback, and it also may naturally change as development commences:

```json
{
"summary": {
"outcome": "fatal",
"num_severe": 2,
"num_warnings": 1,
"runtime": 210.2,
"timestamp": "2023-04-24T13:05:30Z"
},
"fatal": {
"id": "F1000",
"summary": "EnergyPlus failed because of a Meta-reason",
},
"severe": [
{
"id": "S1021",
"synopsis": "Flow controller did not converge",
"extra_info": "Air loop 'AirLoop' diverged with oscillating flow rate. Flow controller 'ControlStrategy' is set to 'InverseMobiusStrip', which only exists in hyper dimensional systems. Choose a more appropriate option."
},
{
"id": "S3001",
"synopsis": "Encountered meta-building in the input file",
"extra_info": "Building 'MetaBuilding' was declared with meta-latitude '\\342\\200\\234' and meta-longitude '\\742\\234\\876', which are not supported in this version of EnergyPlus"
}
],
"warnings": [
{
"id": "W4040",
"synopsis": "Timestep is set to 3600 seconds, which is incompatible with some cool short time step model, suggest setting it to 3599 or less.",
"extra_info": ""
}
],
"information": [
"Testing Individual Branch Integrity",
"All Branches passed integrity testing",
"Testing Individual Supply Air Path Integrity",
"All Supply Air Paths passed integrity testing",
"Testing Individual Return Air Path Integrity",
"All Return Air Paths passed integrity testing",
"No node connection errors were found.",
"Beginning Simulation",
"EnergyPlus Completed Successfully"
]
}
```

We'll need to keep in mind how new errors can be introduced into the category system to future proof it.
Many tools do something like: A001, A002, B001, B002, so I would probably start by doing something similar.

Things I still need to solve:
- Should I include timestamp info for every single warning?
- Add structure for recurring
- Add extra flag for whether warmup, sizing, or kickoff were true

## Current questions for dev team

- What should we name this JSON based error file?
- Do we just keep a list of message string templates in a single dedicated file, and use these in calls to errors?
- Anyone have strong feelings on the ID or JSON form?
4 changes: 2 additions & 2 deletions src/EnergyPlus/Boilers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ void GetBoilerInput(EnergyPlusData &state)
ShowSevereError(
state, fmt::format("{}{}=\"{}\",", RoutineName, state.dataIPShortCut->cCurrentModuleObject, state.dataIPShortCut->cAlphaArgs(1)));
ShowContinueError(state, format("Invalid {}={:.3R}", state.dataIPShortCut->cNumericFieldNames(2), state.dataIPShortCut->rNumericArgs(2)));
ShowSevereError(state, format("...{} must be greater than 0.0", state.dataIPShortCut->cNumericFieldNames(2)));
ShowContinueError(state, format("...{} must be greater than 0.0", state.dataIPShortCut->cNumericFieldNames(2)));
ErrorsFound = true;
} else if (state.dataIPShortCut->rNumericArgs(2) > 1.0) {
ShowWarningError(state,
Expand Down Expand Up @@ -288,7 +288,7 @@ void GetBoilerInput(EnergyPlusData &state)
ShowSevereError(state,
fmt::format("{}{}=\"{}\"", RoutineName, state.dataIPShortCut->cCurrentModuleObject, state.dataIPShortCut->cAlphaArgs(1)));
ShowContinueError(state, format("Invalid {}={}", state.dataIPShortCut->cAlphaFieldNames(4), state.dataIPShortCut->cAlphaArgs(4)));
ShowSevereError(state, format("...{} not found.", state.dataIPShortCut->cAlphaFieldNames(4)));
ShowContinueError(state, format("...{} not found.", state.dataIPShortCut->cAlphaFieldNames(4)));
ErrorsFound = true;
}
thisBoiler.VolFlowRate = state.dataIPShortCut->rNumericArgs(3);
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/ChillerExhaustAbsorption.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,7 @@ void ExhaustAbsorberSpecs::size(EnergyPlusData &state)
if (this->CondVolFlowRateWasAutoSized) {
if (state.dataPlnt->PlantFirstSizesOkayToFinalize) {
ShowSevereError(state, format("SizeExhaustAbsorber: ChillerHeater:Absorption:DoubleEffect=\"{}\", autosize error.", this->Name));
ShowSevereError(state, "Autosizing of Exhaust Fired Absorption Chiller condenser flow rate requires a condenser");
ShowContinueError(state, "Autosizing of Exhaust Fired Absorption Chiller condenser flow rate requires a condenser");
ShowContinueError(state, "loop Sizing:Plant object.");
ErrorsFound = true;
}
Expand Down
36 changes: 18 additions & 18 deletions src/EnergyPlus/DElightManagerF.cc
Original file line number Diff line number Diff line change
Expand Up @@ -604,34 +604,34 @@ namespace DElightManagerF {

// Validate that Reference Point coordinates are within the host Zone
if (RefPt_WCS_Coord.x < thisZone.MinimumX || RefPt_WCS_Coord.x > thisZone.MaximumX) {
ShowWarningError(
state, format("DElightInputGenerator:Reference point X Value outside Zone Min/Max X, Zone={}", zn.Name));
ShowSevereError(state,
format("...X Reference Point= {:.2R}, Zone Minimum X= {:.2R}, Zone Maximum X= {:.2R}",
thisZone.MinimumX,
RefPt_WCS_Coord.x,
thisZone.MaximumX));
format("DElightInputGenerator:Reference point X Value outside Zone Min/Max X, Zone={}", zn.Name));
ShowContinueError(state,
format("...X Reference Point= {:.2R}, Zone Minimum X= {:.2R}, Zone Maximum X= {:.2R}",
thisZone.MinimumX,
RefPt_WCS_Coord.x,
thisZone.MaximumX));
ErrorsFound = true;
}
if (RefPt_WCS_Coord.y < thisZone.MinimumY || RefPt_WCS_Coord.y > thisZone.MaximumY) {
ShowWarningError(
state, format("DElightInputGenerator:Reference point Y Value outside Zone Min/Max Y, Zone={}", zn.Name));
ShowSevereError(state,
format("...Y Reference Point= {:.2R}, Zone Minimum Y= {:.2R}, Zone Maximum Y= {:.2R}",
thisZone.MinimumY,
RefPt_WCS_Coord.y,
thisZone.MaximumY));
format("DElightInputGenerator:Reference point Y Value outside Zone Min/Max Y, Zone={}", zn.Name));
ShowContinueError(state,
format("...Y Reference Point= {:.2R}, Zone Minimum Y= {:.2R}, Zone Maximum Y= {:.2R}",
thisZone.MinimumY,
RefPt_WCS_Coord.y,
thisZone.MaximumY));
ErrorsFound = true;
}
if (RefPt_WCS_Coord.z < state.dataHeatBal->Zone(izone).MinimumZ || RefPt_WCS_Coord.z > thisZone.MaximumZ) {
ShowWarningError(
ShowSevereError(
state,
format("DElightInputGenerator:Reference point Z Value outside Zone Min/Max Z, Zone={}", thisZone.Name));
ShowSevereError(state,
format("...Z Reference Point= {:.2R}, Zone Minimum Z= {:.2R}, Zone Maximum Z= {:.2R}",
thisZone.MinimumZ,
RefPt_WCS_Coord.z,
thisZone.MaximumZ));
ShowContinueError(state,
format("...Z Reference Point= {:.2R}, Zone Minimum Z= {:.2R}, Zone Maximum Z= {:.2R}",
thisZone.MinimumZ,
RefPt_WCS_Coord.z,
thisZone.MaximumZ));
ErrorsFound = true;
}

Expand Down
12 changes: 9 additions & 3 deletions src/EnergyPlus/DataSurfaceLists.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ void GetSurfaceListsInputs(EnergyPlusData &state)
cNumericFields.deallocate();
lNumericBlanks.deallocate();

if (ErrorsFound) ShowSevereError(state, format("{}{}", CurrentModuleObject1, " errors found getting input. Program will terminate."));
if (ErrorsFound) {
ShowSevereError(state, format("{}{}", CurrentModuleObject1, " errors found getting input. Program will terminate."));
}
}

if (NumOfSurfListVentSlab > 0) {
Expand Down Expand Up @@ -329,10 +331,14 @@ void GetSurfaceListsInputs(EnergyPlusData &state)
cNumericFields.deallocate();
lNumericBlanks.deallocate();

if (ErrorsFound) ShowSevereError(state, format("{}{}", CurrentModuleObject2, " errors found getting input. Program will terminate."));
if (ErrorsFound) {
ShowSevereError(state, format("{}{}", CurrentModuleObject2, " errors found getting input. Program will terminate."));
}
}

if (ErrorsFound) ShowFatalError(state, "GetSurfaceListsInputs: Program terminates due to preceding conditions.");
if (ErrorsFound) {
ShowFatalError(state, "GetSurfaceListsInputs: Program terminates due to preceding conditions.");
}
}

int GetNumberOfSurfaceLists(EnergyPlusData &state)
Expand Down
4 changes: 2 additions & 2 deletions src/EnergyPlus/DesiccantDehumidifiers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -774,8 +774,8 @@ namespace DesiccantDehumidifiers {
DataLoopNode::ObjectIsNotParent);

if (desicDehum.ControlNodeNum == 0) {
ShowContinueError(state, format("{} = \"{}\"", desicDehum.DehumType, desicDehum.Name));
ShowSevereError(state, format("{} must be specified.", cAlphaFields(5)));
ShowSevereError(state, format("{} = \"{}\"", desicDehum.DehumType, desicDehum.Name));
ShowContinueError(state, format("{} must be specified.", cAlphaFields(5)));
ErrorsFoundGeneric = true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/EnergyPlus/FuelCellElectricGenerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ namespace FuelCellElectricGenerator {
DataGenerators::AirSupRateMode::QuadraticFuncofPel)) {
ShowSevereError(state, format("Invalid, {} = {}", state.dataIPShortCut->cAlphaFieldNames(5), AlphArray(5)));
ShowContinueError(state, format("Entered in {}={}", state.dataIPShortCut->cCurrentModuleObject, AlphArray(1)));
ShowSevereError(state, "Curve name was not found");
ShowContinueError(state, "Curve name was not found");
ErrorsFound = true;
}

Expand All @@ -460,7 +460,7 @@ namespace FuelCellElectricGenerator {
DataGenerators::AirSupRateMode::QuadraticFuncofNdot)) {
ShowSevereError(state, format("Invalid, {} = {}", state.dataIPShortCut->cAlphaFieldNames(6), AlphArray(6)));
ShowContinueError(state, format("Entered in {}={}", state.dataIPShortCut->cCurrentModuleObject, AlphArray(1)));
ShowSevereError(state, "Curve name was not found");
ShowContinueError(state, "Curve name was not found");
ErrorsFound = true;
}

Expand Down
10 changes: 5 additions & 5 deletions src/EnergyPlus/Furnaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -937,15 +937,15 @@ namespace Furnaces {
}
if (!AirNodeFound) {
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
ShowSevereError(state, "Did not find Air Node (Zone with Thermostat).");
ShowContinueError(state, "Did not find Air Node (Zone with Thermostat).");
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(6), Alphas(6)));
ShowContinueError(
state, "Both a ZoneHVAC:EquipmentConnections object and a ZoneControl:Thermostat object must be specified for this zone.");
ErrorsFound = true;
}
if (!AirLoopFound) {
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
ShowSevereError(state, "Did not find correct Primary Air Loop.");
ShowContinueError(state, "Did not find correct Primary Air Loop.");
ShowContinueError(state, format("Specified {} = {} is not served by this AirLoopHVAC equipment.", cAlphaFields(6), Alphas(6)));
ErrorsFound = true;
}
Expand Down Expand Up @@ -1478,7 +1478,7 @@ namespace Furnaces {
}
if (!AirLoopFound) {
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
ShowSevereError(state, "Did not find correct AirLoopHVAC.");
ShowContinueError(state, "Did not find correct AirLoopHVAC.");
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(6), Alphas(6)));
ErrorsFound = true;
}
Expand Down Expand Up @@ -2759,7 +2759,7 @@ namespace Furnaces {
}
if (!AirLoopFound) {
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
ShowSevereError(state, "Did not find correct AirLoopHVAC.");
ShowContinueError(state, "Did not find correct AirLoopHVAC.");
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(5), Alphas(5)));
ErrorsFound = true;
}
Expand Down Expand Up @@ -3680,7 +3680,7 @@ namespace Furnaces {
}
if (!AirLoopFound) {
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
ShowSevereError(state, "Did not find correct AirLoopHVAC.");
ShowContinueError(state, "Did not find correct AirLoopHVAC.");
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(5), Alphas(5)));
ErrorsFound = true;
}
Expand Down
11 changes: 6 additions & 5 deletions src/EnergyPlus/HVACControllers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -481,17 +481,18 @@ void GetControllerInput(EnergyPlusData &state)
controllerProps.ControlVar = static_cast<EnergyPlus::HVACControllers::CtrlVarType>(getEnumValue(ctrlVarNamesUC, AlphArray(2)));
if (controllerProps.ControlVar == HVACControllers::CtrlVarType::Invalid) {
ShowSevereError(state, format("{}{}=\"{}\".", RoutineName, CurrentModuleObject, AlphArray(1)));
ShowSevereError(state,
format("...Invalid {}=\"{}\", must be Temperature, HumidityRatio, or TemperatureAndHumidityRatio.",
cAlphaFields(2),
AlphArray(2)));
ShowContinueError(state,
format("...Invalid {}=\"{}\", must be Temperature, HumidityRatio, or TemperatureAndHumidityRatio.",
cAlphaFields(2),
AlphArray(2)));
ErrorsFound = true;
}

controllerProps.Action = static_cast<ControllerAction>(getEnumValue(actionNamesUC, AlphArray(3)));
if (controllerProps.Action == ControllerAction::Invalid) {
ShowSevereError(state, format("{}{}=\"{}\".", RoutineName, CurrentModuleObject, AlphArray(1)));
ShowSevereError(state, format("...Invalid {}=\"{}{}", cAlphaFields(3), AlphArray(3), R"(", must be "Normal", "Reverse" or blank.)"));
ShowContinueError(state,
format("...Invalid {}=\"{}{}", cAlphaFields(3), AlphArray(3), R"(", must be "Normal", "Reverse" or blank.)"));
ErrorsFound = true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/HeatBalanceManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1622,7 +1622,7 @@ namespace HeatBalanceManager {
ShowSevereError(
state,
format("Invalid material layer type in window {} = {}", state.dataHeatBalMgr->CurrentModuleObject, thisConstruct.Name));
ShowSevereError(
ShowContinueError(
state,
format("Equivalent Layer material type = {} is allowed only in Construction:WindowEquivalentLayer window object.",
ConstructAlphas(Layer)));
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/Humidifiers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ namespace Humidifiers {
} else if (!lAlphaBlanks(3)) {
ShowSevereError(state, format("{}{}=\"{}\",", RoutineName, CurrentModuleObject, Alphas(1)));
ShowContinueError(state, format("Invalid {}={}", cAlphaFields(3), Alphas(3)));
ShowSevereError(state, format("...{} not found.", cAlphaFields(3)));
ShowContinueError(state, format("...{} not found.", cAlphaFields(3)));
ErrorsFound = true;
}

Expand Down
Loading

4 comments on commit 9f0fc00

@nrel-bot-3
Copy link

Choose a reason for hiding this comment

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

develop (Myoldmopar) - x86_64-MacOS-10.18-clang-15.0.0: Tests Failed (2 of 2872 tests passed, 0 test warnings)

Failures:\n

API Test Summary

  • Failed: 10
  • notrun: 5

ConvertInputFormat Test Summary

  • Failed: 4
  • notrun: 1

integration Test Summary

  • Passed: 2
  • Failed: 794

Build Badge Test Badge

@nrel-bot
Copy link

Choose a reason for hiding this comment

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

develop (Myoldmopar) - Win64-Windows-10-VisualStudio-16: OK (2871 of 2871 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-2
Copy link

Choose a reason for hiding this comment

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

develop (Myoldmopar) - x86_64-Linux-Ubuntu-22.04-gcc-11.4: OK (2893 of 2893 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-2
Copy link

Choose a reason for hiding this comment

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

develop (Myoldmopar) - x86_64-Linux-Ubuntu-22.04-gcc-11.4-UnitTestsCoverage-Debug: OK (2077 of 2077 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

Please sign in to comment.