[OV JS] Add method copyTo in tensor file #29760

sanleo-wq · 2025-03-26T16:15:15Z

Details:

Added the copyTo method to the Tensor class to copy data from one tensor to another.

Added tests to verify that the copyTo method works correctly.

Fixed [any bug or error].

Tickets:

29758

almilosz · 2025-03-27T08:49:14Z

src/bindings/js/node/lib/addon.ts

@@ -423,6 +423,8 @@ interface Tensor {
   * Reports whether the tensor is continuous or not.
   */
  isContinuous(): boolean;
+


Suggested change

/**

* Copy tensor, destination tensor should have the same element type and shape.

*/

almilosz

Hi, thanks for creating this Pull Request!
Please take a look at my comments

almilosz · 2025-03-27T08:49:27Z

src/bindings/js/node/lib/addon.ts

@@ -423,6 +423,8 @@ interface Tensor {
   * Reports whether the tensor is continuous or not.
   */
  isContinuous(): boolean;
+
+  copyTo(): boolean;


copyTo should take one argument. Please update this definition

Suggested change

copyTo(): boolean;

copyTo(tensor: Tensor): undefined;

almilosz · 2025-03-27T08:51:17Z

tests/test_tensor_copyTo/package.json

@@ -0,0 +1,5 @@
+{
+  "scripts": {


I think it's not necessary. Please remove it

almilosz · 2025-03-27T08:51:51Z

tests/test_tensor_copyTo/tensor.test.js

@@ -0,0 +1,15 @@
+const { Tensor } = require('../../src/bindings/js/node/lib/addon.ts');
+
+describe('Tensor class', () => {


Please add this test to src/bindings/js/node/tests/unit/tensor.test.js

Retribution98

Hello @sanleo-wq ,
Thank you for your contribution!
Please fix these comments.

Retribution98 · 2025-03-27T15:18:28Z

src/bindings/js/node/src/tensor.cpp

@@ -191,3 +192,25 @@ Napi::Value TensorWrap::is_continuous(const Napi::CallbackInfo& info) {
    }
    return Napi::Boolean::New(env, _tensor.is_continuous());
 }
+
+Napi::Value TensorWrap::copy_to(const Napi::CallbackInfo& info) {


The declaration of this method must be defined in the header file src/bindings/js/node/include/tensor.hpp. Do not forget add a doc string there.

Retribution98 · 2025-03-27T15:19:11Z

src/bindings/js/node/src/tensor.cpp

@@ -191,3 +192,25 @@ Napi::Value TensorWrap::is_continuous(const Napi::CallbackInfo& info) {
    }
    return Napi::Boolean::New(env, _tensor.is_continuous());
 }
+
+Napi::Value TensorWrap::copy_to(const Napi::CallbackInfo& info) {


This method should return void instead of Boolean.

Suggested change

Napi::Value TensorWrap::copy_to(const Napi::CallbackInfo& info) {

void TensorWrap::copy_to(const Napi::CallbackInfo& info) {

Retribution98 · 2025-03-27T15:19:35Z

tests/test_tensor_copyTo/tensor.test.js

+
+describe('Tensor class', () => {
+  it('should correctly copy data to another tensor', () => {
+    const tensor1 = new Tensor('float32', [2, 2]);


Suggested change

const tensor1 = new Tensor('float32', [2, 2]);

const tensor1 = new Tensor('ov.element.f32', [2, 2]);

Retribution98 · 2025-03-27T15:19:50Z

tests/test_tensor_copyTo/tensor.test.js

+  it('should correctly copy data to another tensor', () => {
+    const tensor1 = new Tensor('float32', [2, 2]);
+
+    const tensor2 = new Tensor('float32', [2, 2]);


Suggested change

const tensor2 = new Tensor('float32', [2, 2]);

const tensor2 = new Tensor('ov.element.f32', [2, 2]);

Retribution98 · 2025-03-27T15:21:00Z

tests/test_tensor_copyTo/tensor.test.js

+
+    assert.deepEqual(tensor1.getData(), tensor2.getData(), 'Data was not copied correctly.');
+  });
+});


Please add tests with incorrect arguments:

incorrect count of args

different types

different shapes

Retribution98 · 2025-03-27T15:45:42Z

src/bindings/js/node/src/tensor.cpp

+    if (info.Length() != 1 || !info[0].IsObject()) {
+        reportError(env, "The copyTo method requires one argument of type Tensor.");
+        return env.Undefined();
+    }


It is required to check if the tensor from the argument is an acceptable tensor (same element type and shape).

Retribution98 · 2025-03-27T15:59:01Z

src/bindings/js/node/lib/addon.ts

@@ -423,6 +423,8 @@ interface Tensor {
   * Reports whether the tensor is continuous or not.
   */
  isContinuous(): boolean;
+
+  copyTo(): boolean;


Suggested change

copyTo(): boolean;

copyTo(tensor: Tensor): undefined;

Retribution98 · 2025-03-27T16:13:26Z

src/bindings/js/node/src/tensor.cpp

@@ -44,7 +44,8 @@ Napi::Function TensorWrap::get_class(Napi::Env env) {
                        InstanceMethod("getShape", &TensorWrap::get_shape),
                        InstanceMethod("getElementType", &TensorWrap::get_element_type),
                        InstanceMethod("getSize", &TensorWrap::get_size),
-                        InstanceMethod("isContinuous", &TensorWrap::is_continuous)});
+                        InstanceMethod("isContinuous", &TensorWrap::is_continuous), 


As you can see in the CI, this space is unnecessary. Please fix other problems on your own.

Suggested change

InstanceMethod("isContinuous", &TensorWrap::is_continuous),

InstanceMethod("isContinuous", &TensorWrap::is_continuous),

Add method copyTo in tensor file

1f32a07

sanleo-wq requested review from a team as code owners March 26, 2025 16:15

github-actions bot added category: JS API OpenVino JS API Bindings no-match-files labels Mar 26, 2025

sys-openvino-ci added the ExternalPR External contributor label Mar 26, 2025

mlukasze requested review from Retribution98 and almilosz and removed request for a team March 27, 2025 05:50

mlukasze linked an issue Mar 27, 2025 that may be closed by this pull request

[Good First Issue]: Enable Tensor.copyTo() method #29758

Open

almilosz reviewed Mar 27, 2025

View reviewed changes

Retribution98 requested changes Mar 27, 2025

View reviewed changes

sanleo-wq closed this by deleting the head repository Mar 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OV JS] Add method copyTo in tensor file #29760

[OV JS] Add method copyTo in tensor file #29760

sanleo-wq commented Mar 26, 2025 •

edited by mlukasze

Loading

almilosz Mar 27, 2025

almilosz left a comment

almilosz Mar 27, 2025

Retribution98 Mar 27, 2025

almilosz Mar 27, 2025

almilosz Mar 27, 2025

Retribution98 left a comment

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

Retribution98 Mar 27, 2025

+/**
+* Copy tensor, destination tensor should have the same element type and shape.
+*/

		@@ -0,0 +1,15 @@
		const { Tensor } = require('../../src/bindings/js/node/lib/addon.ts');

		describe('Tensor class', () => {

	Napi::Value TensorWrap::copy_to(const Napi::CallbackInfo& info) {
	void TensorWrap::copy_to(const Napi::CallbackInfo& info) {

	const tensor1 = new Tensor('float32', [2, 2]);
	const tensor1 = new Tensor('ov.element.f32', [2, 2]);

	const tensor2 = new Tensor('float32', [2, 2]);
	const tensor2 = new Tensor('ov.element.f32', [2, 2]);

	InstanceMethod("isContinuous", &TensorWrap::is_continuous),
	InstanceMethod("isContinuous", &TensorWrap::is_continuous),

[OV JS] Add method copyTo in tensor file #29760

[OV JS] Add method copyTo in tensor file #29760

Conversation

sanleo-wq commented Mar 26, 2025 • edited by mlukasze Loading

Details:

Tickets:

Choose a reason for hiding this comment

almilosz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Retribution98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanleo-wq commented Mar 26, 2025 •

edited by mlukasze

Loading