Skip to content

Commit 9f0fc00

Browse files
MyoldmoparMyoldmopar
authored
Merge pull request #9996 from NREL/ErrorHandling
Modernized Error Reporting
2 parents 2fda843 + 472ae9a commit 9f0fc00

29 files changed

+551
-380
lines changed

‎design/FY2023/ImprovedErrorHandling.md

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# Modern Error Reporting
2+
3+
One feature request pretty much summarizes the issue and solution for our error reporting:
4+
5+
> "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.
6+
We would prefer to start establishing a standard template for warnings/error records with unique IDs being written to the JSON files. "
7+
8+
OK, so the big picture is we want an error file in JSON format with some form of ID associated with the error messages.
9+
A nice side benefit of this would be that we could generate a bit of documentation that cross references the ID.
10+
The documentation could be built up over time with additions like typical causes, tips and tricks for fixing or working around, etc.
11+
12+
## Detailed task list
13+
14+
To meet this need, I think development would follow something like this:
15+
16+
- Analyze existing structure, figuring out a feasible plan for refactoring error message calls
17+
- Create a new error manager in EnergyPlus that tracks errors in a nice structure
18+
- Refactor calls to error message routines to use a vector of strings rather than individual calls to *Continue*
19+
- Refactor the error message routines to not just emit to the err file but also add them to the new error manager
20+
- (at this point I should be able to get no diffs)
21+
- Try to find "nearly" identical error messages and bring them together
22+
- I would expect this to cause valid diffs in the err file as minor wording changes are collected together
23+
- Create a set of ID categories, and modify the error emission to report the error ID
24+
- Use the error manager to generate the JSON error file
25+
- Create an auto-generated document for the different error message IDs
26+
27+
## Error File Contents
28+
29+
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:
30+
31+
```json
32+
{
33+
"summary": {
34+
"outcome": "fatal",
35+
"num_severe": 2,
36+
"num_warnings": 1,
37+
"runtime": 210.2,
38+
"timestamp": "2023-04-24T13:05:30Z"
39+
},
40+
"fatal": {
41+
"id": "F1000",
42+
"summary": "EnergyPlus failed because of a Meta-reason",
43+
},
44+
"severe": [
45+
{
46+
"id": "S1021",
47+
"synopsis": "Flow controller did not converge",
48+
"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."
49+
},
50+
{
51+
"id": "S3001",
52+
"synopsis": "Encountered meta-building in the input file",
53+
"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"
54+
}
55+
],
56+
"warnings": [
57+
{
58+
"id": "W4040",
59+
"synopsis": "Timestep is set to 3600 seconds, which is incompatible with some cool short time step model, suggest setting it to 3599 or less.",
60+
"extra_info": ""
61+
}
62+
],
63+
"information": [
64+
"Testing Individual Branch Integrity",
65+
"All Branches passed integrity testing",
66+
"Testing Individual Supply Air Path Integrity",
67+
"All Supply Air Paths passed integrity testing",
68+
"Testing Individual Return Air Path Integrity",
69+
"All Return Air Paths passed integrity testing",
70+
"No node connection errors were found.",
71+
"Beginning Simulation",
72+
"EnergyPlus Completed Successfully"
73+
]
74+
}
75+
```
76+
77+
We'll need to keep in mind how new errors can be introduced into the category system to future proof it.
78+
Many tools do something like: A001, A002, B001, B002, so I would probably start by doing something similar.
79+
80+
Things I still need to solve:
81+
- Should I include timestamp info for every single warning?
82+
- Add structure for recurring
83+
- Add extra flag for whether warmup, sizing, or kickoff were true
84+
85+
## Current questions for dev team
86+
87+
- What should we name this JSON based error file?
88+
- Do we just keep a list of message string templates in a single dedicated file, and use these in calls to errors?
89+
- Anyone have strong feelings on the ID or JSON form?

