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

DPxx latitude relative error checking fix #3111

Merged
merged 4 commits into from
Nov 20, 2024
Merged

Conversation

bogensch
Copy link
Contributor

When verifying that the user-defined latitude matches the model-expected latitude, the code block was failing for latitudes < 0 when computing the relative error. This issue occurred because the normalization value was set to the minimum allowable threshold (0.1). To address this, we now use the absolute value of latitude to calculate the relative error. This is not an issue for longitude where all values are required to be between 0 and 360, therefore no fix was applied there.

bartgol and others added 2 commits November 13, 2024 13:16
The name of skipped jobs from a matrix needed to be fixed
@bogensch bogensch added BFB Bit for bit bugfix DP-SCREAM CI: automerge WARNING: Still in an experimental phase labels Nov 13, 2024
@bogensch bogensch requested a review from tcclevenger November 13, 2024 21:45
Copy link
Contributor

mergify bot commented Nov 13, 2024

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce checks passing

Wonderful, this rule succeeded.

Make sure that checks are not failing on the PR, and reviewers approved

  • #approved-reviews-by >= 1
  • #changes-requested-reviews-by == 0
  • any of:
    • all of:
      • check-success="gcc-openmp / dbg"
      • check-success="gcc-openmp / fpe"
      • check-success="gcc-openmp / opt"
      • check-success="gcc-openmp / sp"
    • check-skipped={% raw %}gcc-openmp / ${{ matrix.build_type }}{% endraw %}
  • any of:
    • all of:
      • check-success="gcc-cuda / dbg"
      • check-success="gcc-cuda / opt"
      • check-success="gcc-cuda / sp"
    • check-skipped={% raw %}gcc-cuda / ${{ matrix.build_type }}{% endraw %}
  • any of:
    • all of:
      • check-success="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
      • check-success="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
      • check-success="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
      • check-success="cpu-gcc / SMS_D_Ln5.ne4pg2_oQU480.F2010-SCREAMv1-MPASSI.scream-mam4xx-all_mam4xx_procs"
    • check-skipped={% raw %}cpu-gcc / ${{ matrix.test.short_name }}{% endraw %}
  • any of:
    • check-skipped=cpu-gcc
    • check-success=cpu-gcc

tcclevenger
tcclevenger previously approved these changes Nov 13, 2024
@tcclevenger
Copy link
Contributor

Probably need to resolve conflicts by taking whatever is on master.

@bartgol
Copy link
Contributor

bartgol commented Nov 13, 2024

Probably need to resolve conflicts by taking whatever is on master.

Sorry, that's my fault. I was working the kinks of mergify's syntax, and you must have opened the PR right before I forced push the final config. You can just do an interactive rebase where you purge the "Mergify: ..." commit, then force push.

@AaronDonahue
Copy link
Contributor

The failing test for eamxx-sa/gcc-openmp/opt are because of the missing p3 baselines, so not related to this PR. I suspect we'll see similar fails once the other sa test suites run. We may need to retrigger testing after we fix the p3 baselines issue.

@mergify mergify bot merged commit 35c97e8 into master Nov 20, 2024
20 checks passed
@mergify mergify bot deleted the bogensch/dpxx_latfix branch November 20, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB Bit for bit bugfix CI: automerge WARNING: Still in an experimental phase DP-SCREAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants