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

Fix Meter:Custom with mix of valid and invalid names #10773

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

amirroth
Copy link
Collaborator

PR #10384 changed the behavior of Meter:Custom so that meter creation fails if one of the component variables or meters is missing. According to issue #10771, the desired behavior is to create the custom meter but ignore the missing component variable or meter. This behavior was restored.

@amirroth amirroth self-assigned this Sep 27, 2024
@amirroth amirroth added the Defect Includes code to repair a defect in EnergyPlus label Sep 27, 2024
@amirroth amirroth added this to the EnergyPlus 24.2 milestone Sep 27, 2024
@amirroth amirroth requested a review from Myoldmopar September 27, 2024 14:45
@amirroth amirroth assigned Myoldmopar and unassigned amirroth Sep 27, 2024
@@ -803,7 +803,7 @@ namespace OutputProcessor {

// Has to be a summed variable
if (srcDDVar->storeType != StoreType::Sum) {
ShowWarningCustomMessage(state,
ShowWarningCustomMessage(state, // Is clang-format formatting things like this? This is gross.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seriously, clang-format is not worth it if this is the result.

@@ -867,8 +867,7 @@ namespace OutputProcessor {
ipsc->cAlphaFieldNames(fldIndex + 1),
ipsc->cAlphaArgs(fldIndex + 1)));
ShowContinueError(state, "...will not be shown with the Meter results.");
foundBadSrc = true;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One line fix. A missing component variable/meter no longer sets an error condition that prevents the custom meter from being created.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me!

@mjwitte mjwitte changed the title Attempted Fix for Issue #10771 Fix Meter:Custom with mix of valid and invalid names Sep 27, 2024
Copy link
Contributor

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

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

Tested with the defect file and with mismatched units. Works as advertised.
Thanks @amirroth and @eringold!

@amirroth
Copy link
Collaborator Author

@mjwitte, you've already fixed one of my bugs. You shouldn't have to fix all of them.

@@ -867,8 +867,7 @@ namespace OutputProcessor {
ipsc->cAlphaFieldNames(fldIndex + 1),
ipsc->cAlphaArgs(fldIndex + 1)));
ShowContinueError(state, "...will not be shown with the Meter results.");
foundBadSrc = true;
Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me!

@Myoldmopar
Copy link
Member

Test fine with latest develop pulled in, going in. Thanks @amirroth

@Myoldmopar Myoldmopar merged commit b422061 into develop Sep 30, 2024
10 of 11 checks passed
@Myoldmopar Myoldmopar deleted the CustomMeterFix branch September 30, 2024 14:29
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.

'Invalid Output Variable or Meter Name' in Meter:Custom results in no meter output
7 participants