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

Simplify selfAllocationReplay #4057

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Simplify selfAllocationReplay #4057

wants to merge 1 commit into from

Conversation

wujingyue
Copy link
Collaborator

No description provided.

@wujingyue
Copy link
Collaborator Author

!test

Copy link

github-actions bot commented Mar 11, 2025

Review updated until commit ff8c560

Description

  • Simplify selfAllocationReplay function

  • Remove unnecessary reduction filtering

  • Update replay logic to include all allocation IDs


Changes walkthrough 📝

Relevant files
Enhancement
transform_replay.cpp
Simplify selfAllocationReplay logic                                           

csrc/transform_replay.cpp

  • Removed reduction filtering from self_logical
  • Updated ReplaySelf to use all self_allocation IDs
  • Simplified logic for pushing reduction IDs
  • +3/-11   

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ Recommended focus areas for review

    Possible Issue

    The change in self_logical from TensorDomain::noReductions(self->logical()) to self->logical() might lead to incorrect behavior if reduction IDs are present and need to be excluded.

    auto self_logical = self->logical();
    Possible Issue

    The removal of self_allocation_no_reduction and the change in ReplaySelf instantiation might cause issues if non-reduction IDs are necessary for correct replay behavior.

    ReplaySelf replay(self_allocation, axis_map);
    Code Clarity

    The comment // NOLINTNEXTLINE(modernize-use-emplace) should be removed or justified if emplace_back is not applicable.

    new_contiguity.push_back(std::nullopt);

    @wujingyue
    Copy link
    Collaborator Author

    !test --diff

    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.

    None yet

    1 participant