Skip to content
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 77 additions & 16 deletions src/liger_kernel/ops/tiled_mlp.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class LigerTiledMLPFunction(torch.autograd.Function):
mlp_module: the MLP nn.Module object
x: the input to MLP.forward (hidden_states)
shards: how many shards to use
compute_params: a list of weights engaged in the compute
*params: MLP parameters (passed as explicit inputs for FSDP compatibility)

Returns:
the computed hidden_states
Expand All @@ -39,12 +39,14 @@ def forward(
mlp_module: torch.nn.Module,
x: torch.Tensor,
shards: int,
compute_params: Optional[List[torch.nn.Parameter]] = None,
*params: torch.nn.Parameter,
Comment on lines 42 to +182

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.

Could you explain why we use position args instead of the original list?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This would be a consequence of using .grad() vs .autograd()

  • .backward() computes gradients on the leaf tensors, without having to return anything
  • .grad() return tensors per input specified.

In the past implementation, we only called .autograd(), which populated automatically the gradients, without having to worry about input definitions. The input in this case was a list of torch.nn.Parameter. Now that we are using .grad() instead of .autograd(), we need to take into account that .grad() returns a tuple with the gradients, depending on the inputs that we pass the function. I noticed this when expanding the test suite on the gradients.

) -> torch.Tensor:
ctx.fn = fn
ctx.mlp_module = mlp_module
ctx.shards = shards
ctx.save_for_backward(x)
ctx.num_params = len(params)
ctx.params = params # Store params as tuple, don't save (they're in mlp_module)
ctx.save_for_backward(x) # Only save input tensor

