Skip to content

Fix misleading comments in commsOverlapBench overlap-pair-pgs (#222)#222

Open
MogicianWu wants to merge 1 commit into
facebookresearch:mainfrom
MogicianWu:export-D103730559
Open

Fix misleading comments in commsOverlapBench overlap-pair-pgs (#222)#222
MogicianWu wants to merge 1 commit into
facebookresearch:mainfrom
MogicianWu:export-D103730559

Conversation

@MogicianWu
Copy link
Copy Markdown

@MogicianWu MogicianWu commented May 4, 2026

Summary:

Fix three comments in commsOverlapBench.py that incorrectly described the --overlap-pair-pgs feature as creating only "two" process groups. The code actually creates 1 + len(pair_collectives_list) PGs, supporting N concurrent collectives on separate streams. Updated comments to accurately reflect this behavior.

Reviewed By: dsjohns2

Differential Revision: D103730559

@MogicianWu MogicianWu requested a review from louisfeng as a code owner May 4, 2026 21:02
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 4, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented May 4, 2026

@MogicianWu has exported this pull request. If you are a Meta employee, you can view the originating Diff in D103730559.

@meta-codesync meta-codesync Bot changed the title Fix misleading comments in commsOverlapBench overlap-pair-pgs Fix misleading comments in commsOverlapBench overlap-pair-pgs (#222) May 4, 2026
MogicianWu added a commit to MogicianWu/param that referenced this pull request May 4, 2026
…okresearch#222)

Summary:

Fix three comments in commsOverlapBench.py that incorrectly described the --overlap-pair-pgs feature as creating only "two" process groups. The code actually creates 1 + len(pair_collectives_list) PGs, supporting N concurrent collectives on separate streams. Updated comments to accurately reflect this behavior.

Reviewed By: dsjohns2

Differential Revision: D103730559
@MogicianWu MogicianWu force-pushed the export-D103730559 branch from 8083bcd to e12ace0 Compare May 4, 2026 21:08
@meta-codesync meta-codesync Bot changed the title Fix misleading comments in commsOverlapBench overlap-pair-pgs (#222) Fix misleading comments in commsOverlapBench overlap-pair-pgs May 4, 2026
@MogicianWu MogicianWu force-pushed the export-D103730559 branch from e12ace0 to a80461d Compare May 4, 2026 21:21
@meta-codesync meta-codesync Bot changed the title Fix misleading comments in commsOverlapBench overlap-pair-pgs Fix misleading comments in commsOverlapBench overlap-pair-pgs (#222) May 5, 2026
MogicianWu added a commit to MogicianWu/param that referenced this pull request May 5, 2026
…okresearch#222)

Summary:

Fix three comments in commsOverlapBench.py that incorrectly described the --overlap-pair-pgs feature as creating only "two" process groups. The code actually creates 1 + len(pair_collectives_list) PGs, supporting N concurrent collectives on separate streams. Updated comments to accurately reflect this behavior.

Reviewed By: dsjohns2

Differential Revision: D103730559
@MogicianWu MogicianWu force-pushed the export-D103730559 branch from a80461d to a6b6e50 Compare May 5, 2026 16:47
…okresearch#222)

Summary:

Fix three comments in commsOverlapBench.py that incorrectly described the --overlap-pair-pgs feature as creating only "two" process groups. The code actually creates 1 + len(pair_collectives_list) PGs, supporting N concurrent collectives on separate streams. Updated comments to accurately reflect this behavior.

Reviewed By: dsjohns2

Differential Revision: D103730559
@MogicianWu MogicianWu force-pushed the export-D103730559 branch from a6b6e50 to 1f9514e Compare May 5, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant