Skip to content

[fx] Allow customization of submod name in split graph#164035

Closed
kwen2501 wants to merge 3 commits intogh/kwen2501/265/basefrom
gh/kwen2501/265/head
Closed

[fx] Allow customization of submod name in split graph#164035
kwen2501 wants to merge 3 commits intogh/kwen2501/265/basefrom
gh/kwen2501/265/head

Conversation

@kwen2501
Copy link
Collaborator

@kwen2501 kwen2501 commented Sep 27, 2025

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 27, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/164035

Note: Links to docs will display an error until the docs builds have been completed.

⏳ No Failures, 32 Pending

As of commit 572a163 with merge base 8d81564 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

kwen2501 added a commit that referenced this pull request Sep 27, 2025
@pytorch-bot pytorch-bot bot added the release notes: fx release notes category label Sep 27, 2025
@kwen2501 kwen2501 requested a review from zou3519 September 27, 2025 03:59
keep_original_order: Optional[bool] = False,
keep_original_node_name: Optional[bool] = False,
keep_original_input_name: bool = True,
partition_affix: Optional[str] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making this kwarg only

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this "affix" and not "prefix"?

Copy link
Collaborator Author

@kwen2501 kwen2501 Sep 28, 2025

Choose a reason for hiding this comment

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

Because the eventual submod name will be submod_abc_i, where abc is the added argument here. I don't want to give user an illusion that their new submod starts with abc.

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

suuure why not

[ghstack-poisoned]
[ghstack-poisoned]
@kwen2501
Copy link
Collaborator Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 29, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Sep 29, 2025
Changing PP submodules' name from `submod_i` to `submod_pp_i` to distinguish from the submodule created by HOP.

Pull Request resolved: #164037
Approved by: https://github.com/H-Huang
ghstack dependencies: #164045, #164035
@yangw-dev
Copy link
Contributor

yangw-dev commented Oct 1, 2025

@pytorchbot revert -m "internal build failed Buck build failed for this target, and is likely caused by your changes." -c ghfirst

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

Halting the revert as the revert comment has been edited.

@yangw-dev
Copy link
Contributor

@pytorchbot revert -m "internal build failed Buck build failed for this target, and is likely caused by your changes." -c ghfirst

@yangw-dev
Copy link
Contributor

the associated internal task D83479121

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot added a commit that referenced this pull request Oct 1, 2025
This reverts commit 704cd77.

Reverted #164037 on behalf of https://github.com/yangw-dev due to internal build failed Buck build failed for this target, and is likely caused by your changes. ([comment](#164035 (comment)))
pytorchmergebot added a commit that referenced this pull request Oct 1, 2025
…)"

This reverts commit 615da7b.

Reverted #164035 on behalf of https://github.com/yangw-dev due to internal build failed Buck build failed for this target, and is likely caused by your changes. ([comment](#164035 (comment)))
@pytorchmergebot
Copy link
Collaborator

@kwen2501 your PR has been successfully reverted.

@pytorchmergebot pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels Oct 1, 2025
@kwen2501
Copy link
Collaborator Author

kwen2501 commented Oct 1, 2025

@pytorchbot merge -f "internal signal succeeded upon retry"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Oct 1, 2025
Changing PP submodules' name from `submod_i` to `submod_pp_i` to distinguish from the submodule created by HOP.

Pull Request resolved: #164037
Approved by: https://github.com/H-Huang
ghstack dependencies: #164045, #164035
Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Oct 21, 2025
This reverts commit 704cd77.

Reverted pytorch#164037 on behalf of https://github.com/yangw-dev due to internal build failed Buck build failed for this target, and is likely caused by your changes. ([comment](pytorch#164035 (comment)))
Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Oct 21, 2025
…ch#164035)"

This reverts commit 615da7b.

Reverted pytorch#164035 on behalf of https://github.com/yangw-dev due to internal build failed Buck build failed for this target, and is likely caused by your changes. ([comment](pytorch#164035 (comment)))
Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Oct 21, 2025
Fixes pytorch#164030: HOP and pipelining both name things submod_i
by adding an optional argument `partition_affix` to `split_module` API.

Pull Request resolved: pytorch#164035
Approved by: https://github.com/ezyang
ghstack dependencies: pytorch#164045
Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Oct 21, 2025
Changing PP submodules' name from `submod_i` to `submod_pp_i` to distinguish from the submodule created by HOP.

Pull Request resolved: pytorch#164037
Approved by: https://github.com/H-Huang
ghstack dependencies: pytorch#164045, pytorch#164035
@github-actions github-actions bot deleted the gh/kwen2501/265/head branch November 1, 2025 02:19
Khanaksahu pushed a commit to Khanaksahu/pytorch-fork that referenced this pull request Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-td Do not run TD on this PR ciflow/trunk Trigger trunk jobs on your pull request fx Merged release notes: fx release notes category Reverted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants