Python type mismatch #1082

borbrudar · 2024-07-31T13:29:50Z

Added an extra argument to addTensor to allow users to specify the underlying tensor data type.

moratom

I think this will still behave a bit unexpected due to how C++ API works.

The int version I believe gets collapsed to uint8_t for example.

Let's add tests (see nndata_tensor_test.py).
To catch all various edge cases.

borbrudar · 2024-07-31T16:11:51Z

True, but that's due to the internals of how addTensor works, specifically:

        if(std::is_integral<_Ty>::value) {
            for(uint32_t i = 0; i < tensor.size(); i++) {
                vecData->data()[i + offset] = (uint8_t)tensor.data()[i];
            }
        } else {
            for(uint32_t i = 0; i < tensor.size(); i++) {
                *(uint16_t*)(&vecData->data()[2 * i + offset]) = fp32_to_fp16(tensor.data()[i]);
            }
        }

Tests from nndata_tensor_test.py seem to pass with the modified signatures, although the optional parameter doesn't appear to be optional.

moratom

Let's remove all of the specialized functions and rather have it as such:

By default we try to infer the type from the tensor
We allow to override the type with a function argument

We can do so both in C++ and Python I believe.

And please add tests as well.

moratom · 2024-09-16T15:11:30Z

include/depthai/pipeline/datatype/NNData.hpp

+ size_t sConvertedData = tensor.size();
+ if(dataType == dai::TensorInfo::DataType::FP16) sConvertedData *= 2;
+ else if(dataType == dai::TensorInfo::DataType::FP32 || dataType == dai::TensorInfo::DataType::INT) sConvertedData *= 4;


Let's do a switch here and cover all cases, so we visually see that nothing is missing

borbrudar · 2024-09-17T14:32:41Z

Reworked, however there are limitations to type conversions from python to c++ and vice versa, e.g. fp16 is not supported in pybind, so we have to cast it to a different type before we can pass it to c++ (i've chosen to upcast it to fp32). Similarly fp64 is not supported in c++ so i downcast it to fp32.

…match

moratom · 2024-09-19T12:22:04Z

include/depthai/pipeline/datatype/NNData.hpp

@@ -518,7 +617,7 @@ class NNData : public Buffer {

 return {};
 }
-#endif
+//#endif


Intentional?

moratom · 2024-09-19T12:23:20Z

include/depthai/pipeline/datatype/NNData.hpp

@@ -198,7 +198,7 @@ class NNData : public Buffer {
 */
 span<std::uint8_t> emplaceTensor(TensorInfo& tensor);

-#ifdef DEPTHAI_XTENSOR_SUPPORT
+//#ifdef DEPTHAI_XTENSOR_SUPPORT


I think this is still needed?

moratom · 2024-09-19T12:23:39Z

include/depthai/pipeline/datatype/NNData.hpp

+ NNData& addTensor(const std::string& name, const std::vector<_Ty>& data, dai::TensorInfo::DataType dataType) {
+ return addTensor<_Ty>(name, xt::adapt(data, std::vector<size_t>{1, data.size()}), dataType);
+ };
+ // addTensor vector overloads
+ NNData& addTensor(const std::string& name, const std::vector<int>& tensor) {
+ return addTensor<int>(name, tensor, dai::TensorInfo::DataType::INT);
+ };
+ NNData& addTensor(const std::string& name, const std::vector<uint16_t>& tensor) {
+ return addTensor<uint16_t>(name, tensor, dai::TensorInfo::DataType::FP16);
+ };
+ NNData& addTensor(const std::string& name, const std::vector<float>& tensor) {
+ return addTensor<float>(name, tensor, dai::TensorInfo::DataType::FP32);
+ };
+ NNData& addTensor(const std::string& name, const std::vector<double>& tensor) {
+ return addTensor<double>(name, tensor, dai::TensorInfo::DataType::FP64);
+ };
+ NNData& addTensor(const std::string& name, const std::vector<std::int8_t>& tensor) {
+ return addTensor<std::int8_t>(name, tensor, dai::TensorInfo::DataType::I8);
+ };
+ NNData& addTensor(const std::string& name, const std::vector<std::uint8_t>& tensor) {
+ return addTensor<std::uint8_t>(name, tensor, dai::TensorInfo::DataType::U8F);
+ };
+
+ // addTensor overloads
+ NNData& addTensor(const std::string& name, const xt::xarray<int>& tensor) {
+ return addTensor<int>(name, tensor, dai::TensorInfo::DataType::INT);
+ };
+ NNData& addTensor(const std::string& name, const xt::xarray<uint16_t>& tensor) {
+ return addTensor<uint16_t>(name, tensor, dai::TensorInfo::DataType::FP16);
+ };
+ NNData& addTensor(const std::string& name, const xt::xarray<float>& tensor) {
+ return addTensor<float>(name, tensor, dai::TensorInfo::DataType::FP32);
+ };
+ NNData& addTensor(const std::string& name, const xt::xarray<double>& tensor) {
+ return addTensor<double>(name, tensor, dai::TensorInfo::DataType::FP64);
+ };
+ NNData& addTensor(const std::string& name, const xt::xarray<std::int8_t>& tensor) {
+ return addTensor<std::int8_t>(name, tensor, dai::TensorInfo::DataType::I8);
+ };
+ NNData& addTensor(const std::string& name, const xt::xarray<std::uint8_t>& tensor) {
+ return addTensor<std::uint8_t>(name, tensor, dai::TensorInfo::DataType::U8F);
 };


Let's add tests for these too

borbrudar added 2 commits July 31, 2024 11:55

reversed python binding order

3784788

addTensor now takes an additional (optional) parameter

1a4b8ba

moratom reviewed Jul 31, 2024

View reviewed changes

borbrudar and others added 7 commits July 31, 2024 19:11

reworked nndata, bindings dont work yet

32b8ea1

template not needed

5d8eeeb

further template rework

59202d4

Merge branch 'auto-reconnect' into v3_develop

5f4dba6

compiles at least

534b122

mvp

3d08053

Merge branch 'luxonis:v3_develop' into v3_develop

d29b922

moratom requested changes Sep 16, 2024

View reviewed changes

borbrudar and others added 6 commits September 16, 2024 17:48

added switch

c1e3cde

Delete _3rdParty/Hunter directory

69b8c0a

Delete CMakeFiles directory

1a4c1e3

Delete generated directory

584154f

removed erronous dependency

9ec071c

added dynamic dispatch instead of many static functions

18c1ecf

borbrudar added 11 commits September 17, 2024 17:33

made function binding more explicit

a4cb609

python arrays implicitly converted to work

48db165

added C++ addTensor overloads

98d4afc

added fp64 support

cec87c4

added fp64 binding

b330336

fixed fp16 type conversion

6dd14ac

removed addTensor overloads

de1fb34

removed debug statements

945f0d6

added std::vector overloads

aea2f7a

defaulting doubles to fp32

0e3daa6

Merge remote-tracking branch 'origin/v3_develop' into python-type-mis…

3eb3d25

…match

borbrudar force-pushed the python-type-mismatch branch from 3cf81f7 to 3eb3d25 Compare September 18, 2024 15:46

borbrudar added 2 commits September 18, 2024 18:59

reordered

d1d8125

added tests

f275e55

moratom requested changes Sep 19, 2024

View reviewed changes

borbrudar added 3 commits September 19, 2024 16:10

readded ifdefs

b062bd7

added tests

62dea76

reintroduced templated arguments for backwards compatibility, tests pass

f03c5f7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python type mismatch #1082

Python type mismatch #1082

borbrudar commented Jul 31, 2024

moratom left a comment

borbrudar commented Jul 31, 2024 •

edited

Loading

moratom left a comment

moratom Sep 16, 2024

borbrudar Sep 16, 2024

borbrudar commented Sep 17, 2024

moratom Sep 19, 2024

borbrudar Sep 19, 2024

moratom Sep 19, 2024

moratom Sep 19, 2024

borbrudar Sep 19, 2024

Python type mismatch #1082

Are you sure you want to change the base?

Python type mismatch #1082

Conversation

borbrudar commented Jul 31, 2024

moratom left a comment

Choose a reason for hiding this comment

borbrudar commented Jul 31, 2024 • edited Loading

moratom left a comment

Choose a reason for hiding this comment

moratom Sep 16, 2024

Choose a reason for hiding this comment

borbrudar Sep 16, 2024

Choose a reason for hiding this comment

borbrudar commented Sep 17, 2024

moratom Sep 19, 2024

Choose a reason for hiding this comment

borbrudar Sep 19, 2024

Choose a reason for hiding this comment

moratom Sep 19, 2024

Choose a reason for hiding this comment

moratom Sep 19, 2024

Choose a reason for hiding this comment

borbrudar Sep 19, 2024

Choose a reason for hiding this comment

borbrudar commented Jul 31, 2024 •

edited

Loading