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

Fixup small kernel situation #3110

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Fixup small kernel situation #3110

merged 1 commit into from
Nov 14, 2024

Conversation

jgfouca
Copy link
Member

@jgfouca jgfouca commented Nov 13, 2024

It was a bit surprising that only the p3_main test actually did something different with small kernels turned on. After reviewing the code, I think I understand why. All the _disp functions are just thin wrappers around the corresponding p3 function. I don't think it's really worth the effort to have separate BFB unit tests for all the _disp functions. The p3_main test (which does test the small kernel stuff) should be adequate.

I've added some comments and change the p3_sk test to only test p3_main since any other bfb testing would be redundant.

@jgfouca jgfouca added the CI: automerge WARNING: Still in an experimental phase label Nov 13, 2024
@jgfouca jgfouca requested a review from bartgol November 13, 2024 20:01
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

Waiting checks: gcc-cuda / ${{ matrix.build_type }}, gcc-cuda / dbg, gcc-cuda / opt, gcc-cuda / sp, gcc-openmp / ${{ matrix.build_type }}, gcc-openmp / dbg, gcc-openmp / opt, gcc-openmp / sp.

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

  • any of:
    • check-skipped={% raw %}gcc-openmp / ${{ matrix.build_type }}{% endraw %}
    • all of:
      • check-success="gcc-openmp / dbg"
      • check-success="gcc-openmp / opt"
      • check-success="gcc-openmp / sp"
      • check-success="gcc-openmp / fpe"
  • any of:
    • check-skipped={% raw %}gcc-cuda / ${{ matrix.build_type }}{% endraw %}
    • all of:
      • check-success="gcc-cuda / dbg"
      • check-success="gcc-cuda / opt"
      • check-success="gcc-cuda / sp"
  • #approved-reviews-by >= 1
  • #changes-requested-reviews-by == 0
  • 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

@bartgol
Copy link
Contributor

bartgol commented Nov 14, 2024

I'm going to merge this, since testing will continue to fail until baselines are regenerated

@bartgol bartgol merged commit 224e637 into master Nov 14, 2024
12 of 19 checks passed
@bartgol bartgol deleted the jgfouca/fix_small_kernels branch November 14, 2024 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: automerge WARNING: Still in an experimental phase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants