-
Notifications
You must be signed in to change notification settings - Fork 598
Defer the data-parallel gradient all-reduce to update() under gradient accumulation #5099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ga-bench-5060
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -209,6 +209,14 @@ def mesh_axes_for_dim(axis_names): | |||||||||
| return tuple(axis for axis in axis_names if axis is not None) | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def batch_mesh_axes(mesh, rules=None): | ||||||||||
| """Returns the mesh axes of size > 1 that the activation batch dimension is sharded over.""" | ||||||||||
| spec = logical_to_mesh_axes(("activation_batch",), mesh, rules=rules) | ||||||||||
| if spec is None: | ||||||||||
| return frozenset() | ||||||||||
|
Comment on lines
+215
to
+216
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Suggested change
|
||||||||||
| return frozenset(axis for axis in mesh_axes_for_dim(spec.partitions[0]) if mesh.shape.get(axis, 1) > 1) | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def mesh_axes_size(mesh, axes, *, label): | ||||||||||
| """Returns the product of mesh sizes for a set of axes.""" | ||||||||||
| size = 1 | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
sharding.mesh_axes_for_dimonnamed_sharding.spec.partitionswill fail to detect_DATA_AXISif it is nested inside a tuple (e.g., when a dimension is sharded over multiple axes like('data', 'model')). This can lead to JAX rejecting the spec at runtime because it thinks the axis is not already sharded.Using the existing helper
sharding.get_mesh_axes_used_by_tensor_specis much more robust as it correctly flattens the PartitionSpec and checks all used axes.