Skip to content

Conversation

H-Huang
Copy link
Member

@H-Huang H-Huang commented Jan 22, 2025

Stack from ghstack (oldest at bottom):

test_modules_can_be_imported test is currently failing due to a few missing private modules and this PR gets it working before I start to clean up the public allow list

@H-Huang H-Huang requested a review from albanD as a code owner January 22, 2025 18:12
H-Huang added a commit that referenced this pull request Jan 22, 2025
ghstack-source-id: f2abea8
Pull Request resolved: #145387
@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jan 22, 2025
Copy link

pytorch-bot bot commented Jan 22, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 8dc9142 with merge base 40e27fb (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Sounds ok as a stopgap
We should discuss with @wconstab what to do with this partial dependency.

"torch.onnx._internal.exporter._reporting",
"torch.onnx._internal.exporter._schemas",
"torch.onnx._internal.exporter._tensors",
"torch.onnx._internal.exporter._torchlib.ops",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wconstab this is failing because this relies on the "pulp" package to run which is not a dependency. Why do we depend on this to even import the files / any details where I can read about why we are making this a dependency?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's for the file below in torch.distributed._tools.sac_ilp which was built as an optional tool for memory estimation for FSDP and uses pulp as an ILP solver. But without pulp the tool doesn't work

from pulp import ( # type: ignore[import-untyped,import-not-found]

@H-Huang
Copy link
Member Author

H-Huang commented Jan 23, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 23, 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

@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 23, 2025 14:21 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 23, 2025 14:21 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 23, 2025 14:21 Inactive
pytorchmergebot pushed a commit that referenced this pull request Jan 23, 2025
Make torchelastic publicly importable by raising error on import etcd lazily, [BE task, row 7](https://docs.google.com/spreadsheets/d/1TtATnLJf1rVXaBQd3X3yYqm9xNN9BIWG7QqRgrFiRRI/edit?gid=1748512924#gid=1748512924)

Pull Request resolved: #145396
Approved by: https://github.com/albanD
ghstack dependencies: #145387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants