pytorch3d icon indicating copy to clipboard operation
pytorch3d copied to clipboard

Errors in NeRF implementation

Open rakhimovv opened this issue 4 years ago • 12 comments

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

rakhimovv avatar Oct 06 '21 14:10 rakhimovv

up

rakhimovv avatar Oct 30 '21 01:10 rakhimovv

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.

github-actions[bot] avatar Nov 29 '21 05:11 github-actions[bot]

up

rakhimovv avatar Nov 30 '21 01:11 rakhimovv

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.

github-actions[bot] avatar Dec 30 '21 05:12 github-actions[bot]

This issue was closed because it has been stalled for 5 days with no activity.

github-actions[bot] avatar Jan 04 '22 05:01 github-actions[bot]

This issue was closed because it has been stalled for 5 days with no activity.

github-actions[bot] avatar Feb 13 '22 05:02 github-actions[bot]

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.

github-actions[bot] avatar Mar 20 '22 05:03 github-actions[bot]

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.

github-actions[bot] avatar Apr 21 '22 05:04 github-actions[bot]

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.

github-actions[bot] avatar May 26 '22 05:05 github-actions[bot]

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.

github-actions[bot] avatar Jun 26 '22 05:06 github-actions[bot]

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.

github-actions[bot] avatar Jul 28 '22 05:07 github-actions[bot]

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!

sergeyprokudin avatar Aug 12 '22 07:08 sergeyprokudin