‎src/EnergyPlus/Boilers.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ void GetBoilerInput(EnergyPlusData &state)
225225
ShowSevereError(
226226
state, fmt::format("{}{}=\"{}\",", RoutineName, state.dataIPShortCut->cCurrentModuleObject, state.dataIPShortCut->cAlphaArgs(1)));
227227
ShowContinueError(state, format("Invalid {}={:.3R}", state.dataIPShortCut->cNumericFieldNames(2), state.dataIPShortCut->rNumericArgs(2)));
228-
ShowSevereError(state, format("...{} must be greater than 0.0", state.dataIPShortCut->cNumericFieldNames(2)));
228+
ShowContinueError(state, format("...{} must be greater than 0.0", state.dataIPShortCut->cNumericFieldNames(2)));
229229
ErrorsFound = true;
230230
} else if (state.dataIPShortCut->rNumericArgs(2) > 1.0) {
231231
ShowWarningError(state,
@@ -288,7 +288,7 @@ void GetBoilerInput(EnergyPlusData &state)
288288
ShowSevereError(state,
289289
fmt::format("{}{}=\"{}\"", RoutineName, state.dataIPShortCut->cCurrentModuleObject, state.dataIPShortCut->cAlphaArgs(1)));
290290
ShowContinueError(state, format("Invalid {}={}", state.dataIPShortCut->cAlphaFieldNames(4), state.dataIPShortCut->cAlphaArgs(4)));
291-
ShowSevereError(state, format("...{} not found.", state.dataIPShortCut->cAlphaFieldNames(4)));
291+
ShowContinueError(state, format("...{} not found.", state.dataIPShortCut->cAlphaFieldNames(4)));
292292
ErrorsFound = true;
293293
}
294294
thisBoiler.VolFlowRate = state.dataIPShortCut->rNumericArgs(3);

‎src/EnergyPlus/ChillerExhaustAbsorption.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1362,7 +1362,7 @@ void ExhaustAbsorberSpecs::size(EnergyPlusData &state)
13621362
if (this->CondVolFlowRateWasAutoSized) {
13631363
if (state.dataPlnt->PlantFirstSizesOkayToFinalize) {
13641364
ShowSevereError(state, format("SizeExhaustAbsorber: ChillerHeater:Absorption:DoubleEffect=\"{}\", autosize error.", this->Name));
1365-
ShowSevereError(state, "Autosizing of Exhaust Fired Absorption Chiller condenser flow rate requires a condenser");
1365+
ShowContinueError(state, "Autosizing of Exhaust Fired Absorption Chiller condenser flow rate requires a condenser");
13661366
ShowContinueError(state, "loop Sizing:Plant object.");
13671367
ErrorsFound = true;
13681368
}

‎src/EnergyPlus/DElightManagerF.cc

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -604,34 +604,34 @@ namespace DElightManagerF {
604604

605605
// Validate that Reference Point coordinates are within the host Zone
606606
if (RefPt_WCS_Coord.x < thisZone.MinimumX || RefPt_WCS_Coord.x > thisZone.MaximumX) {
607-
ShowWarningError(
608-
state, format("DElightInputGenerator:Reference point X Value outside Zone Min/Max X, Zone={}", zn.Name));
609607
ShowSevereError(state,
610-
format("...X Reference Point= {:.2R}, Zone Minimum X= {:.2R}, Zone Maximum X= {:.2R}",
611-
thisZone.MinimumX,
612-
RefPt_WCS_Coord.x,
613-
thisZone.MaximumX));
608+
format("DElightInputGenerator:Reference point X Value outside Zone Min/Max X, Zone={}", zn.Name));
609+
ShowContinueError(state,
610+
format("...X Reference Point= {:.2R}, Zone Minimum X= {:.2R}, Zone Maximum X= {:.2R}",
611+
thisZone.MinimumX,
612+
RefPt_WCS_Coord.x,
613+
thisZone.MaximumX));
614614
ErrorsFound = true;
615615
}
616616
if (RefPt_WCS_Coord.y < thisZone.MinimumY || RefPt_WCS_Coord.y > thisZone.MaximumY) {
617-
ShowWarningError(
618-
state, format("DElightInputGenerator:Reference point Y Value outside Zone Min/Max Y, Zone={}", zn.Name));
619617
ShowSevereError(state,
620-
format("...Y Reference Point= {:.2R}, Zone Minimum Y= {:.2R}, Zone Maximum Y= {:.2R}",
621-
thisZone.MinimumY,
622-
RefPt_WCS_Coord.y,
623-
thisZone.MaximumY));
618+
format("DElightInputGenerator:Reference point Y Value outside Zone Min/Max Y, Zone={}", zn.Name));
619+
ShowContinueError(state,
620+
format("...Y Reference Point= {:.2R}, Zone Minimum Y= {:.2R}, Zone Maximum Y= {:.2R}",
621+
thisZone.MinimumY,
622+
RefPt_WCS_Coord.y,
623+
thisZone.MaximumY));
624624
ErrorsFound = true;
625625
}
626626
if (RefPt_WCS_Coord.z < state.dataHeatBal->Zone(izone).MinimumZ || RefPt_WCS_Coord.z > thisZone.MaximumZ) {
627-
ShowWarningError(
627+
ShowSevereError(
628628
state,
629629
format("DElightInputGenerator:Reference point Z Value outside Zone Min/Max Z, Zone={}", thisZone.Name));
630-
ShowSevereError(state,
631-
format("...Z Reference Point= {:.2R}, Zone Minimum Z= {:.2R}, Zone Maximum Z= {:.2R}",
632-
thisZone.MinimumZ,
633-
RefPt_WCS_Coord.z,
634-
thisZone.MaximumZ));
630+
ShowContinueError(state,
631+
format("...Z Reference Point= {:.2R}, Zone Minimum Z= {:.2R}, Zone Maximum Z= {:.2R}",
632+
thisZone.MinimumZ,
633+
RefPt_WCS_Coord.z,
634+
thisZone.MaximumZ));
635635
ErrorsFound = true;
636636
}
637637

‎src/EnergyPlus/DataSurfaceLists.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,9 @@ void GetSurfaceListsInputs(EnergyPlusData &state)
224224
cNumericFields.deallocate();
225225
lNumericBlanks.deallocate();
226226

227-
if (ErrorsFound) ShowSevereError(state, format("{}{}", CurrentModuleObject1, " errors found getting input. Program will terminate."));
227+
if (ErrorsFound) {
228+
ShowSevereError(state, format("{}{}", CurrentModuleObject1, " errors found getting input. Program will terminate."));
229+
}
228230
}
229231

230232
if (NumOfSurfListVentSlab > 0) {
@@ -329,10 +331,14 @@ void GetSurfaceListsInputs(EnergyPlusData &state)
329331
cNumericFields.deallocate();
330332
lNumericBlanks.deallocate();
331333

332-
if (ErrorsFound) ShowSevereError(state, format("{}{}", CurrentModuleObject2, " errors found getting input. Program will terminate."));
334+
if (ErrorsFound) {
335+
ShowSevereError(state, format("{}{}", CurrentModuleObject2, " errors found getting input. Program will terminate."));
336+
}
333337
}
334338

335-
if (ErrorsFound) ShowFatalError(state, "GetSurfaceListsInputs: Program terminates due to preceding conditions.");
339+
if (ErrorsFound) {
340+
ShowFatalError(state, "GetSurfaceListsInputs: Program terminates due to preceding conditions.");
341+
}
336342
}
337343

338344
int GetNumberOfSurfaceLists(EnergyPlusData &state)

‎src/EnergyPlus/DesiccantDehumidifiers.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -774,8 +774,8 @@ namespace DesiccantDehumidifiers {
774774
DataLoopNode::ObjectIsNotParent);
775775

776776
if (desicDehum.ControlNodeNum == 0) {
777-
ShowContinueError(state, format("{} = \"{}\"", desicDehum.DehumType, desicDehum.Name));
778-
ShowSevereError(state, format("{} must be specified.", cAlphaFields(5)));
777+
ShowSevereError(state, format("{} = \"{}\"", desicDehum.DehumType, desicDehum.Name));
778+
ShowContinueError(state, format("{} must be specified.", cAlphaFields(5)));
779779
ErrorsFoundGeneric = true;
780780
}
781781

‎src/EnergyPlus/FuelCellElectricGenerator.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ namespace FuelCellElectricGenerator {
448448
DataGenerators::AirSupRateMode::QuadraticFuncofPel)) {
449449
ShowSevereError(state, format("Invalid, {} = {}", state.dataIPShortCut->cAlphaFieldNames(5), AlphArray(5)));
450450
ShowContinueError(state, format("Entered in {}={}", state.dataIPShortCut->cCurrentModuleObject, AlphArray(1)));
451-
ShowSevereError(state, "Curve name was not found");
451+
ShowContinueError(state, "Curve name was not found");
452452
ErrorsFound = true;
453453
}
454454

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

