Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call rescale_output_resolution before early return #2897

Merged
merged 6 commits into from
Feb 13, 2024

Conversation

pierotofy
Copy link
Contributor

@pierotofy pierotofy commented Feb 10, 2024

Hello 馃憢

While looking through the splatfacto implementation, I noticed that in the case of an early exit in the forward pass due to a zero radii sum the camera does not get rescaled back to its original dimensions. I think this might be a mistake? I haven't found a dataset where the condition occurs, but it seems correct to make sure the camera is rescaled back before returning.

@chungmin99
Copy link
Contributor

in the case of an early exit in the forward pass due to a zero radii sum the camera does not get rescaled back

Thank you for catching it! This condition can actually happen quite frequently when there's no gaussians in front (e.g., due to cropping + moving the camera away from the cropbox).


The fix looks great, but I'm just wondering if we can remove duplicate code, by moving the camera rescaling logic (L745, and L763) before the radii check (L738):

tile_bounds,
) # type: ignore
if (self.radii).sum() == 0:
rgb = background.repeat(int(camera.height.item()), int(camera.width.item()), 1)
depth = background.new_ones(*rgb.shape[:2], 1) * 10
accumulation = background.new_zeros(*rgb.shape[:2], 1)
# rescale the camera back to original dimensions before returning
camera.rescale_output_resolution(camera_downscale)
return {"rgb": rgb, "depth": depth, "accumulation": accumulation}

And we can change rgb = background.repeat(int(camera.height.item()), int(camera.width.item()), 1) to rgb=background.repeat(H, W, 1).

... unless there's any dependence on the camera object I'm missing between L750-760.

@jb-ye
Copy link
Collaborator

jb-ye commented Feb 13, 2024

Thanks for catching the issue. Instead of adding this line to early return, could you move the line later to an earlier position: right after project_gaussian() call. We no longer need camera after that.

@jb-ye
Copy link
Collaborator

jb-ye commented Feb 13, 2024

You can test your code by enabling --pipeline.model.num_downscales=2 because currently it is set to zero by default.

@jb-ye
Copy link
Collaborator

jb-ye commented Feb 13, 2024

lgtm

@kerrj kerrj enabled auto-merge (squash) February 13, 2024 23:11
@kerrj kerrj merged commit 45db2bc into nerfstudio-project:main Feb 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants