Enable pickling for tensor and tensor based geometry #5509
Conversation
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
|
LGTM after adding python unit tests. |
…pen3D into yueci/tgeometry-pickle
|
.gitignore
Outdated
| @@ -90,3 +90,6 @@ docs/Doxyfile | |||
| docs/getting_started.rst | |||
| docs/docker.rst | |||
| docs/tensorboard.md | |||
|
|
|||
| # test | |||
| *.pkl | |||
| utility::LogError( | ||
| "Invalid state! Expecting a tuple of size 2."); | ||
| } | ||
| const Device& device = t[0].cast<Device>(); |
There was a problem hiding this comment.
Use Device::IsAvailable(). For example, when pickling, we might store CUDA:1, but another computer may only have CUDA:0. Same for others.
cpp/pybind/core/device.cpp
Outdated
| [](py::tuple t) { | ||
| if (t.size() != 2) { | ||
| utility::LogError( | ||
| "Invalid state! Expecting a tuple of size " |
There was a problem hiding this comment.
Add a comment: "Cannot unpickle Device." It is useful for the user to know which class went wrong. Same for others.
cpp/pybind/core/tensor.cpp
Outdated
| const Device& device = t[0].cast<Device>(); | ||
| if (device.IsCUDA() && !core::cuda::IsAvailable()) { | ||
| utility::LogWarning( | ||
| "CUDA is not available, tensor will be " |
There was a problem hiding this comment.
Change this comment as well. Same for others.
python/test/t/geometry/test_image.py
Outdated
| @pytest.mark.parametrize("device", list_devices()) | ||
| def test_pickle(device): | ||
| img = o3d.t.geometry.Image(o3c.Tensor.ones((10, 10, 3), o3c.uint8, device)) | ||
| pickle.dump(img, open("img.pkl", "wb")) |
There was a problem hiding this comment.
Use
import tempfile
with tempfile.TemporaryDirectory() as temp_dir:
pickle_path = xxxSame for others.
|
Great work. It would be very helpful to add a brief documentation example on the Tensor page. |
Sounds good, I will add it later. |
This PR makes
Tensor,TensorMapand tensor based geometry pickling.Current behaviours:
This change is