Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2958867394 Things are looking green 💚 (The failures are not related though I haven't seen the integration test fail on other PRs, see https://github.com/apache/arrow/actions/runs/1006367/job/43793532564?pr=42118#step:12:6104) Will wait for a day or two for possible comments, then I plan to merge. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
github-actions[bot] commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2958379404 Revision: e001a495e65c2c45ff8d0e34d2954c96067fae62 Submitted crossbow builds: [ursacomputing/crossbow @ actions-2a3342027a](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2a3342027a) |Task|Status| ||--| |example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/1865757/job/43796343208)| |example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/1865513/job/43796342216)| |example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/1865524/job/43796342244)| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/1864253/job/43796338052)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/1865383/job/43796341998)| |test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/1865368/job/43796342335)| |test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/1864429/job/43796338745)| |test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/1864493/job/43796338792)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/1865105/job/43796340894)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/1864141/job/43796337676)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/1865646/job/43796342749)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/1864795/job/43796339824)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/1865231/job/43796341214)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/1865742/job/43796343006)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/1864955/job/43796340298)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2958368809 @github-actions crossbow submit -g cpp -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2893631142 I need to check two related build failures: - https://github.com/ursacomputing/crossbow/actions/runs/15112601862/job/42475351126#step:6:6111 - https://github.com/ursacomputing/crossbow/actions/runs/15112600796/job/42475340772#step:6:8806 then I will need to ask for another round of review 😊 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
github-actions[bot] commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2890782951 Revision: 76c33c28c77184f6c7558802653edd3c74f2c0e9 Submitted crossbow builds: [ursacomputing/crossbow @ actions-7781855d5b](https://github.com/ursacomputing/crossbow/branches/all?query=actions-7781855d5b) |Task|Status| ||--| |example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601339/job/42475344518)| |example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/15112600723/job/42475340342)| |example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601391/job/42475347020)| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601491/job/42475347486)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/15112600499/job/42475339205)| |test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601846/job/42475351174)| |test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601483/job/42475344533)| |test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/15112600629/job/42475340502)| |test-conda-cpp-meson|[](https://github.com/ursacomputing/crossbow/actions/runs/15112600554/job/42475339894)| |test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601659/job/42475348064)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/15112600826/job/42475340990)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601412/job/42475347038)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601829/job/42475351133)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601739/job/42475348003)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/15112602347/job/42475355541)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/15112601874/job/42475352813)| |test-conda-p
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2890774395 @github-actions crossbow submit -g cpp -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
rok commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2077400941
##
python/pyarrow/tests/test_dlpack.py:
##
@@ -103,6 +101,30 @@ def test_dlpack(value_type, np_type_str):
expected = np.array([], dtype=np.dtype(np_type_str))
check_dlpack_export(arr_zero, expected)
+t = pa.Tensor.from_numpy(expected)
+check_dlpack_export(t, expected)
+
+
+@check_bytes_allocated
+@pytest.mark.parametrize('np_type',
+ [np.uint8, np.uint16, np.uint32, np.uint64,
+ np.int8, np.int16, np.int32, np.int64,
+ np.float16, np.float32, np.float64,])
+def test_tensor_dlpack(np_type):
+if Version(np.__version__) < Version("1.24.0"):
+pytest.skip("No dlpack support in numpy versions older than 1.22.0, "
+"strict keyword in assert_array_equal added in numpy
version "
+"1.24.0")
+
+arr = np.array([1, 2, 3, 4, 5, 6, 1, 1])
+expected = np.ndarray((2, 2, 2), dtype=np_type, buffer=arr)
Review Comment:
Nit: oddly [numpy doesn't specify what the default order
is](https://numpy.org/doc/stable/reference/generated/numpy.ndarray.html#numpy-ndarray),
so how about:
```suggestion
expected = np.ndarray((2, 2, 2), dtype=np_type, buffer=arr, order='C')
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
rok commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2855321375 @AlenkaF sorry, didn't get to this today, will do first thing tomorrow. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2853462324 Pinging for a Python review when you have a moment @rok. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
zeroshade commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2051556087
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -66,15 +67,15 @@ struct ManagerCtx {
} // namespace
Result ExportArray(const std::shared_ptr& arr) {
- // Define DLDevice struct nad check if array type is supported
+ // Define DLDevice struct and check if array type is supported
// by the DLPack protocol at the same time. Raise TypeError if not.
// Supported data types: int, uint, float with no validity buffer.
- ARROW_ASSIGN_OR_RAISE(auto device, ExportDevice(arr))
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(arr))
// Define the DLDataType struct
const DataType& type = *arr->type();
std::shared_ptr data = arr->data();
- ARROW_ASSIGN_OR_RAISE(auto dlpack_type, GetDLDataType(type));
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
Review Comment:
in this case, it probably doesn't matter and I'd probably just leave it as
auto unless someone specifically would prefer it to be more explicit.
In most cases, unless it isn't particularly clear what the type is or there
is some ambiguity, it's likely better "style" to just use auto (at least IMO)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2048535783
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -66,15 +67,15 @@ struct ManagerCtx {
} // namespace
Result ExportArray(const std::shared_ptr& arr) {
- // Define DLDevice struct nad check if array type is supported
+ // Define DLDevice struct and check if array type is supported
// by the DLPack protocol at the same time. Raise TypeError if not.
// Supported data types: int, uint, float with no validity buffer.
- ARROW_ASSIGN_OR_RAISE(auto device, ExportDevice(arr))
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(arr))
// Define the DLDataType struct
const DataType& type = *arr->type();
std::shared_ptr data = arr->data();
- ARROW_ASSIGN_OR_RAISE(auto dlpack_type, GetDLDataType(type));
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
Review Comment:
No particular reason—just a naive assumption that being explicit might be
more in line with “good” C++ style. =)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2048558050
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -130,4 +131,71 @@ Result ExportDevice(const
std::shared_ptr& arr) {
}
}
+struct TensorManagerCtx {
+ std::shared_ptr t;
+ std::vector strides;
+ std::vector shape;
+ DLManagedTensor tensor;
+};
+
+Result ExportTensor(const std::shared_ptr& t) {
+ // Define the DLDataType struct
+ const DataType& type = *t->type();
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
+
+ // Define DLDevice struct
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(t))
+
+ // Create TensorManagerCtx that will serve as the owner of the
DLManagedTensor
+ std::unique_ptr ctx(new TensorManagerCtx);
+
+ // Define the data pointer to the DLTensor
+ // If tensor is of length 0, data pointer should be NULL
+ if (t->size() == 0) {
+ctx->tensor.dl_tensor.data = NULL;
+ } else {
+ctx->tensor.dl_tensor.data = t->raw_mutable_data();
+ }
+
+ ctx->tensor.dl_tensor.device = device;
+ ctx->tensor.dl_tensor.ndim = t->ndim();
+ ctx->tensor.dl_tensor.dtype = dlpack_type;
+ ctx->tensor.dl_tensor.byte_offset = 0;
+
+ std::vector& shape_arr = ctx->shape;
+ shape_arr.reserve(t->ndim());
+ for (auto i : t->shape()) {
+shape_arr.emplace_back(i);
+ }
+ ctx->tensor.dl_tensor.shape = shape_arr.data();
+
+ std::vector& strides_arr = ctx->strides;
+ strides_arr.reserve(t->ndim());
+ auto byte_width = t->type()->byte_width();
+ for (auto i : t->strides()) {
+strides_arr.emplace_back(i / byte_width);
+ }
+ ctx->tensor.dl_tensor.strides = strides_arr.data();
+
+ ctx->t = std::move(t);
+ ctx->tensor.manager_ctx = ctx.get();
+ ctx->tensor.deleter = [](struct DLManagedTensor* self) {
+delete reinterpret_cast(self->manager_ctx);
+ };
+ return &ctx.release()->tensor;
+}
+
+Result ExportDevice(const std::shared_ptr& t) {
+ // Define DLDevice struct
+ DLDevice device;
+ if (t->data()->device_type() == DeviceAllocationType::kCPU) {
+device.device_id = 0;
+device.device_type = DLDeviceType::kDLCPU;
+return device;
+ } else {
+return Status::NotImplemented(
+"DLPack support is implemented only for buffers on CPU device.");
Review Comment:
The only reason is that we're splitting the work across multiple PRs.
There's an umbrella issue tracking this effort here:
https://github.com/apache/arrow/issues/39296, and the GPU-related part is
covered by this issue: https://github.com/apache/arrow/issues/45721.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
zeroshade commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2047495330
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -130,4 +131,71 @@ Result ExportDevice(const
std::shared_ptr& arr) {
}
}
+struct TensorManagerCtx {
+ std::shared_ptr t;
+ std::vector strides;
+ std::vector shape;
+ DLManagedTensor tensor;
+};
+
+Result ExportTensor(const std::shared_ptr& t) {
+ // Define the DLDataType struct
+ const DataType& type = *t->type();
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
+
+ // Define DLDevice struct
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(t))
+
+ // Create TensorManagerCtx that will serve as the owner of the
DLManagedTensor
+ std::unique_ptr ctx(new TensorManagerCtx);
+
+ // Define the data pointer to the DLTensor
+ // If tensor is of length 0, data pointer should be NULL
+ if (t->size() == 0) {
+ctx->tensor.dl_tensor.data = NULL;
+ } else {
+ctx->tensor.dl_tensor.data = t->raw_mutable_data();
+ }
+
+ ctx->tensor.dl_tensor.device = device;
+ ctx->tensor.dl_tensor.ndim = t->ndim();
+ ctx->tensor.dl_tensor.dtype = dlpack_type;
+ ctx->tensor.dl_tensor.byte_offset = 0;
+
+ std::vector& shape_arr = ctx->shape;
+ shape_arr.reserve(t->ndim());
+ for (auto i : t->shape()) {
+shape_arr.emplace_back(i);
+ }
+ ctx->tensor.dl_tensor.shape = shape_arr.data();
+
+ std::vector& strides_arr = ctx->strides;
+ strides_arr.reserve(t->ndim());
+ auto byte_width = t->type()->byte_width();
+ for (auto i : t->strides()) {
+strides_arr.emplace_back(i / byte_width);
+ }
+ ctx->tensor.dl_tensor.strides = strides_arr.data();
+
+ ctx->t = std::move(t);
+ ctx->tensor.manager_ctx = ctx.get();
+ ctx->tensor.deleter = [](struct DLManagedTensor* self) {
+delete reinterpret_cast(self->manager_ctx);
+ };
+ return &ctx.release()->tensor;
+}
+
+Result ExportDevice(const std::shared_ptr& t) {
+ // Define DLDevice struct
+ DLDevice device;
+ if (t->data()->device_type() == DeviceAllocationType::kCPU) {
+device.device_id = 0;
+device.device_type = DLDeviceType::kDLCPU;
+return device;
+ } else {
+return Status::NotImplemented(
+"DLPack support is implemented only for buffers on CPU device.");
Review Comment:
any reason we aren't implementing the non-cpu cases here since we support it
in the generic gpu package etc.?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
zeroshade commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2047491267
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -130,4 +131,71 @@ Result ExportDevice(const
std::shared_ptr& arr) {
}
}
+struct TensorManagerCtx {
+ std::shared_ptr t;
+ std::vector strides;
+ std::vector shape;
+ DLManagedTensor tensor;
+};
+
+Result ExportTensor(const std::shared_ptr& t) {
+ // Define the DLDataType struct
+ const DataType& type = *t->type();
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
+
+ // Define DLDevice struct
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(t))
+
+ // Create TensorManagerCtx that will serve as the owner of the
DLManagedTensor
+ std::unique_ptr ctx(new TensorManagerCtx);
Review Comment:
for stylistic reasons, `std::make_unique`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
zeroshade commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r2047472486
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -66,15 +67,15 @@ struct ManagerCtx {
} // namespace
Result ExportArray(const std::shared_ptr& arr) {
- // Define DLDevice struct nad check if array type is supported
+ // Define DLDevice struct and check if array type is supported
// by the DLPack protocol at the same time. Raise TypeError if not.
// Supported data types: int, uint, float with no validity buffer.
- ARROW_ASSIGN_OR_RAISE(auto device, ExportDevice(arr))
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(arr))
// Define the DLDataType struct
const DataType& type = *arr->type();
std::shared_ptr data = arr->data();
- ARROW_ASSIGN_OR_RAISE(auto dlpack_type, GetDLDataType(type));
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
Review Comment:
any particular reason to be explicit here instead of using auto?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
jjerphan commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2805045957 @jjerphan: I appreciate your ping, unfortunately I don't have time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on PR #42118: URL: https://github.com/apache/arrow/pull/42118#issuecomment-2805040866 This PR is ready for a first round of reviews — tagging @rok and @zeroshade for your thoughts when you have a moment. cc @jjerphan in case you're interested in taking a look as well! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
jorisvandenbossche commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r1638110951
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -130,4 +131,63 @@ Result ExportDevice(const
std::shared_ptr& arr) {
}
}
+struct TensorManagerCtx {
+ std::shared_ptr t;
+ std::vector strides;
+ DLManagedTensor tensor;
+};
+
+Result ExportTensor(const std::shared_ptr& t) {
+ // Define the DLDataType struct
+ const DataType& type = *t->type();
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
+
+ // Define DLDevice struct
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(t))
+
+ // Create TensorManagerCtx that will serve as the owner of the
DLManagedTensor
+ std::unique_ptr ctx(new TensorManagerCtx);
+
+ // Define the data pointer to the DLTensor
+ // If tensor is of length 0, data pointer should be NULL
+ if (t->size() == 0) {
+ctx->tensor.dl_tensor.data = NULL;
+ } else {
+ctx->tensor.dl_tensor.data = t->raw_mutable_data();
+ }
+
+ ctx->tensor.dl_tensor.device = device;
+ ctx->tensor.dl_tensor.ndim = t->ndim();
+ ctx->tensor.dl_tensor.dtype = dlpack_type;
+
+ ctx->tensor.dl_tensor.shape = const_cast(t->shape().data());
+ std::vector* strides_arr = &ctx->strides;
+ strides_arr->resize(t->ndim());
+ for (int i = 0; i < t->ndim(); i++) {
+(*strides_arr)[i] = t->strides().data()[i] / t->type()->byte_width();
+ }
Review Comment:
I am entirely sure if this was written this way for performance, but my
feeling is that this is quite complicated written, and you should also be able
to just iterate through the values in `t->strides()` and add them to
`&ctx->strides` with more standard std::vector functionality.
(for example, have a look at `FixedShapeTensorArray::FromTensor` how some of
the vectors are being created)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
jorisvandenbossche commented on code in PR #42118:
URL: https://github.com/apache/arrow/pull/42118#discussion_r1638074823
##
cpp/src/arrow/c/dlpack.cc:
##
@@ -130,4 +131,67 @@ Result ExportDevice(const
std::shared_ptr& arr) {
}
}
+struct TensorManagerCtx {
+ std::shared_ptr t;
+ std::vector shape;
+ std::vector strides;
+ DLManagedTensor tensor;
+};
+
+Result ExportTensor(const std::shared_ptr& t) {
+ // Define the DLDataType struct
+ const DataType& type = *t->type();
+ ARROW_ASSIGN_OR_RAISE(DLDataType dlpack_type, GetDLDataType(type));
+
+ // Define DLDevice struct
+ ARROW_ASSIGN_OR_RAISE(DLDevice device, ExportDevice(t))
+
+ // Create ManagerCtx that will serve as the owner of the DLManagedTensor
+ std::unique_ptr ctx(new TensorManagerCtx);
+
+ // Define the data pointer to the DLTensor
+ // If tensor is of length 0, data pointer should be NULL
+ if (t->size() == 0) {
+ctx->tensor.dl_tensor.data = NULL;
+ } else {
+ctx->tensor.dl_tensor.data = t->raw_mutable_data();
+ }
+
+ ctx->tensor.dl_tensor.device = device;
+ ctx->tensor.dl_tensor.ndim = t->ndim();
+ ctx->tensor.dl_tensor.dtype = dlpack_type;
+
+ std::vector* shape_arr = &ctx->shape;
+ std::vector* strides_arr = &ctx->strides;
+ shape_arr->resize(t->ndim());
+ strides_arr->resize(t->ndim());
+ for (int i = 0; i < t->ndim(); i++) {
+(*shape_arr)[i] = t->shape().data()[i];
+(*strides_arr)[i] = t->strides().data()[i] / t->type()->byte_width();
+ }
+ ctx->tensor.dl_tensor.shape = shape_arr->data();
+ ctx->tensor.dl_tensor.strides = strides_arr->data();
+ ctx->tensor.dl_tensor.byte_offset = 0;
+
+ ctx->t = std::move(t);
+ ctx->tensor.manager_ctx = ctx.get();
+ ctx->tensor.deleter = [](struct DLManagedTensor* self) {
+delete reinterpret_cast(self->manager_ctx);
Review Comment:
```suggestion
delete reinterpret_cast(self->manager_ctx);
```
That might fix the memory leak?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
AlenkaF commented on code in PR #42118: URL: https://github.com/apache/arrow/pull/42118#discussion_r1636737282 ## cpp/src/arrow/c/dlpack.h: ## @@ -48,4 +51,7 @@ Result ExportArray(const std::shared_ptr& arr); ARROW_EXPORT Result ExportDevice(const std::shared_ptr& arr); +ARROW_EXPORT +Result ExportTensorDevice(const std::shared_ptr& t); Review Comment: https://github.com/apache/arrow/pull/42118/commits/646b8b9d0ab4119618731329871076107f4bd2c1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-39294: [C++][Python] DLPack on Tensor class [arrow]
jorisvandenbossche commented on code in PR #42118: URL: https://github.com/apache/arrow/pull/42118#discussion_r1636595894 ## cpp/src/arrow/c/dlpack.h: ## @@ -48,4 +51,7 @@ Result ExportArray(const std::shared_ptr& arr); ARROW_EXPORT Result ExportDevice(const std::shared_ptr& arr); +ARROW_EXPORT +Result ExportTensorDevice(const std::shared_ptr& t); Review Comment: For the C++ side, it might be more natural API to call this `ExportDevice` as well (so just add an additional supported signature for the existing function) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
