Skip to content

Fix Metal try_clone sharing the source buffer - #4030

Open
muzammil35 wants to merge 1 commit into
huggingface:mainfrom
muzammil35:fix-metal-copy-shares-buffer
Open

muzammil35 wants to merge 1 commit into
huggingface:mainfrom
muzammil35:fix-metal-copy-shares-buffer

Conversation

@muzammil35

Copy link
Copy Markdown
  • Fixes Metal: Tensor::copy() shares the source's buffer, so in-place updates (Var::set, zero_set) show through the "copy" #4027
  • MetalStorage::try_clone cloned the Arc, so Tensor::copy()
    returned a tensor backed by the same GPU buffer as the original. Any
    in-place write to the source (Var::set, zero_set, slice_set) showed
    through the copy. Allocate a new buffer and blit the data into it,
    matching the CPU and CUDA backends.
  • Testing: added copy_is_independent, which fails on Metal before the fix and passes after it.
  • existing failure: conv2d_grad_noncontiguous_kernel_metal also fails on main, so it isn't related to this change.

MetalStorage::try_clone cloned the Arc<Buffer>, so Tensor::copy()
returned a tensor backed by the same GPU buffer as the original. Any
in-place write to the source (Var::set, zero_set, slice_set) showed
through the copy. Allocate a new buffer and blit the data into it,
matching the CPU and CUDA backends.

This branch has not been deployed

No deployments
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.

Metal: Tensor::copy() shares the source's buffer, so in-place updates (Var::set, zero_set) show through the "copy"

1 participant