Skip to content

Initialize gradient accumulation tensors before UT projection path#608

Merged
harrism merged 1 commit into
openvdb:mainfrom
harrism:issue-279-ut-gradient-accumulation
Apr 15, 2026
Merged

Initialize gradient accumulation tensors before UT projection path#608
harrism merged 1 commit into
openvdb:mainfrom
harrism:issue-279-ut-gradient-accumulation

Conversation

@harrism

@harrism harrism commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Move lazy initialization of gradient accumulation tensors (_accumulated_gradient_step_counts, _accumulated_mean_2d_gradient_norms, _accumulated_max_2d_radii) to before the Unscented Transform early return in _do_projection
  • Add test proving the UT path now initializes gradient state when accumulate_mean_2d_gradients=True

Root Cause

The UT projection path in _do_projection returned early (line ~1500) without lazily initializing the gradient accumulation tensors. The lazy init block (lines ~1524-1547) only ran for the ANALYTIC path. This left accumulated_gradient_step_counts and accumulated_mean_2d_gradient_norms as None for the entire training run when using OpenCV camera models, causing downstream consumers to crash.

Fix

Move the lazy init block before the _use_ut() check. The UT kernel (_C.project_gaussians_ut_fwd) does not accumulate into these tensors, so they remain zero — a correct "no gradient data" representation. Full UT gradient accumulation support can follow as a separate enhancement.

See: openvdb/fvdb-reality-capture#279

Test plan

  • New test test_ut_projection_initializes_gradient_accumulation fails before fix (asserting None), passes after
  • All 8 TestGaussianCameraApi tests pass with zero regressions

🤖 Generated with Claude Code

@harrism harrism requested a review from a team as a code owner April 15, 2026 04:15
@harrism harrism requested review from areidmeyer and swahtz April 15, 2026 04:15
The Unscented Transform (UT) projection path in _do_projection returned
early without lazily initializing _accumulated_gradient_step_counts and
_accumulated_mean_2d_gradient_norms. This left these tensors as None
when accumulate_mean_2d_gradients was True, causing downstream consumers
(e.g., refinement in fvdb-reality-capture) to crash.

Move the lazy initialization block before the UT early return so the
tensors are always created when gradient accumulation is enabled,
regardless of which projection method is used. The UT kernel does not
accumulate into these tensors, so they remain zero — correct "no data"
representation until full UT gradient accumulation support is added.

See: openvdb/fvdb-reality-capture#279

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
@harrism harrism force-pushed the issue-279-ut-gradient-accumulation branch from b070048 to dcf3494 Compare April 15, 2026 04:16
@harrism harrism requested a review from fwilliams April 15, 2026 04:18
@harrism harrism added the bug Something isn't working label Apr 15, 2026
@harrism harrism added the Gaussian Splatting Issues related to Gaussian splattng in the core library label Apr 15, 2026
@harrism harrism requested a review from Copilot April 15, 2026 04:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a missing side effect in GaussianSplat3d._do_projection where the UNSCENTED (UT) projection early-return bypassed lazy initialization of gradient-accumulation tensors, leaving them as None even when gradient accumulation was enabled.

Changes:

  • Move lazy initialization of _accumulated_gradient_step_counts, _accumulated_mean_2d_gradient_norms, and _accumulated_max_2d_radii to occur before the UT early-return path in _do_projection.
  • Add a unit test asserting that the UNSCENTED projection path initializes gradient accumulation state when accumulate_mean_2d_gradients=True.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
fvdb/gaussian_splatting.py Ensures UT projection path runs gradient-accumulation lazy init before returning early.
tests/unit/test_gaussian_splat_3d.py Adds regression test covering UT-path initialization of accumulated gradient tensors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@harrism harrism moved this to In progress in fvdb-realitycapture Apr 15, 2026
@harrism harrism moved this from In progress to In review in fvdb-realitycapture Apr 15, 2026

@swahtz swahtz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@harrism harrism merged commit 9870f7b into openvdb:main Apr 15, 2026
42 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in fvdb-realitycapture Apr 15, 2026
@harrism harrism deleted the issue-279-ut-gradient-accumulation branch April 15, 2026 06:21
fwilliams added a commit that referenced this pull request Apr 15, 2026
Resolve conflict in fvdb/gaussian_splatting.py: accept main's structural
change from PR #608 (move UT projection block after gradient accumulation
init), then re-apply our ut->unscented rename and empty shN fix for
degree-0 SH evaluation.

Signed-off-by: Francis Williams <francis@fwilliams.info>
Made-with: Cursor
@swahtz swahtz added this to the v0.5 milestone Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Gaussian Splatting Issues related to Gaussian splattng in the core library

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants