search

facebookresearch / pytorch3d

PyTorch3D is FAIR's library of reusable components for deep learning with 3D data
https://pytorch3d.org/
Other
8.71k stars 1.31k forks source link

Errors in NeRF implementation #868

Open rakhimovv opened 2 years ago

rakhimovv commented 2 years ago

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 commented 2 years ago

up

github-actions[bot] commented 2 years ago

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.

rakhimovv commented 2 years ago

up

github-actions[bot] commented 2 years ago

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] commented 2 years ago

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

github-actions[bot] commented 2 years ago

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

github-actions[bot] commented 2 years ago

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] commented 2 years ago

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] commented 2 years ago

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] commented 2 years ago

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] commented 2 years ago

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.

sergeyprokudin commented 2 years ago

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!