Skip to content

[JAX] Fix classification of sync-tagged collective start instructions - #3425

Merged
KshitijLakhani merged 1 commit into
NVIDIA:mainfrom
KshitijLakhani:klakhani/fix/jax-sync-tagged-collective-start
Aug 26, 2026
Merged

[JAX] Fix classification of sync-tagged collective start instructions#3425
KshitijLakhani merged 1 commit into
NVIDIA:mainfrom
KshitijLakhani:klakhani/fix/jax-sync-tagged-collective-start

Conversation

@KshitijLakhani

@KshitijLakhani KshitijLakhani commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Description

Fix JAX collective-byte accounting when XLA emits an all-reduce-start instruction with "is_sync":true.
The parser previously treated this as an unsuffixed synchronous collective and classified the communication bytes as other. This change prioritizes the explicit *-start instruction form.

No LayerNorm/RMSNorm implementation or expected byte calculations are changed.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@KshitijLakhani KshitijLakhani self-assigned this Aug 25, 2026
@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR fixes JAX distributed-test collective accounting by prioritizing explicit *-start instructions over overlapping "is_sync":true metadata.

  • Classifies direct and wrapped asynchronous collective starts before synchronous collectives.
  • Preserves ROOT-prefix handling and unsuffixed synchronous collective detection.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
tests/jax/distributed_test_base.py Reorders collective classification so sync-tagged *-start instructions use the asynchronous-start identification path; no eligible follow-up defect was established.

Reviews (2): Last reviewed commit: "[JAX] Fix sync-tagged collective start c..." | Re-trigger Greptile

@KshitijLakhani
KshitijLakhani marked this pull request as draft August 25, 2026 22:17
Signed-off-by: Kshitij Janardan Lakhani <klakhani@nvidia.com>
@KshitijLakhani
KshitijLakhani force-pushed the klakhani/fix/jax-sync-tagged-collective-start branch from 0bee041 to ef0887c Compare August 25, 2026 22:19
@KshitijLakhani
KshitijLakhani marked this pull request as ready for review August 25, 2026 22:20
@KshitijLakhani

Copy link
Copy Markdown
Collaborator Author

/te-ci jax L0 L1

@jberchtold-nvidia jberchtold-nvidia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM pending CI, thanks!

@KshitijLakhani
KshitijLakhani merged commit d7340a4 into NVIDIA:main Aug 26, 2026
24 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants