Open bibbygoodwin opened 2 years ago
Hi @bibbygoodwin!
Thank you for your pull request and welcome to our community.
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed
. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
I believe that there is a bug in the Co3dDataset class, in the
_get_pytorch3d_camera
, in the case where the dataset is loaded withself.box_crop=True
and without resizing (i.e.self.image_height is None or self.image_width is None
). The bug comes from theout_size
parameter not being set appropriately under these conditions.The code under the condition at https://github.com/facebookresearch/co3d/blob/7ee9f5ba0b87b22e1dfe92c4d2010cb14dd467a6/dataset/co3d_dataset.py#L516 returns
out_size
as the original image size, in cases whereself.image_height is None or self.image_width is None
. However, ifself.box_crop=True
, theout_size
should be the size of the crop, not the whole image.Plotting the back-projected point clouds under various resize/crop settings demonstrates the error - below, the purple point cloud is from a dataset loaded without cropping (either
self.image_height=None
or with some value e.g.self.image_height=256
-- both, correctly, lead to the same back-projection, though with different numbers of points). The blue point cloud is from a cropped and resized dataset (self.box_crop=True
,self.image_height=256
,self.image_width=256
). This back-projects to overlap appropriately with the full point cloud. The red point cloud is from the problematic combination (self.box_crop=True
,self.image_height=None
,self.image_width=None
), and can be seen to not overlap with either of the other point clouds.By contrast, the following show two views with the bugfix (green point cloud comes from dataset with same crop-and-no-resize conditions as the red point cloud before). It can be seen that the green point cloud is now fully aligned with the other back-projected point clouds.
I believe that the no-crop-or-resize format is being quite commonly used by users of the dataset, so I hope that this pull request will prove useful.
Thanks for reviewing.