Skip to content

Enable TensorIndexer#5559

Merged
naoyam merged 8 commits intomainfrom
tensorindexer_misc_update
Nov 20, 2025
Merged

Enable TensorIndexer#5559
naoyam merged 8 commits intomainfrom
tensorindexer_misc_update

Conversation

@naoyam
Copy link
Collaborator

@naoyam naoyam commented Nov 20, 2025

Enabled TensorIndexer with some of the tests. Code diffs look benign.

@naoyam
Copy link
Collaborator Author

naoyam commented Nov 20, 2025

!test --diff

@github-actions
Copy link

github-actions bot commented Nov 20, 2025

Review updated until commit 854acd0

Description

  • Enable TensorIndexer by setting IdModel option to "all" in test SetUp methods

  • Convert test classes from simple type aliases to proper class definitions with SetUp

  • Add EnableOptionsGuard configuration to multiple test files: abstract_tensor, cluster, rope, unary, vectorization

  • Ensure TensorIndexer feature is consistently enabled across test suite

Changes walkthrough

Relevant files
Tests
test_abstract_tensor.cpp
Enable TensorIndexer in AbstractTensorTest                             

tests/cpp/test_abstract_tensor.cpp

  • Add EnableOptionsGuard configuration in SetUp method
  • Enable IdModel option with "all" value for TensorIndexer
  • +1/-0     
    test_cluster.cpp
    Enable TensorIndexer in ClusterReductionTest                         

    tests/cpp/test_cluster.cpp

  • Add EnableOptionsGuard configuration in SetUp method
  • Enable IdModel option with "all" value for TensorIndexer
  • +1/-0     
    test_rope.cpp
    Enable TensorIndexer in RopeTest                                                 

    tests/cpp/test_rope.cpp

  • Convert RopeTest from type alias to class definition
  • Add SetUp method with EnableOptionsGuard configuration
  • Enable IdModel option with "all" value for TensorIndexer
  • +6/-1     
    test_unary.cpp
    Enable TensorIndexer in UnaryTest                                               

    tests/cpp/test_unary.cpp

  • Convert UnaryTest from type alias to class definition
  • Add SetUp method with EnableOptionsGuard configuration
  • Enable IdModel option with "all" value for TensorIndexer
  • +6/-1     
    test_vectorization.cpp
    Enable TensorIndexer in VectorizationAnalysisTest               

    tests/cpp/test_vectorization.cpp

  • Convert VectorizationAnalysisTest from type alias to class definition
    with SetUp
  • Add SetUp method to VectorizationCastTest class
  • Add SetUp method to Vect256Test class
  • Enable IdModel option with "all" value for TensorIndexer in all test
    classes
  • +10/-1   

    PR Reviewer Guide

    Here are some key observations to aid the review process:

    🧪 PR contains tests
    ⚡ Recommended focus areas for review
    Test Class Definition Change

    Changed from simple type alias to proper class definition. Need to verify that the test instantiation and parameterization still work correctly with the new class structure.

    class RopeTest : public NVFuserFixtureParamTest<RopeConfig> {
      void SetUp() override {
        NVFuserFixtureParamTest<RopeConfig>::SetUp();
        EnableOptionsGuard::getCurOptions().set(EnableOption::IdModel, {"all"});
      }
    };
    Test Class Definition Change

    Similar change from type alias to proper class. Verify test parameterization and execution remain unaffected.

    class UnaryTest : public NVFuserFixtureParamTest<PrimDataType> {
      void SetUp() override {
        NVFuserFixtureParamTest<PrimDataType>::SetUp();
        EnableOptionsGuard::getCurOptions().set(EnableOption::IdModel, {"all"});
      }
    };
    Multiple Test Class Updates

    Three test classes (VectorizationAnalysisTest, VectorizationCastTest, Vect256Test) were updated. Ensure all test cases still execute properly and the IdModel option doesn't interfere with vectorization logic.

    class VectorizationAnalysisTest : public NVFuserTest {
      void SetUp() override {
        NVFuserTest::SetUp();
        EnableOptionsGuard::getCurOptions().set(EnableOption::IdModel, {"all"});
      }
    };

    @naoyam
    Copy link
    Collaborator Author

    naoyam commented Nov 20, 2025

    !test --diff

    @naoyam naoyam requested a review from jacobhinkle November 20, 2025 17:21
    @naoyam naoyam marked this pull request as ready for review November 20, 2025 17:21
    @greptile-apps
    Copy link
    Contributor

    greptile-apps bot commented Nov 20, 2025

    Greptile Overview

    Greptile Summary

    This PR enables TensorIndexer across multiple test suites by setting EnableOption::IdModel to {"all"} in test fixture SetUp() methods. TensorIndexer is part of the IdModel system that provides an alternative indexing mechanism for tensor operations.

    Key changes:

    • Converted type aliases (using) to full class definitions for test fixtures that need custom SetUp() methods
    • Added parent SetUp() calls in all overridden methods (previously missing, causing issues)
    • Enabled IdModel with "all" options to activate TensorIndexer functionality
    • Applied changes consistently across 5 test files covering abstract tensors, clustering, ROPE, unary operations, and vectorization

    The changes are straightforward test infrastructure updates with proper initialization order.

    Confidence Score: 5/5

    • Safe to merge - proper test infrastructure changes with correct initialization order
    • All parent SetUp() calls are correctly placed before IdModel enablement, following the pattern established in existing test fixtures like HopperBase, BlackwellBase, and TmaBase. The changes are limited to test code only and consistently apply the same pattern across all modified files.
    • No files require special attention

    Important Files Changed

    File Analysis

    Filename Score Overview
    tests/cpp/test_abstract_tensor.cpp 5/5 Adds IdModel enablement to AbstractTensorTest SetUp, properly calls parent SetUp first
    tests/cpp/test_cluster.cpp 5/5 Adds IdModel enablement to ClusterReductionTest SetUp, properly calls parent SetUp first
    tests/cpp/test_rope.cpp 5/5 Converts RopeTest to class with SetUp override, adds parent SetUp call and IdModel enablement
    tests/cpp/test_unary.cpp 5/5 Converts UnaryTest to class with SetUp override, adds parent SetUp call and IdModel enablement
    tests/cpp/test_vectorization.cpp 5/5 Converts three test fixtures to classes with SetUp overrides, adds parent SetUp calls and IdModel enablement

    Sequence Diagram

    sequenceDiagram
        participant Test as Test Framework
        participant TestFixture as Test Fixture Class
        participant Parent as Parent Test Class
        participant EnableGuard as EnableOptionsGuard
        participant IdModel as IdModel/TensorIndexer
        
        Test->>TestFixture: SetUp()
        TestFixture->>Parent: Parent::SetUp()
        Note over Parent: Initialize test environment<br/>(CUDA arch checks, guards)
        Parent-->>TestFixture: Return
        TestFixture->>EnableGuard: getCurOptions().set()
        EnableGuard->>IdModel: Enable IdModel {"all"}
        Note over IdModel: Activates TensorIndexer<br/>for consumer/producer indexing,<br/>predicates, and loop generation
        IdModel-->>EnableGuard: Enabled
        EnableGuard-->>TestFixture: Configuration applied
        TestFixture-->>Test: Ready for test execution
        Test->>TestFixture: Run test cases
        Note over TestFixture,IdModel: Tests execute with<br/>TensorIndexer 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.

    5 files reviewed, 3 comments

    Edit Code Review Agent Settings | Greptile

    naoyam and others added 3 commits November 20, 2025 09:39
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    @naoyam
    Copy link
    Collaborator Author

    naoyam commented Nov 20, 2025

    !test --diff

    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.

    5 files reviewed, 2 comments

    Edit Code Review Agent Settings | Greptile

    naoyam and others added 3 commits November 20, 2025 15:28
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    @naoyam
    Copy link
    Collaborator Author

    naoyam commented Nov 20, 2025

    !test --diff

    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.

    5 files reviewed, no comments

    Edit Code Review Agent Settings | Greptile

    Copy link
    Collaborator

    @jjsjann123 jjsjann123 left a comment

    Choose a reason for hiding this comment

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

    :shipit:

    @naoyam naoyam merged commit 1ffe2a9 into main Nov 20, 2025
    20 of 21 checks passed
    @naoyam naoyam deleted the tensorindexer_misc_update branch November 20, 2025 23:50
    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