Improve tensor point cloud io #5404

ZhengyuDiao · 2022-08-04T14:43:01Z

This change is

update-docs · 2022-08-04T14:43:05Z

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

yxlao

Reviewed 5 of 6 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @ZhengyuDiao)

cpp/open3d/t/io/file_format/FileTXT.cpp line 59 at r2 (raw file):

    int64_t num_points = file.GetNumLines();
    int64_t num_elements_per_line = line_template.size();

num_elements_per_line -> num_attributes

if (line_template == "xyz") {
    num_attributes = 3;
} else if (line_template == "xyzi") {
    num_attributes = 4;
} else if (line_template == "xyzn") {
    num_attributes = 6;
} 
...

cpp/open3d/t/io/file_format/FileTXT.cpp line 66 at r2 (raw file):

    int i = 0;
    double x, y, z, attr, attr_x, attr_y, attr_z;

double line_attributes[num_attributes];
// or
std::vector<double> line_attributes(num_attributes);

cpp/open3d/t/io/file_format/FileTXT.cpp line 70 at r2 (raw file):

    // Read TXT to buffer
    while ((line_buffer = file.ReadLine())) {

while (line_buffer = file.ReadLine())

Code quote:

 while ((line_buffer = file.ReadLine()))

cpp/open3d/t/io/file_format/FileTXT.cpp line 95 at r2 (raw file):

            utility::LogWarning("Read TXT failed at line: {}", line_buffer);
            return false;
        }

std::memcpy(&pcd_buffer_ptr[num_attributes * i], line_attributes, num_attributes * sizeof(double));

cpp/open3d/t/io/file_format/FileTXT.cpp line 103 at r2 (raw file):

    // Buffer to point cloud
    pointcloud.SetPointPositions(pcd_buffer.Slice(1, 0, 3, 1));
    if (line_template == std::vector<std::string>{"x", "y", "z"})

if (line_template == "xyz")

cpp/open3d/t/io/file_format/FileTXT.cpp line 104 at r2 (raw file):

    pointcloud.SetPointPositions(pcd_buffer.Slice(1, 0, 3, 1));
    if (line_template == std::vector<std::string>{"x", "y", "z"})
        ;

{ 
    // No additional attributes;
}

cpp/open3d/t/io/file_format/FileTXT.cpp line 106 at r2 (raw file):

        ;
    else if (line_template == std::vector<std::string>{"x", "y", "z", "i"}) {
        pointcloud.SetPointAttr("intensities", pcd_buffer.Slice(1, 3, 4, 1));

pcd_buffer.Slice(1, 3, 4, 1).Contiguous()

cpp/open3d/t/io/file_format/FileTXT.cpp line 125 at r2 (raw file):

                           const open3d::io::ReadPointCloudOption &params) {
    try {
        std::string formatTXT =

format_txt

cpp/open3d/t/io/file_format/FileTXT.cpp line 129 at r2 (raw file):

        if (formatTXT == "xyz") {
            return ReadFileTXT(filename, pointcloud,
                               std::vector<std::string>{"x", "y", "z"}, params);

{"x", "y", "z"} without std::vector<std::string> already works

cpp/open3d/t/io/file_format/FileTXT.cpp line 166 at r2 (raw file):

    }
    utility::CountingProgressReporter reporter(params.update_progress);
    const core::Tensor &points = pointcloud.GetPointPositions();

What if the point cloud is CUDA? Need to call .To(Device("CPU:0")).

cpp/open3d/t/io/file_format/FileTXT.cpp line 181 at r2 (raw file):

        for (int i = 0; i < points.GetShape(0); i++) {
            if (fprintf(file.GetFILE(), "%.10f %.10f %.10f\n",
                        points[i][0].Item<double>(),

Do not use Item as it is slow. First, get the buffer of points and use pointers.

Code quote:

pcd_buffer_ptr

cpp/open3d/t/io/file_format/FileTXT.cpp line 181 at r2 (raw file):

        for (int i = 0; i < points.GetShape(0); i++) {
            if (fprintf(file.GetFILE(), "%.10f %.10f %.10f\n",
                        points[i][0].Item<double>(),

What if the tensor is not double? This may be wrong.

Code quote:

Item<double>

ZhengyuDiao

Reviewable status: 5 of 6 files reviewed, 12 unresolved discussions (waiting on @yxlao)

cpp/open3d/t/io/file_format/FileTXT.cpp line 59 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

num_elements_per_line -> num_attributes

if (line_template == "xyz") {
    num_attributes = 3;
} else if (line_template == "xyzi") {
    num_attributes = 4;
} else if (line_template == "xyzn") {
    num_attributes = 6;
} 
...

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 66 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

double line_attributes[num_attributes];
// or
std::vector<double> line_attributes(num_attributes);

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 70 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

while (line_buffer = file.ReadLine())

I try , but the compiler would report an error if removing brackets.

cpp/open3d/t/io/file_format/FileTXT.cpp line 95 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

std::memcpy(&pcd_buffer_ptr[num_attributes * i], line_attributes, num_attributes * sizeof(double));

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 103 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

if (line_template == "xyz")

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 104 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

{ 
    // No additional attributes;
}

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 106 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

pcd_buffer.Slice(1, 3, 4, 1).Contiguous()

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 125 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

format_txt

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 129 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

{"x", "y", "z"} without std::vector<std::string> already works

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 166 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

What if the point cloud is CUDA? Need to call .To(Device("CPU:0")).

We just get the value of pointcloud and write txt file, still need to convert it to cpu?

cpp/open3d/t/io/file_format/FileTXT.cpp line 181 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

Do not use Item as it is slow. First, get the buffer of points and use pointers.

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 181 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

What if the tensor is not double? This may be wrong.

Done.

yxlao

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yxlao and @ZhengyuDiao)

cpp/open3d/t/io/file_format/FileTXT.cpp line 82 at r3 (raw file):

        int i = 0;
        double *line_attr_ptr =
                (double *)malloc(num_attributes * sizeof(double));

malloc needs free. Prefer not to use malloc when possible. E.g. we can use vector and get data from it, or just use an array.

std::vector<float> line_attrs(num_attributes);
float *line_attr_ptr = line_attrs.data();

yxlao · 2022-08-06T21:54:17Z

cpp/open3d/t/io/file_format/FileTXT.cpp

+            num_attributes = 3;
+        } else if (format_txt == "xyzi") {
+            num_attributes = 4;
+        } else if ((format_txt == "xyzn") || (format_txt == "xyzrgb")) {


} else if (format_txt == "xyzn") { num_attributes = 6; } else if (format_txt == "xyzrgb") { num_attributes = 6;

is more clear

yxlao · 2022-08-06T21:55:50Z

cpp/open3d/t/io/file_format/FileTXT.cpp

+        } else if ((format_txt == "xyzn") || (format_txt == "xyzrgb")) {
+            num_attributes = 6;
+        } else {
+            utility::LogWarning("The format of TXT is not supported");


Print the name as well:

utility::LogWarning("The format of {} is not supported", format_txt);

Same for the rest.

yxlao · 2022-08-06T21:57:38Z

cpp/open3d/t/io/file_format/FileTXT.cpp

+            return false;
+        }
+
+        core::Tensor pcd_buffer({num_points, num_attributes}, core::Float64);


After thinking about it again, it is best to keep everything Float32 by default. This is because all creation APIs of the tensor TriangleMesh and PointCould uses Float32. Could you change everything here to float32?

yxlao · 2022-08-06T21:58:57Z

cpp/open3d/t/io/file_format/FileTXT.cpp

+        double *pcd_buffer_ptr = pcd_buffer.GetDataPtr<double>();
+
+        int i = 0;
+        double *line_attr_ptr =


By vector, you can do something like this. This will avoid malloc and free.

std::vector<float> line_attrs(num_attributes); float *line_attr_ptr = line_attrs.data();

yxlao · 2022-08-06T22:01:03Z

cpp/open3d/t/io/file_format/FileTXT.cpp

+                utility::filesystem::GetFileExtensionInLowerCase(filename);
+
+        int64_t num_points = file.GetNumLines();
+        int64_t num_attributes;


Nit: since we are using attr in the rest of the code, we can also rename this to num_attrs

…into zhengyu/t_pcd_io

ZhengyuDiao

Reviewable status: 5 of 6 files reviewed, 6 unresolved discussions (waiting on @yxlao)

cpp/open3d/t/io/file_format/FileTXT.cpp line 70 at r3 (raw file):

Previously, yxlao (Yixing Lao) wrote…

} else if (format_txt == "xyzn") {
    num_attributes = 6;
} else if (format_txt == "xyzrgb") {
    num_attributes = 6;

is more clear

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 73 at r3 (raw file):

Previously, yxlao (Yixing Lao) wrote…

Print the name as well:
utility::LogWarning("The format of {} is not supported", format_txt);
Same for the rest.

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 81 at r3 (raw file):

Previously, yxlao (Yixing Lao) wrote…

By vector, you can do something like this. This will avoid malloc and free.
std::vector<float> line_attrs(num_attributes);
float *line_attr_ptr = line_attrs.data();

Done.

cpp/open3d/t/io/file_format/FileTXT.cpp line 82 at r3 (raw file):

Previously, yxlao (Yixing Lao) wrote…

malloc needs free. Prefer not to use malloc when possible. E.g. we can use vector and get data from it, or just use an array.

Done.

ZhengyuDiao · 2022-08-07T09:03:34Z

Now the other files io in tensor module use double as default, so the tests of io also use double as default. If I use float as default in FileTXT, it cannot pass the tests.
I think we may keep double as default this time and modify the default dtype from double to float in the relevant files in next pr. What do you think about it? @yxlao

yxlao

Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @ZhengyuDiao)

ZhengyuDiao added 6 commits June 28, 2022 22:43

add create_box

eb53d50

add newline setting

5806017

fetch origin

b562a70

pull origin

c9d5924

Merge branch 'master' of https://github.com/isl-org/Open3D

b5d587b

improve tensor point cloud io

7332a22

yxlao added 2 commits August 4, 2022 20:14

fix r -> w when writing a file

8373d11

fix read num elements

38b5beb

yxlao requested changes Aug 5, 2022

View reviewed changes

ZhengyuDiao added 2 commits August 5, 2022 22:17

improve pointcloud io code

0519ba3

fix line_attribute

b958df9

ZhengyuDiao commented Aug 6, 2022

View reviewed changes

Merge branch 'master' into zhengyu/t_pcd_io

215c5a6

yxlao requested changes Aug 6, 2022

View reviewed changes

yxlao reviewed Aug 6, 2022

View reviewed changes

ZhengyuDiao added 2 commits August 7, 2022 10:43

fix double->float, var name, remove malloc

fce28e6

Merge branch 'zhengyu/t_pcd_io' of https://github.com/isl-org/Open3D …

1ef71e8

…into zhengyu/t_pcd_io

ZhengyuDiao commented Aug 7, 2022

View reviewed changes

keep double as default

1e4ace3

yxlao approved these changes Aug 10, 2022

View reviewed changes

yxlao merged commit b40282c into master Aug 10, 2022

yxlao deleted the zhengyu/t_pcd_io branch August 10, 2022 06:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve tensor point cloud io #5404

Improve tensor point cloud io #5404

ZhengyuDiao commented Aug 4, 2022 •

edited by germanros1987

Loading

update-docs bot commented Aug 4, 2022

yxlao left a comment

ZhengyuDiao left a comment

yxlao left a comment •

edited

Loading

yxlao Aug 6, 2022

yxlao Aug 6, 2022

yxlao Aug 6, 2022

yxlao Aug 6, 2022 •

edited

Loading

yxlao Aug 6, 2022

ZhengyuDiao left a comment •

edited

Loading

ZhengyuDiao commented Aug 7, 2022 •

edited

Loading

yxlao left a comment

Improve tensor point cloud io #5404

Improve tensor point cloud io #5404

Conversation

ZhengyuDiao commented Aug 4, 2022 • edited by germanros1987 Loading

update-docs bot commented Aug 4, 2022

yxlao left a comment

Choose a reason for hiding this comment

ZhengyuDiao left a comment

Choose a reason for hiding this comment

yxlao left a comment • edited Loading

Choose a reason for hiding this comment

yxlao Aug 6, 2022

Choose a reason for hiding this comment

yxlao Aug 6, 2022

Choose a reason for hiding this comment

yxlao Aug 6, 2022

Choose a reason for hiding this comment

yxlao Aug 6, 2022 • edited Loading

Choose a reason for hiding this comment

yxlao Aug 6, 2022

Choose a reason for hiding this comment

ZhengyuDiao left a comment • edited Loading

Choose a reason for hiding this comment

ZhengyuDiao commented Aug 7, 2022 • edited Loading

yxlao left a comment

Choose a reason for hiding this comment

ZhengyuDiao commented Aug 4, 2022 •

edited by germanros1987

Loading

yxlao left a comment •

edited

Loading

yxlao Aug 6, 2022 •

edited

Loading

ZhengyuDiao left a comment •

edited

Loading

ZhengyuDiao commented Aug 7, 2022 •

edited

Loading