pytorch3d
pytorch3d copied to clipboard
Errors in NeRF implementation
Hello, first of all, thanks for the beautiful implementation!
However, certain things seem to be buggy. Please correct me if I am wrong:
Problem 1.
NDCGridRaysampler does not work properly with rectangular images. Since the NDC convention assumes that the XY coordinates lie in [-1, 1] x [u, u] or ([-u, u] x [-1, 1]) depending on which size is shorter.
The simple fix I expect would be:
from pytorch3d.renderer.mesh.rasterize_meshes import pix_to_non_square_ndc
min_x = pix_to_non_square_ndc(image_width - 1, image_width, image_height)
max_x = pix_to_non_square_ndc(0, image_width, image_height)
min_y = pix_to_non_square_ndc(image_height - 1, image_height, image_width)
max_y = pix_to_non_square_ndc(0, image_height, image_width)
A similar problem also persists here when passing arguments: https://github.com/facebookresearch/pytorch3d/blob/9585a58d10cb2efcd159b058fa4af914203c1d0d/projects/nerf/nerf/raysampler.py#L162-L166
If the fix is applied, then grid_sample should be fixed to account for it (the longer side should be divided by u)
https://github.com/facebookresearch/pytorch3d/blob/9585a58d10cb2efcd159b058fa4af914203c1d0d/projects/nerf/nerf/utils.py#L51
also, align_corners should be False since the in NDC convention, the pixel location corresponds to its center
https://github.com/facebookresearch/pytorch3d/blob/6d36c1e2b00d63d994fd4dd7d0b740f1922443df/projects/nerf/nerf/utils.py#L53-L58
Problem 2.
deltas in _get_densities do not take into account the norm of ray_directions
https://github.com/facebookresearch/pytorch3d/blob/9585a58d10cb2efcd159b058fa4af914203c1d0d/projects/nerf/nerf/implicit_function.py#L109-L115
this does not lead to error since the ray_directions were normalized in NeRFRaysampler
but this leads to another problem: since the ray_directions were normalized, ray_bundle_to_ray_points will now produce points that lie before the near plane if, e.g., I pass ray_lengths = near (so the depth values are not actually depths)
it seems to me it is better to avoid ray_directions normalization and take into account the norm of ray_directions when calculating densities like it is done in an original implementation https://github.com/bmild/nerf/blob/20a91e764a28816ee2234fcadb73bd59a613a44c/run_nerf.py#L123
Problem 3.
NeRFRaysampler does not correctly work with batch_size > 1
e.g., here https://github.com/facebookresearch/pytorch3d/blob/8fa438cbda382602ad64afac5713f4e7e0461f88/projects/nerf/nerf/raysampler.py#L349-L357 causes the case when the rays originally pointing to the different batch idx will now point to the same batch_idx
also, some index bound errors would be here: https://github.com/facebookresearch/pytorch3d/blob/8fa438cbda382602ad64afac5713f4e7e0461f88/projects/nerf/nerf/raysampler.py#L329 https://github.com/facebookresearch/pytorch3d/blob/8fa438cbda382602ad64afac5713f4e7e0461f88/projects/nerf/nerf/raysampler.py#L338-L347
here, e.g., also no batch dimension in data actually going to the function https://github.com/facebookresearch/pytorch3d/blob/8fa438cbda382602ad64afac5713f4e7e0461f88/projects/nerf/nerf/nerf_renderer.py#L293-L294 https://github.com/facebookresearch/pytorch3d/blob/8fa438cbda382602ad64afac5713f4e7e0461f88/projects/nerf/nerf/nerf_renderer.py#L210-L211
up
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
up
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
This issue was closed because it has been stalled for 5 days with no activity.
This issue was closed because it has been stalled for 5 days with no activity.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
Hi folks, NeRF implementation for non-square images indeed seems to be the problem. E.g. I cannot reproduce 'fern' dataset results, as also mentioned here by other researchers. Any plans on investigating the issues any time soon?..
Again, thanks for the great repo!