# x.shape could be [bs, seqlen, hidden_size] or [seqlen, hidden_size] (moe experts)
x_shards = list(torch.chunk(x, chunks=shards, dim=-2))
Expand All @@ -58,7 +60,8 @@ def forward(
@ensure_contiguous
def backward(ctx, *grads) -> tuple:
fn = ctx.fn
(x,) = ctx.saved_tensors
x = ctx.saved_tensors[0] # Only x was saved
params = ctx.params # Get params from context (not saved_tensors)
mlp_module = ctx.mlp_module
shards = ctx.shards

Expand All @@ -74,28 +77,82 @@ def backward(ctx, *grads) -> tuple:
# flatten bs+seqlen to avoid having stride issues when narrowing into seqlen w/ bs>1
x = x.view(-1, hidden_size)
incoming_grad = grads[0].view(-1, hidden_size)
x_grad = torch.zeros_like(x)
x_grad = torch.zeros_like(x) if x_requires_grad else None

# Initialize param grad accumulators as None for lazy allocation
param_grads: List[Optional[torch.Tensor]] = [None for _ in params]

x_shards = list(torch.chunk(x, chunks=shards, dim=0))

# Calculate cumulative offsets for correct gradient slicing when shards are uneven
shard_offset = 0

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.

Could you elaborate on the scenarios in which the original offset calculation fails?

shard_offset = i * x_shards[0].shape[0]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This assumes that all shards have an equal size, but in odd cases where n_tokens is not divisible by n_shards, this won't have a correct offset

for i, x_shard in enumerate(x_shards):
x_shard = x_shard.detach()

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.

Why do we need to detach?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Actually, we don't need to detach per se, since x is the leaf of this graph, so we wouldn't extend the gradients back past x.

Back when I edited this part, I added this .detach(), thinking it would skip x.grad computation entirely as an optimization.

after looking into it, perhaps by calling autograd.grad() on the shards we can automatically Get x.grad by coalescing, instead of having to accumulate them manually.

This way, x.grad could be correctly computed when computing the x_shard[i].grad, due to how autograd.grad() works. What do you think of exploring this aproach?

x_shard.requires_grad_(x_requires_grad)

# if seqlen is not exactly divisible by shards the last step will be shorter than shard_step
shard_step = x_shards[i].shape[0]
shard_offset = i * x_shards[0].shape[0]

x_shard.grad = x_grad.narrow(0, shard_offset, shard_step).view_as(x_shard)
incoming_grad_shard = incoming_grad.narrow(0, shard_offset, shard_step).view_as(x_shard)

# Build inputs list: x_shard + params that require grad
inputs = [x_shard] if x_requires_grad else []
inputs.extend([p for p in params if p.requires_grad])

with torch.enable_grad():
output = fn(mlp_module, x_shard)
torch.autograd.backward(output, incoming_grad_shard)

# unflatten
x_grad = x_grad.view(x_shape_orig)

return (None, None, x_grad, None, None)
if inputs:
# Use torch.autograd.grad for FSDP compatibility
# FSDP needs explicit gradient returns to manage sharded parameters
local_grads = torch.autograd.grad(
outputs=output,
inputs=inputs,
grad_outputs=incoming_grad_shard,
)
else:
local_grads = []

# Process gradients
grad_idx = 0
if x_requires_grad and x_grad is not None:
x_grad.narrow(0, shard_offset, shard_step).copy_(local_grads[grad_idx])
grad_idx += 1

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.

Split local grad into local_x_grad and local_param_grad then we don't have to maintain awkward grad_idx logic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Okey, will change on next commit


# Accumulate parameter gradients using in-place operations
for param_idx, p in enumerate(params):
if p.requires_grad:
grad = local_grads[grad_idx]
if param_grads[param_idx] is None:
# First shard: clone to avoid keeping local_grads alive
param_grads[param_idx] = grad.clone()
else:
# Subsequent shards: accumulate in-place
existing_grad = param_grads[param_idx]
assert existing_grad is not None
# Use add_ for true in-place accumulation
existing_grad.add_(grad)
grad_idx += 1

# Update offset for next shard
shard_offset += shard_step

# CRITICAL: Explicitly delete local_grads to free memory immediately
# Without this, the gradient tensors stay alive until loop completion
del local_grads

# unflatten x_grad if needed
if x_grad is not None:
x_grad = x_grad.view(x_shape_orig)

# Return gradients: (fn, mlp_module, x, shards, *params)
# Clone param_grads to ensure they're not views into local_grads
final_param_grads = []
for param_idx, p in enumerate(params):
if param_grads[param_idx] is not None:
final_param_grads.append(param_grads[param_idx].clone())
else:
final_param_grads.append(torch.zeros_like(p))

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.

Suggested change
# Clone param_grads to ensure they're not views into local_grads
final_param_grads = []
for param_idx, p in enumerate(params):
if param_grads[param_idx] is not None:
final_param_grads.append(param_grads[param_idx].clone())
else:
final_param_grads.append(torch.zeros_like(p))
# Clone param_grads to ensure they're not views into local_grads
final_param_grads = [param_grad.clone() if param_grad is not None else torch.zeros_like(params[idx]) for idx, param_grad in enumerate(param_grads)]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good simplification, it makes sense

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will change it on next commit


return (None, None, x_grad, None, *final_param_grads)
Comment thread
alektebel marked this conversation as resolved.
Outdated


def apply_tiled_mlp(
Expand All @@ -113,7 +170,7 @@ def apply_tiled_mlp(
mlp_module: the MLP nn.Module object
x: the input tensor with shape [bs, seqlen, hidden_size] or [seqlen, hidden_size]
num_shards: number of shards to use. If None, automatically calculated as ceil(seqlen / hidden_size)
compute_params: list of parameters for DeepSpeed ZeRO optimization
compute_params: list of parameters engaged in the computation (for FSDP compatibility)

Returns:
output tensor with the same shape as input
Expand All @@ -127,10 +184,14 @@ def apply_tiled_mlp(
# Ensure num_shards is at least 1
num_shards = max(1, num_shards)

# Get all parameters from the module if compute_params not provided
if compute_params is None:
compute_params = list(mlp_module.parameters())

return LigerTiledMLPFunction.apply(
fn,
mlp_module,
x,
num_shards,
compute_params,
*compute_params,
)
Loading