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

Removes unnecessary cloning #6761

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Removes unnecessary cloning #6761

merged 3 commits into from
Nov 21, 2024

Conversation

swigls
Copy link
Contributor

@swigls swigls commented Nov 19, 2024

clone_tensors_for_torch_save() function:

When the item.device is different from device input,
tensor.clone() is not actually required because to() function also clones the original tensor.

+) I observed memory bloat under following conditions:

  • Training a Whisper model w/ transformers framework with ZeRO-0 and ZeRO-1 configuration.
  • Memory bloating can be observed every time the model state_dict is cloned using clone_tensors_for_torch_save()

After I removed the unnecessary clone(), seems like the problem is solved.

About clone_tensors_for_torch_save() function:

if the `item.device` is different from `device` input,
clone() is not required because to() function do the same thing.

* With the unnecessary clone(), I observed memory bloat. After I removed the unnecessary clone(), it's solved.
@swigls swigls requested a review from tjruwase as a code owner November 19, 2024 08:05
@tjruwase
Copy link
Contributor

@swigls, thanks!

@loadams loadams added this pull request to the merge queue Nov 21, 2024
Merged via the queue into microsoft:master with commit f515104 Nov 21, 2024
10 checks passed
@swigls swigls deleted the patch-1 branch November 22, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants