Skip to content

skip ThunderRMSNormBwd#4455

Merged
liqiangxl merged 4 commits intomainfrom
llu/skip_ThunderRMSNormBwd
May 15, 2025
Merged

skip ThunderRMSNormBwd#4455
liqiangxl merged 4 commits intomainfrom
llu/skip_ThunderRMSNormBwd

Conversation

@liqiangxl
Copy link
Collaborator

New restriction on warp specialization was added in #4395
Needs to temporarily skip ThunderRMSNormBwd

@liqiangxl liqiangxl marked this pull request as ready for review May 15, 2025 14:06
@liqiangxl
Copy link
Collaborator Author

!test

@github-actions
Copy link

github-actions bot commented May 15, 2025

Review updated until commit 78bafc0

Description

  • Temporarily skip ThunderRMSNormBwd and LayerNormBackward tests

  • Add warp specialization check in test parameters

  • Format code for consistency


Changes walkthrough 📝

Relevant files
Enhancement
test_combined_inner_outer_reduction.cpp
Add warp specialization checks in tests                                   

tests/cpp/test_combined_inner_outer_reduction.cpp

  • Added warp specialization check in ThunderRMSNormBwd test
  • Added warp specialization check in LayerNormBackward test
  • Updated test parameters to include warp specialization flag
  • +13/-2   

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 PR contains tests
    ⚡ Recommended focus areas for review

    Code Duplication

    The same code block for checking ws_enabled and skipping the test is duplicated in two different test functions (ThunderRMSNormBwd and LayerNormBackward). This can lead to maintenance issues.

    if (ws_enabled) {
      GTEST_SKIP() << "Bdimx is dynamic, Warp Specialization is disabled.";
      return;
    }
    Test Parameter Usage

    The variable ws_enabled is used to skip the test, but its purpose and implications should be clearly documented in the test description or comments.

    if (ws_enabled) {
      GTEST_SKIP() << "Bdimx is dynamic, Warp Specialization is disabled.";
      return;
    }

    @liqiangxl
    Copy link
    Collaborator Author

    !test

    @liqiangxl
    Copy link
    Collaborator Author

    !test

    @liqiangxl liqiangxl merged commit f6c131c into main May 15, 2025
    11 of 12 checks passed
    @liqiangxl liqiangxl deleted the llu/skip_ThunderRMSNormBwd branch May 15, 2025 16:05
    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.

    2 participants