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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix v_mean3d in project_g and v_conic calculation in *rasterize_backward #139

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

jb-ye
Copy link
Collaborator

@jb-ye jb-ye commented Mar 1, 2024

Previous #136 doesn't fully solve the problem and in fact caused the splatfacto performance degradation. This is because there are two bugs that canceling each other in training GS

  • the first bug which is solved in Fix backprop grad of cov2d / scales and unit tests #136 is missing a 0.5 multiplier when computing v_cov2d from v_conics.y
  • the second bug which is solved in this PR is to remove an extra 0.5 multiplier when computing v_conics.y in rasterize_backward call

Effectively they cancel out each other to some degree in splafacto when updating cov2d.

By only fixing the first bug but not the second, the performance of splatfacto significantly degraded. In an example dataset ichsan2895 provided me, PSNR degrades from 25.97 to 25.40. Post this PR, the PSNR performance is restored.

There is a third bug in updating v_mean3d which @vye16 mentioned to me earlier. This change also fix the bug and no side effect is observed (either improvement or regression).

Copy link
Collaborator

@kerrj kerrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@kerrj kerrj merged commit 94cbd12 into nerfstudio-project:main Mar 4, 2024
2 checks passed
oseiskar added a commit to SpectacularAI/gsplat that referenced this pull request Mar 19, 2024
…ze_backward_kernel (nerfstudio-project#139)

Ported the relevant part of commit 94cbd12 to this branch.

Co-authored-by: J.Y <132313008+jb-ye@users.noreply.github.com>
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

2 participants