‎src/EnergyPlus/Furnaces.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -937,15 +937,15 @@ namespace Furnaces {
937937
}
938938
if (!AirNodeFound) {
939939
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
940-
ShowSevereError(state, "Did not find Air Node (Zone with Thermostat).");
940+
ShowContinueError(state, "Did not find Air Node (Zone with Thermostat).");
941941
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(6), Alphas(6)));
942942
ShowContinueError(
943943
state, "Both a ZoneHVAC:EquipmentConnections object and a ZoneControl:Thermostat object must be specified for this zone.");
944944
ErrorsFound = true;
945945
}
946946
if (!AirLoopFound) {
947947
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
948-
ShowSevereError(state, "Did not find correct Primary Air Loop.");
948+
ShowContinueError(state, "Did not find correct Primary Air Loop.");
949949
ShowContinueError(state, format("Specified {} = {} is not served by this AirLoopHVAC equipment.", cAlphaFields(6), Alphas(6)));
950950
ErrorsFound = true;
951951
}
@@ -1478,7 +1478,7 @@ namespace Furnaces {
14781478
}
14791479
if (!AirLoopFound) {
14801480
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
1481-
ShowSevereError(state, "Did not find correct AirLoopHVAC.");
1481+
ShowContinueError(state, "Did not find correct AirLoopHVAC.");
14821482
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(6), Alphas(6)));
14831483
ErrorsFound = true;
14841484
}
@@ -2759,7 +2759,7 @@ namespace Furnaces {
27592759
}
27602760
if (!AirLoopFound) {
27612761
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
2762-
ShowSevereError(state, "Did not find correct AirLoopHVAC.");
2762+
ShowContinueError(state, "Did not find correct AirLoopHVAC.");
27632763
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(5), Alphas(5)));
27642764
ErrorsFound = true;
27652765
}
@@ -3680,7 +3680,7 @@ namespace Furnaces {
36803680
}
36813681
if (!AirLoopFound) {
36823682
ShowSevereError(state, format("{} = {}", CurrentModuleObject, Alphas(1)));
3683-
ShowSevereError(state, "Did not find correct AirLoopHVAC.");
3683+
ShowContinueError(state, "Did not find correct AirLoopHVAC.");
36843684
ShowContinueError(state, format("Specified {} = {}", cAlphaFields(5), Alphas(5)));
36853685
ErrorsFound = true;
36863686
}

‎src/EnergyPlus/HVACControllers.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -481,17 +481,18 @@ void GetControllerInput(EnergyPlusData &state)
481481
controllerProps.ControlVar = static_cast<EnergyPlus::HVACControllers::CtrlVarType>(getEnumValue(ctrlVarNamesUC, AlphArray(2)));
482482
if (controllerProps.ControlVar == HVACControllers::CtrlVarType::Invalid) {
483483
ShowSevereError(state, format("{}{}=\"{}\".", RoutineName, CurrentModuleObject, AlphArray(1)));
484-
ShowSevereError(state,
485-
format("...Invalid {}=\"{}\", must be Temperature, HumidityRatio, or TemperatureAndHumidityRatio.",
486-
cAlphaFields(2),
487-
AlphArray(2)));
484+
ShowContinueError(state,
485+
format("...Invalid {}=\"{}\", must be Temperature, HumidityRatio, or TemperatureAndHumidityRatio.",
486+
cAlphaFields(2),
487+
AlphArray(2)));
488488
ErrorsFound = true;
489489
}
490490

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

‎src/EnergyPlus/HeatBalanceManager.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1622,7 +1622,7 @@ namespace HeatBalanceManager {
16221622
ShowSevereError(
16231623
state,
16241624
format("Invalid material layer type in window {} = {}", state.dataHeatBalMgr->CurrentModuleObject, thisConstruct.Name));
1625-
ShowSevereError(
1625+
ShowContinueError(
16261626
state,
16271627
format("Equivalent Layer material type = {} is allowed only in Construction:WindowEquivalentLayer window object.",
16281628
ConstructAlphas(Layer)));

0 commit comments

Comments
 (0)