-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Errors in NeRF implementation #868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
up |
Summary: - Old NDC convention had xy coords in [-1,1]x[-1,1] - New NDC convention has xy coords in [-1, 1]x[-u, u] or [-u, u]x[-1, 1] where u > 1 is the aspect ratio of the image. This PR fixes the NDC raysampler to use the new convention. Partial fix for #868 Pull Request resolved: fairinternal/pytorch3d#29 Reviewed By: davnov134 Differential Revision: D31926148 Pulled By: bottler fbshipit-source-id: c6c42c60d1473b04e60ceb49c8c10951ddf03c74
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! |
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:
A similar problem also persists here when passing arguments:
pytorch3d/projects/nerf/nerf/raysampler.py
Lines 162 to 166 in 9585a58
If the fix is applied, then grid_sample should be fixed to account for it (the longer side should be divided by
u
)pytorch3d/projects/nerf/nerf/utils.py
Line 51 in 9585a58
also,
align_corners
should beFalse
since the in NDC convention, the pixel location corresponds to its centerpytorch3d/projects/nerf/nerf/utils.py
Lines 53 to 58 in 6d36c1e
Problem 2.
deltas in
_get_densities
do not take into account the norm ofray_directions
pytorch3d/projects/nerf/nerf/implicit_function.py
Lines 109 to 115 in 9585a58
this does not lead to error since the
ray_directions
were normalized inNeRFRaysampler
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 ofray_directions
when calculating densities like it is done in an original implementation https://github.com/bmild/nerf/blob/20a91e764a28816ee2234fcadb73bd59a613a44c/run_nerf.py#L123Problem 3.
NeRFRaysampler does not correctly work with batch_size > 1
e.g., here
pytorch3d/projects/nerf/nerf/raysampler.py
Lines 349 to 357 in 8fa438c
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:
pytorch3d/projects/nerf/nerf/raysampler.py
Line 329 in 8fa438c
pytorch3d/projects/nerf/nerf/raysampler.py
Lines 338 to 347 in 8fa438c
here, e.g., also no batch dimension in data actually going to the function
pytorch3d/projects/nerf/nerf/nerf_renderer.py
Lines 293 to 294 in 8fa438c
pytorch3d/projects/nerf/nerf/nerf_renderer.py
Lines 210 to 211 in 8fa438c
The text was updated successfully, but these errors were encountered: