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

[MXNET-779]Add DLPack Transformation API #12047

Open
wants to merge 31 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@wkcn
Contributor

wkcn commented Aug 6, 2018

Description

Add DLPack Transformation API

DLPack is important for sharing tensors and operators among Deep Learning Frameworks.
Issue: #11964

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Add MXNDArrayToDLPack, MXNDArrayFromDLPack, MXNDArrayCallDLPackDeleter and MXNDArrayCallDLPackCapsuleDeleter in c_api
  • Add ToDLPack() and FromDLPack for the class NDArray
  • Add the constructor TBlob(const DLTensor &dltensor)
  • Add the function DLDataTypeTransform in tensor_blob.h for the transformation from DLDataType to MShadow Type Flag
  • Add NDArray(const TBlob &data, int dev_id, const std::function<void()>& deleter) function
  • Add mx.nd.NDArray.to_dlpack_for_read, mx.nd.NDArray.to_dlpack_for_write, mx.nd.to_dlpack_to_read, mx.nd.to_dlpack_for_write and mx.nd.from_dlpack
  • Add unit-test for DLPack Transformation API

Comments

Extra Test with PyTorch

@wkcn wkcn requested review from anirudh2290 and szha as code owners Aug 6, 2018

@wkcn wkcn changed the title from Add DLPack Transformation API to [MXNET-779]Add DLPack Transformation API Aug 6, 2018

@tqchen

This is good change, I will make a few additional comments below

Show outdated Hide outdated python/mxnet/_ctypes/ndarray.py
Show outdated Hide outdated python/mxnet/cython/ndarray.pyx
Show outdated Hide outdated src/ndarray/ndarray.cc
Show outdated Hide outdated src/ndarray/ndarray.cc
@tqchen

This comment has been minimized.

Show comment
Hide comment
@tqchen

tqchen Aug 6, 2018

Member

see my additional comments in #11964

Member

tqchen commented Aug 6, 2018

see my additional comments in #11964

@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Aug 7, 2018

Contributor

@tqchen Thank you!
I have changed it as you suggest.

Contributor

wkcn commented Aug 7, 2018

@tqchen Thank you!
I have changed it as you suggest.

Show outdated Hide outdated include/mxnet/c_api.h
Show outdated Hide outdated include/mxnet/c_api.h
Show outdated Hide outdated include/mxnet/tensor_blob.h
Show outdated Hide outdated python/mxnet/_ctypes/ndarray.py
@tqchen

This comment has been minimized.

Show comment
Hide comment
@tqchen

tqchen Aug 8, 2018

Member

Since it is a C API change @piiswrong can you also take a look?

Member

tqchen commented Aug 8, 2018

Since it is a C API change @piiswrong can you also take a look?

@szha szha requested review from piiswrong and removed request for szha Aug 9, 2018

Show outdated Hide outdated python/mxnet/ndarray/ndarray.py
Show outdated Hide outdated src/c_api/c_api.cc
@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Aug 22, 2018

Contributor

@tqchen
Hi! What should I do next for this PR?
For the release of DLPack,
here are the implements of TVM, PyTorch and Cupy.

TVM src/runtime/ndarray.cc#L148

NDArray NDArray::FromDLPack(DLManagedTensor* tensor) {
  NDArray::Container* data = new NDArray::Container();
  data->deleter = Internal::DLPackDeleter;
  data->manager_ctx = tensor;
  data->dl_tensor = tensor->dl_tensor;
  return NDArray(data);
}

PyTorch aten/src/ATen/DLConvertor.cpp#L170

Tensor fromDLPack(const DLManagedTensor* src) {
  Backend backend = getATenBackend(src->dl_tensor.ctx);
  ScalarType stype = toScalarType(src->dl_tensor.dtype);
  auto deleter = [src](void * self) {
    src->deleter(const_cast<DLManagedTensor*>(src));
  };
  return getType(backend, stype).tensorFromBlob(
      src->dl_tensor.data,
      IntList(src->dl_tensor.shape, src->dl_tensor.ndim),
      IntList(src->dl_tensor.strides, src->dl_tensor.ndim),
      deleter);
}

CuPy cupy/core/dlpack.pyx#L231

mem = DLPackMemory(dltensor)
mem_ptr = memory.MemoryPointer(mem, mem.dlm_tensor.dl_tensor.byte_offset)
cupy_array = ndarray(shape_vec, cp_dtype, mem_ptr)

The all NDArray save the deleter of dlpack or DLManagedTensor.

So I added a new member dlpack for mx.nd.NDArray (Python) for store of dlpack.
When the NDArray release, the dlpack will call the deleter.
Dose NDArray(C++) manage the release of dltensor?

Contributor

wkcn commented Aug 22, 2018

@tqchen
Hi! What should I do next for this PR?
For the release of DLPack,
here are the implements of TVM, PyTorch and Cupy.

TVM src/runtime/ndarray.cc#L148

NDArray NDArray::FromDLPack(DLManagedTensor* tensor) {
  NDArray::Container* data = new NDArray::Container();
  data->deleter = Internal::DLPackDeleter;
  data->manager_ctx = tensor;
  data->dl_tensor = tensor->dl_tensor;
  return NDArray(data);
}

PyTorch aten/src/ATen/DLConvertor.cpp#L170

Tensor fromDLPack(const DLManagedTensor* src) {
  Backend backend = getATenBackend(src->dl_tensor.ctx);
  ScalarType stype = toScalarType(src->dl_tensor.dtype);
  auto deleter = [src](void * self) {
    src->deleter(const_cast<DLManagedTensor*>(src));
  };
  return getType(backend, stype).tensorFromBlob(
      src->dl_tensor.data,
      IntList(src->dl_tensor.shape, src->dl_tensor.ndim),
      IntList(src->dl_tensor.strides, src->dl_tensor.ndim),
      deleter);
}

CuPy cupy/core/dlpack.pyx#L231

mem = DLPackMemory(dltensor)
mem_ptr = memory.MemoryPointer(mem, mem.dlm_tensor.dl_tensor.byte_offset)
cupy_array = ndarray(shape_vec, cp_dtype, mem_ptr)

The all NDArray save the deleter of dlpack or DLManagedTensor.

So I added a new member dlpack for mx.nd.NDArray (Python) for store of dlpack.
When the NDArray release, the dlpack will call the deleter.
Dose NDArray(C++) manage the release of dltensor?

@tqchen

This comment has been minimized.

Show comment
Hide comment
@tqchen

tqchen Aug 22, 2018

Member

My point is that the DLManagerd Tensor should manage its own deletion and you do not have to add dlpack field for mx.nd.NDArray. Even when the frontend NDArray release, the internal data will be kept alive until the pycapsule get released and the DLManagedTensor's deleter get called

Member

tqchen commented Aug 22, 2018

My point is that the DLManagerd Tensor should manage its own deletion and you do not have to add dlpack field for mx.nd.NDArray. Even when the frontend NDArray release, the internal data will be kept alive until the pycapsule get released and the DLManagedTensor's deleter get called

@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Aug 22, 2018

Contributor

@tqchen Hi! I have modified the PR, the DLManaged Tensor will manage itself.
However, there is a problem in PR(DLManagedTensor manages itself).
In this case,

import mxnet as mx
pack = mx.nd.array([1,2,3]).to_dlpack_for_write()
b = mx.nd.from_dlpack(pack)
del pack # DLManagedTensor's deleter get called when `pack` get released.
print (b) # b != [1,2,3]

Does it need to manage the release of dlpack in NDArray Chunk (C++) ?

I add deleter in TBlob and NDArray::Chunk in the latest PR (add deleter for TBlob and Chunk in NDArray), and solve the problem.

Contributor

wkcn commented Aug 22, 2018

@tqchen Hi! I have modified the PR, the DLManaged Tensor will manage itself.
However, there is a problem in PR(DLManagedTensor manages itself).
In this case,

import mxnet as mx
pack = mx.nd.array([1,2,3]).to_dlpack_for_write()
b = mx.nd.from_dlpack(pack)
del pack # DLManagedTensor's deleter get called when `pack` get released.
print (b) # b != [1,2,3]

Does it need to manage the release of dlpack in NDArray Chunk (C++) ?

I add deleter in TBlob and NDArray::Chunk in the latest PR (add deleter for TBlob and Chunk in NDArray), and solve the problem.

@tqchen

This comment has been minimized.

Show comment
Hide comment
@tqchen

tqchen Aug 22, 2018

Member

pack is only supposed to be consumed once, when from_dlpack is called, the pack itself should be marked as used and its deleter will no longer be called when the capsule get destroyed

Member

tqchen commented Aug 22, 2018

pack is only supposed to be consumed once, when from_dlpack is called, the pack itself should be marked as used and its deleter will no longer be called when the capsule get destroyed

@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Aug 22, 2018

Contributor

@tqchen Yes. The latest PR has implemented it in ndarray.py.
And the new NDArray created by from_dlpack will call the deleter.

Contributor

wkcn commented Aug 22, 2018

@tqchen Yes. The latest PR has implemented it in ndarray.py.
And the new NDArray created by from_dlpack will call the deleter.

@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Sep 9, 2018

Contributor

@tqchen @piiswrong
Hi!
Is there any problem? Could you please merge the PR?
Thank you!

Contributor

wkcn commented Sep 9, 2018

@tqchen @piiswrong
Hi!
Is there any problem? Could you please merge the PR?
Thank you!

Show outdated Hide outdated include/mxnet/tensor_blob.h
Show outdated Hide outdated include/mxnet/ndarray.h
@tqchen

This comment has been minimized.

Show comment
Hide comment
@tqchen

tqchen Sep 9, 2018

Member

@wkcn sorry I was away for a vacation recently, please see my comments

Member

tqchen commented Sep 9, 2018

@wkcn sorry I was away for a vacation recently, please see my comments

@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Sep 10, 2018

Contributor

@tqchen Thank you! I will try to change it.

Contributor

wkcn commented Sep 10, 2018

@tqchen Thank you! I will try to change it.

@wkcn

This comment has been minimized.

Show comment
Hide comment
@wkcn

wkcn Sep 11, 2018

Contributor

@tqchen Hi! I have updated the code as your comments.
I add a new constructor NDArray(const TBlob &data, int dev_id, const std::function<void()>& deleter) for NDArray class, because the performance between std::make_shared and std::shared_ptr::shared_ptr is different.

Contributor

wkcn commented Sep 11, 2018

@tqchen Hi! I have updated the code as your comments.
I add a new constructor NDArray(const TBlob &data, int dev_id, const std::function<void()>& deleter) for NDArray class, because the performance between std::make_shared and std::shared_ptr::shared_ptr is different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment