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

Handle Tensor.__deepcopy__ via clone(), on IPU (#89129) #89999

Merged
merged 1 commit into from Dec 7, 2022

Commits on Dec 6, 2022

  1. Handle Tensor.__deepcopy__ via clone(), on IPU (pytorch#89129)

    Currently it falls through to a call to `storage()`, which the IPU doesn't support.
    
    I've made the minimal change here for ease of merging (this'd help us if it was in for 1.13.1), however...
    
    **QUESTION**: Is there any reason why `not torch._C._has_storage(self)` needs to *also* be guarded on `self.device.type == privateuseone`? in other words, could the condition for using `clone` not be this?
    
    ```python
    self.is_sparse
    or self.device.type
    in ["lazy", "xla", "mps", "ort", "meta", "hpu", "ipu"]
    or not torch._C._has_storage(self)
    or (type(self) is not Tensor and self.data_ptr() == 0)
    ```
    
    If the condition fails, the very next thing is a call to `self._typed_storage()` which will fail, so it feels to me like *any* case without storage shouldn't fall through to the `storage()` call.
    
    The original PR for adding the 'no storage and device is `PrivateUse1`' condition ([86557](pytorch#86557)) doesn't discuss whether this could be broadened.
    Pull Request resolved: pytorch#89129
    Approved by: https://github.com/albanD
    charlie-wt authored and pytorchmergebot committed Dec 6, 2022
    Copy the full SHA
    1e0e36c View commit details
    Browse the repository at this point in the history