Skip to content

Conversation

kwen2501
Copy link
Contributor

@kwen2501 kwen2501 commented Jul 4, 2025

Stack from ghstack (oldest at bottom):

maybe_initialize_env_vars and initialize_nvshmem_with_store are only used in NVSHMEMSymmetricMemory.cu. Moving them there.

cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Jul 4, 2025

🔗 Helpful Links

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

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

⏳ No Failures, 30 Pending

As of commit b281ab8 with merge base a6fab82 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added ciflow/h100-symm-mem oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (c10d) release notes category labels Jul 4, 2025
[ghstack-poisoned]
[ghstack-poisoned]
@kwen2501 kwen2501 changed the title [SymmMem] Remove unused APIs from header [SymmMem] Move code to where it is used Jul 4, 2025
@kwen2501 kwen2501 requested review from fegin and fduwjj July 4, 2025 05:16
// Print version
int major, minor;
::nvshmem_info_get_version(&major, &minor);
LOG(INFO) << "NVSHMEM is available, version: " << major << "." << minor;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
LOG(INFO) << "NVSHMEM is available, version: " << major << "." << minor;
LOG(INFO) << "NVSHMEM is available, version: " << major << '.' << minor;

int result = (stmt); \
TORCH_CHECK( \
result == 0, \
std::string(__FILE__) + ":" + std::to_string(__LINE__) + " " + msg + \
Copy link
Collaborator

@Skylion007 Skylion007 Jul 4, 2025

Choose a reason for hiding this comment

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

Hmm, this string is always constructed whether the torch check fails or passes... Reformulate to c10::LazyStr or atleast fmtlib.

`maybe_initialize_env_vars` and `initialize_nvshmem_with_store` are only used in `NVSHMEMSymmetricMemory.cu`. Moving them there.

cc H-Huang awgu wanchaol fegin fduwjj wz337 wconstab d4l3k

[ghstack-poisoned]
kwen2501 added a commit that referenced this pull request Jul 4, 2025
ghstack-source-id: 7eb6311
Pull-Request-resolved: #157611
@kwen2501
Copy link
Contributor Author

kwen2501 commented Jul 4, 2025

@pytorchbot merge -f "CI passed; minor format change"

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/h100-symm-mem Merged oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (c10d) release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants