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 valgrind errors in eamxx_homme_iop.cpp #2859

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

jgfouca
Copy link
Member

@jgfouca jgfouca commented Jun 4, 2024

We were running off the end of a couple views.

Example from my debugging printing:
JGF lev=79 hyam.size=72, hybm.size=72

We were running off the end of a couple views.
Mask nudge_level;
for (int p=0; p<Pack::n; ++p) {
const auto lev = k*Pack::n + p;
Mask nudge_level(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

I assume I should default this mask to all false?

@E3SM-Bot
Copy link
Collaborator

E3SM-Bot commented Jun 4, 2024

Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects:

Pull Request Auto Testing STARTING (click to expand)

Build Information

Test Name: SCREAM_PullRequest_Autotester_Mappy

  • Build Num: 5503
  • Status: STARTED

Jenkins Parameters

Parameter Name Value
PR_LABELS AT: AUTOMERGE
PULLREQUESTNUM 2859
SCREAM_SOURCE_REPO https://github.com/E3SM-Project/scream
SCREAM_SOURCE_SHA 54ab366
SCREAM_TARGET_BRANCH master
SCREAM_TARGET_REPO https://github.com/E3SM-Project/scream
SCREAM_TARGET_SHA 9f8ce1f
TEST_REPO_ALIAS SCREAM

Build Information

Test Name: SCREAM_PullRequest_Autotester_Weaver

  • Build Num: 5779
  • Status: STARTED

Jenkins Parameters

Parameter Name Value
PR_LABELS AT: AUTOMERGE
PULLREQUESTNUM 2859
SCREAM_SOURCE_REPO https://github.com/E3SM-Project/scream
SCREAM_SOURCE_SHA 54ab366
SCREAM_TARGET_BRANCH master
SCREAM_TARGET_REPO https://github.com/E3SM-Project/scream
SCREAM_TARGET_SHA 9f8ce1f
TEST_REPO_ALIAS SCREAM

Using Repos:

Repo: SCREAM (E3SM-Project/scream)
  • Branch: jgfouca/fix_valgrind_err_homme
  • SHA: 54ab366
  • Mode: TEST_REPO

Pull Request Author: jgfouca

@jgfouca jgfouca requested a review from bogensch June 4, 2024 19:44
const auto lev = k*Pack::n + p;
Mask nudge_level(false);
int max_size = hyam.size();
for (int lev=k*Pack::n, p = 0; p < Pack::n && lev < max_size; ++lev, ++p) {
const auto pressure_from_iop = hyam(lev)*ps0 + hybm(lev)*ps_iop;
nudge_level.set(p, pressure_from_iop <= iop_nudge_tq_low*100
Copy link
Member Author

Choose a reason for hiding this comment

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

@bogensch , this looks like a bug. How can pressure ever be both low and high? This and should be an or I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a bug, just confusing names..... Valid example values: iop_nudge_tq_low = 1000 hPa; iop_nudge_tq_high=700 hPa. Thus low/high refers to the relative position in the atmosphere with respect to the surface rather than the value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Makes sense now.

@E3SM-Bot
Copy link
Collaborator

E3SM-Bot commented Jun 4, 2024

Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED

Pull Request Auto Testing has PASSED (click to expand)

Build Information

Test Name: SCREAM_PullRequest_Autotester_Mappy

  • Build Num: 5503
  • Status: PASSED

Jenkins Parameters

Parameter Name Value
PR_LABELS AT: AUTOMERGE
PULLREQUESTNUM 2859
SCREAM_SOURCE_REPO https://github.com/E3SM-Project/scream
SCREAM_SOURCE_SHA 54ab366
SCREAM_TARGET_BRANCH master
SCREAM_TARGET_REPO https://github.com/E3SM-Project/scream
SCREAM_TARGET_SHA 9f8ce1f
TEST_REPO_ALIAS SCREAM

Build Information

Test Name: SCREAM_PullRequest_Autotester_Weaver

  • Build Num: 5779
  • Status: PASSED

Jenkins Parameters

Parameter Name Value
PR_LABELS AT: AUTOMERGE
PULLREQUESTNUM 2859
SCREAM_SOURCE_REPO https://github.com/E3SM-Project/scream
SCREAM_SOURCE_SHA 54ab366
SCREAM_TARGET_BRANCH master
SCREAM_TARGET_REPO https://github.com/E3SM-Project/scream
SCREAM_TARGET_SHA 9f8ce1f
TEST_REPO_ALIAS SCREAM

@E3SM-Bot E3SM-Bot merged commit b80e1bd into master Jun 4, 2024
8 checks passed
@E3SM-Bot E3SM-Bot deleted the jgfouca/fix_valgrind_err_homme branch June 4, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants