Skip to content

Conversation

@naoyam
Copy link
Collaborator

@naoyam naoyam commented Nov 21, 2025

Confirmed code diff results are benign

@naoyam
Copy link
Collaborator Author

naoyam commented Nov 21, 2025

!test --diff

@github-actions
Copy link

github-actions bot commented Nov 21, 2025

Description

  • Enable TensorIndexer functionality for reduction-related tests by setting IdModel option to "all"

  • Convert test classes from simple type aliases to proper classes with SetUp() methods

  • Apply consistent TensorIndexer enabling pattern across 6 test files

  • Maintain existing test logic while adding TensorIndexer support

Changes walkthrough

Relevant files
Enhancement
test_outer_reduction.cpp
Enable TensorIndexer for outer reduction tests                     

tests/cpp/test_outer_reduction.cpp

  • Convert OuterReductionTest from type alias to class inheritance
  • Add SetUp() method enabling TensorIndexer with IdModel option
  • Maintain existing test functionality while enabling TensorIndexer
  • +7/-1     
    test_persistent_buffer.cpp
    Enable TensorIndexer for persistent buffer tests                 

    tests/cpp/test_persistent_buffer.cpp

  • Convert PersistentBufferTest from type alias to class inheritance
  • Add SetUp() method enabling TensorIndexer with IdModel option
  • Maintain existing test functionality while enabling TensorIndexer
  • +8/-1     
    test_reduction.cpp
    Enable TensorIndexer for reduction tests                                 

    tests/cpp/test_reduction.cpp

  • Convert ReductionTest from type alias to class inheritance
  • Add SetUp() method enabling TensorIndexer with IdModel option
  • Maintain existing test functionality while enabling TensorIndexer
  • +7/-1     
    test_reduction_pointwise.cpp
    Enable TensorIndexer for pointwise reduction tests             

    tests/cpp/test_reduction_pointwise.cpp

  • Convert PointwiseFusedReductionTest from type alias to class
    inheritance
  • Add SetUp() method enabling TensorIndexer with IdModel option
  • Maintain existing test functionality while enabling TensorIndexer
  • +7/-1     
    test_serial_gridreduce.cpp
    Enable TensorIndexer for serial grid reduction tests         

    tests/cpp/test_serial_gridreduce.cpp

  • Convert SerialGridReductionTest from type alias to class inheritance
  • Add SetUp() method enabling TensorIndexer with IdModel option
  • Maintain existing test functionality while enabling TensorIndexer
  • +7/-1     
    test_welford.cpp
    Enable TensorIndexer for Welford tests                                     

    tests/cpp/test_welford.cpp

  • Convert WelfordTest from type alias to class inheritance
  • Add SetUp() method enabling TensorIndexer with IdModel option
  • Maintain existing test functionality while enabling TensorIndexer
  • +7/-1     

    PR Reviewer Guide

    Here are some key observations to aid the review process:

    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Test failures

    • (Medium, 1) Tensor numerical mismatch in nvFuser matmul tests on Hopper (H100)

      Test Name 20 Source
      HopperMatmulTest.HSH_NT_UseScheduler_MultipleInstructionsPerWarpTile Link

    @naoyam naoyam marked this pull request as ready for review November 21, 2025 07:23
    @naoyam naoyam requested a review from jjsjann123 November 21, 2025 07:24
    @greptile-apps
    Copy link
    Contributor

    greptile-apps bot commented Nov 21, 2025

    Greptile Overview

    Greptile Summary

    Enables TensorIndexer (via IdModel) for six reduction-related test suites by converting test fixture type aliases to proper classes with SetUp() methods.

    Key Changes:

    Confidence Score: 5/5

    • This PR is safe to merge with minimal risk
    • The changes are mechanical refactorings that follow an established pattern from previous PRs. Each test fixture is converted identically, properly calling parent SetUp() before enabling IdModel. No test logic is modified, only the initialization mechanism changes.
    • No files require special attention

    Important Files Changed

    File Analysis

    Filename Score Overview
    tests/cpp/test_outer_reduction.cpp 5/5 Converted test fixture from type alias to class with SetUp() override to enable IdModel for all tests
    tests/cpp/test_persistent_buffer.cpp 5/5 Converted test fixture from type alias to class with SetUp() override to enable IdModel for all tests
    tests/cpp/test_reduction.cpp 5/5 Converted test fixture from type alias to class with SetUp() override to enable IdModel for all tests
    tests/cpp/test_reduction_pointwise.cpp 5/5 Converted test fixture from type alias to class with SetUp() override to enable IdModel for all tests
    tests/cpp/test_serial_gridreduce.cpp 5/5 Converted test fixture from type alias to class with SetUp() override to enable IdModel for all tests
    tests/cpp/test_welford.cpp 5/5 Converted test fixture from type alias to class with SetUp() override to enable IdModel for all tests

    Sequence Diagram

    sequenceDiagram
        participant TestRunner as Test Runner
        participant TestFixture as ReductionTest/WelfordTest/etc
        participant NVFuserTest as NVFuserTest Base
        participant EnableOptionsGuard as EnableOptionsGuard
        participant IdModel as IdModel System
    
        TestRunner->>TestFixture: Create test instance
        TestRunner->>TestFixture: Call SetUp()
        TestFixture->>NVFuserTest: Call parent SetUp()
        NVFuserTest-->>TestFixture: Base initialization complete
        TestFixture->>EnableOptionsGuard: getCurOptions().set(IdModel, {"all"})
        EnableOptionsGuard->>IdModel: Enable IdModel for indexing
        IdModel-->>EnableOptionsGuard: IdModel enabled
        EnableOptionsGuard-->>TestFixture: Configuration set
        TestFixture-->>TestRunner: Setup complete
        TestRunner->>TestFixture: Execute test case
        Note over TestFixture,IdModel: All tests run with IdModel enabled
    
    Loading

    Copy link
    Contributor

    @greptile-apps greptile-apps bot left a comment

    Choose a reason for hiding this comment

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

    6 files reviewed, no comments

    Edit Code Review Agent Settings | Greptile

    @naoyam naoyam merged commit df9c33b into main Nov 21, 2025
    68 of 69 checks passed
    @naoyam naoyam deleted the tensorindexer_enable_reduction branch November 21, 2025 17:58
    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.

    3 participants