Skip to content

Conversation

@wujingyue
Copy link
Collaborator

Summary

  • add debug logging hooks for propagation paths
  • adjust related scheduler/propagation utilities

Testing

  • not run

@github-actions
Copy link

github-actions bot commented Jan 29, 2026

Review updated until commit 080fdea

Description

  • Add debug logging for sharding propagation operations

  • Implement PropagateDirection enum streaming operator

  • Enable TransformPropagator debug output in shardLoopLike

  • Reorganize SPECIALIZE_PRINTER macros in base.h

  • Remove unused multidevice/utils.h include

Changes walkthrough

Relevant files
Enhancement
propagation.cpp
Add debug logging to shardLoopLike function                           

csrc/multidevice/propagation.cpp

  • Added base.h include for debug utilities
  • Added debug logging in shardLoopLike function to log propagation
    details
  • Logs ref/target tensors, direction, and selected parallel types when
    debug enabled
  • +7/-0     
    utils.cpp
    Implement PropagateDirection streaming operator                   

    csrc/scheduler/utils.cpp

  • Added std::ostream& operator<< overload for PropagateDirection enum
  • Enables string representation of propagation direction for debugging
  • +16/-0   
    utils.h
    Add PropagateDirection operator declaration                           

    csrc/scheduler/utils.h

  • Added forward declaration of operator<< for PropagateDirection
  • Enables proper compilation of streaming operator in utils.cpp
  • +2/-0     
    Cleanup
    propagate_shardings.cpp
    Remove unused include directive                                                   

    csrc/preseg_passes/propagate_shardings.cpp

    • Removed unused #include "multidevice/utils.h"
    +0/-1     
    Formatting
    base.h
    Reorganize SPECIALIZE_PRINTER macros                                         

    csrc/base.h

  • Reorganized SPECIALIZE_PRINTER macro declarations
  • Added std::vector and std::vector specializations
  • Improved alphabetical ordering of printer specializations
  • +13/-11 

    PR Reviewer Guide

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ Recommended focus areas for review
    Debug Logging Implementation

    The debug logging in shardLoopLike function uses isDebugDumpEnabled and debug() calls. Need to verify that the debug stream and debug dump option are properly initialized and that this won't cause issues in production builds where debug logging might be disabled.

    if (isDebugDumpEnabled(DebugDumpOption::TransformPropagator)) {
      debug() << "Propagating shardings from " << ref->toString() << " to "
              << target->toString() << " in " << direction << " for "
              << toDelimitedString(selected_parallel_types) << std::endl;
    }
    Missing Include Dependencies

    The operator<< implementation for PropagateDirection is added but it's unclear if the necessary headers for std::ostream are included. Also need to verify this doesn't conflict with other stream operators in the codebase.

    std::ostream& operator<<(std::ostream& os, PropagateDirection direction) {
      switch (direction) {
        case PropagateDirection::kForward:
          os << "Forward";
          break;
        case PropagateDirection::kBackward:
          os << "Backward";
          break;
      }
      return os;
    }
    Removed Include Impact

    The removal of "#include multidevice/utils.h" should be verified to ensure no functionality is broken. Need to check if any symbols from that header are still being used in this file after the removal.

    // clang-format off
    /*
     * SPDX-FileCopyrightText: Copyright (c) 2024-present NVIDIA CORPORATION & AFFILIATES.
     * All rights reserved.
     * SPDX-License-Identifier: BSD-3-Clause
     */
    // clang-format on
    #include "preseg_passes/propagate_shardings.h"
    
    #include <vector>
    
    #include "ir/interface_nodes.h"
    #include "ir/iostream.h"
    #include "ir/utils.h"
    #include "multidevice/propagation.h"
    #include "scheduler/utils.h"
    
    namespace nvfuser::preseg_passes {
    
    namespace {

    @greptile-apps
    Copy link
    Contributor

    greptile-apps bot commented Jan 29, 2026

    Greptile Overview

    Greptile Summary

    This PR adds comprehensive debug logging infrastructure for propagation paths in the multidevice sharding system. The changes enable developers to trace how shardings propagate between tensor views by logging the source, target, direction (Forward/Backward), and selected parallel types.

    Key Changes:

    • Added debug logging hook in shardLoopLike function that outputs propagation details when TransformPropagator debug option is enabled
    • Implemented operator<< for PropagateDirection enum to enable readable stream output
    • Added SPECIALIZE_PRINTER(ParallelType) to support printing parallel types in debug messages
    • Cleaned up include dependencies by removing unused multidevice/utils.h
    • Alphabetized printer specializations for better maintainability

    The implementation is clean and follows existing debugging patterns in the codebase. All required infrastructure (printer specializations, stream operators) is properly set up to support the new logging functionality.

    Confidence Score: 5/5

    • This PR is safe to merge - it only adds debug logging infrastructure without changing functional behavior
    • All changes are non-functional debug additions with proper infrastructure support. The logging is gated behind a debug flag, the printer specializations follow existing patterns, and the operator<< implementation is complete and correct
    • No files require special attention

    Important Files Changed

    Filename Overview
    csrc/base.h Alphabetized SPECIALIZE_PRINTER declarations and added ParallelType for debug logging support
    csrc/scheduler/utils.cpp Implemented operator<< for PropagateDirection with Forward/Backward string conversion
    csrc/multidevice/propagation.cpp Added debug logging to shardLoopLike function showing propagation details including direction and parallel types

    Sequence Diagram

    sequenceDiagram
        participant Caller
        participant shardLoopLike
        participant Debug
        participant PropagateDirection
        participant ParallelType
        participant toDelimitedString
        
        Caller->>shardLoopLike: Call with ref, target, selected_parallel_types, direction
        shardLoopLike->>Debug: Check isDebugDumpEnabled(TransformPropagator)
        alt Debug enabled
            shardLoopLike->>Debug: debug() stream
            shardLoopLike->>Debug: << ref->toString()
            shardLoopLike->>Debug: << target->toString()
            shardLoopLike->>PropagateDirection: << direction (uses operator<<)
            PropagateDirection-->>shardLoopLike: Returns "Forward" or "Backward"
            shardLoopLike->>toDelimitedString: Convert selected_parallel_types
            toDelimitedString->>ParallelType: Printer<ParallelType>::toString() for each
            toDelimitedString-->>shardLoopLike: Returns delimited string
            shardLoopLike->>Debug: Output complete log message
        end
        shardLoopLike->>shardLoopLike: Continue with propagation logic
    
    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.

    3 files reviewed, no comments

    Edit Code Review Agent Settings | Greptile

    @wujingyue
    Copy link
    Collaborator Author

    !test

    @wujingyue wujingyue requested a review from Priya2698 January 30, 2026 23:55
    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.

    3 files reviewed, no comments

    Edit Code Review Agent Settings | Greptile

    @wujingyue
    Copy link
    Collaborator Author

    !test

    @wujingyue wujingyue added the enable-auto-merge Auto-merge a PR when: 1) PR mergeable 2) Internal CI complete 3) No failures label Feb 2, 2026
    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.

    2 files reviewed, 1 comment

    Edit Code Review Agent Settings | Greptile

    @wujingyue
    Copy link
    Collaborator Author

    !test

    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.

    1 file reviewed, 1 comment

    Edit Code Review Agent Settings | Greptile

    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    @wujingyue
    Copy link
    Collaborator Author

    !test

    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.

    3 files reviewed, no comments

    Edit Code Review Agent Settings | Greptile

    @wujingyue wujingyue merged commit fdac859 into main Feb 3, 2026
    42 of 43 checks passed
    @wujingyue wujingyue deleted the wjy/prop branch February 3, 2026 06:27
    @github-actions github-actions bot removed the enable-auto-merge Auto-merge a PR when: 1) PR mergeable 2) Internal CI complete 3) No failures label Feb 3, 2026
    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