Skip to content

Defect - Fix 11185 incorrect severe tariff error#11201

Merged
mitchute merged 8 commits intoNatLabRockies:developfrom
bigladder:Fix-11185-Incorrect-Severe-Tariff-Error
Oct 9, 2025
Merged

Defect - Fix 11185 incorrect severe tariff error#11201
mitchute merged 8 commits intoNatLabRockies:developfrom
bigladder:Fix-11185-Incorrect-Severe-Tariff-Error

Conversation

@GaryMarksBigladder
Copy link
Contributor

@GaryMarksBigladder GaryMarksBigladder commented Sep 8, 2025

Pull request overview

Description of the purpose of this PR

Missing item severe tariff error doesn't display the missing item and instead displays some default information. The fix adds the item that's missing to the severe error.

Pull Request Author

  • [ X] Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • [ X] Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • [ X] Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@GaryMarksBigladder GaryMarksBigladder marked this pull request as ready for review September 8, 2025 19:20
@tanaya-mankad tanaya-mankad added the Defect Includes code to repair a defect in EnergyPlus label Sep 8, 2025
@JasonGlazer
Copy link
Contributor

The test files provided recreated the original error in develop and work properly in the branch with the revised code.

@JasonGlazer
Copy link
Contributor

@GaryMarksBigladder, is this a specific case of a general problem with using ShowSevereItemNotFound() that has now just been fixed for this specific case? The ShowSevereItemNotFound() is widely used. I don't think we want a little bandaid over a big problem.

@GaryMarksBigladder
Copy link
Contributor Author

To the best of my knowledge, ShowSevereItemNotFound is working for other parts of the code. I noticed that just fixing the usage of ShowSevereItemNotFound, in this case, would still leave the user searching for information from warnings higher up instead of just giving them all the information they needed to fix their idf in the severe error message itself. That's why I made a new function that other code can call if they feel it makes sense in their usage. We could change ShowSevereItemNotFound for everyone, but I don't know if there is code out there that is programmatically looking for specific verbiage in the error, so it seemed safer to me to change it for this specific use case while allowing others in the future to decide which severe error message best fits their use case. Please let me know if that was not the correct course of action.

@JasonGlazer
Copy link
Contributor

Thanks @GaryMarksBigladder

void ShowDetailedSevereItemNotFound(EnergyPlusData& state, ErrorObjectHeader const& eoh, std::string_view fieldName, std::string_view fieldVal)
{
ShowSevereError(state, format("{}: {} = {}, item not found.", eoh.routineName, fieldName, fieldVal));
ShowContinueError(state, format("{} = {}, item not found.", fieldName, fieldVal));
Copy link
Member

Choose a reason for hiding this comment

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

I'll be honest - I'm not sure I understand. This is repeating the same line twice? Except in the first it just shows the routine name? That seems odd to me. Maybe I'm missing something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ShowSevereItemNotFound:

ShowSevereError(state, format("{}: {} = {}", eoh.routineName, eoh.objectType, eoh.objectName));
ShowContinueError(state, format("{} = {}, item not found.", fieldName, fieldVal));

@nealkruis
Copy link
Member

@Myoldmopar any idea what's happening with the regression error in the CI? It doesn't look like the regression files were uploaded.

@JasonGlazer
Copy link
Contributor

JasonGlazer commented Sep 19, 2025

The formatting fix (from the static code analysis) can be downloaded from the patch file and shows:

diff --git a/src/EnergyPlus/UtilityRoutines.cc b/src/EnergyPlus/UtilityRoutines.cc
index bf2f0fb..772467e 100644
--- a/src/EnergyPlus/UtilityRoutines.cc
+++ b/src/EnergyPlus/UtilityRoutines.cc
@@ -1682,7 +1682,7 @@ void ShowSevereItemNotFound(EnergyPlusData &state, ErrorObjectHeader const &eoh,
     ShowContinueError(state, format("{} = {}, item not found.", fieldName, fieldVal));
 }
 
-void ShowDetailedSevereItemNotFound(EnergyPlusData& state, ErrorObjectHeader const& eoh, std::string_view fieldName, std::string_view fieldVal)
+void ShowDetailedSevereItemNotFound(EnergyPlusData &state, ErrorObjectHeader const &eoh, std::string_view fieldName, std::string_view fieldVal)
 {
     ShowSevereError(state, format("{}: {} = {}, item not found.", eoh.routineName, fieldName, fieldVal));
     ShowContinueError(state, format("{} = {}, item not found.", fieldName, fieldVal));

A minor quick fix. I'm not sure I understand the other issues being reported on the Build and Test related to a forked repo. When I google that it seems to be pointing to the fact that the pull request is coming from the bigladder repo and maybe there is a permission issue.

@tanaya-mankad
Copy link
Collaborator

Formatting fixed!

@mitchute mitchute self-requested a review October 8, 2025 18:24
Copy link
Collaborator

@mitchute mitchute left a comment

Choose a reason for hiding this comment

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

GTG. Merging.

Comment on lines +290 to +298
/// <summary>
/// Similar to ShowSevereItemNotFound, except for the severe error added to the list contains the information about
/// what item is missing.
/// </summary>
/// <param name="state">EnergyPus state, used for tracking the severe error</param>
/// <param name="eoh">Error object header, used to get the routineName</param>
/// <param name="fieldName">Name of the unmatched field</param>
/// <param name="fieldValue">Value of the unmatched field</param>
void ShowDetailedSevereItemNotFound(EnergyPlusData &state, ErrorObjectHeader const &eoh, std::string_view fieldName, std::string_view fieldValue);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I could see this being pretty useful if we had it propagated throughout the code, but I'm having trouble seeing where this would kick in, and IDD/IDF file validation would end. This is fine to go in... I'm just thinking out loud..

@mitchute mitchute merged commit dbaa37f into NatLabRockies:develop Oct 9, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UtilityRate:Tariff schedule name mismatch produces incorrect severe error message

9 participants