Skip to content
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

multi-gpu: fix tensor device placements for various models #35763

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dvrogozh
Copy link
Contributor

@@ -309,6 +309,7 @@ def slow_forward(
) # [batch, intermediate_size, seq_len]
else:
conv_state = cache_params.update_conv_state(self.layer_idx, hidden_states, cache_position)
conv_state = conv_state.to(self.conv1d.weight.device)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI reviewers, this fix was basically taken from here:

conv_states = cache_params.conv_states[self.layer_idx].to(device=self.conv1d.weight.device)

@@ -818,7 +818,7 @@ def forward(
if inputs_embeds is None:
inputs_embeds = self.wte(input_ids)
position_embeds = self.wpe(position_ids)
hidden_states = inputs_embeds + position_embeds
hidden_states = inputs_embeds + position_embeds.to(inputs_embeds.device)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI reviewers, this fix was taken from here:

hidden_states = inputs_embeds + positions.to(inputs_embeds.device)

See #30836 (comment) for associated discussion.

Copy link
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

See comment here: #35762 (comment)

@qubvel qubvel removed their request for review January 20, 2025 16:49
Fixing the following errors in few models:
```
>       hidden_states = inputs_embeds + pos_embeds
E       RuntimeError: Expected all tensors to be on the same device, but found at least two devices, xpu:2 and xpu:3!
```

Fixes: huggingface#35762
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
@dvrogozh
Copy link
Contributor Author

@SunMarc, that's a big change since models reuse the code from each other and that's enforced on ci level with make fix-copies. To simplify the review I have structured commits as follows:

  1. First and second commits are making changed to the "primary" sources (those from which other models copy code)
  2. Third commit is automatically generated with make fix-copies from the first 2

So, I think it might be easier to review first 2 commits separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants