[Open for discussion] Rework/remove density_fn to support Gaussians.

[Mxbonn]

issue

The density_fn in class Field takes different arguments than get_density (it takes positions instead of RaySamples).
As a result, it is not possible to use the ProposalNetworkSampler with networks that do not take positions as arguments. One example is: MipNerf360, which uses Gaussians in the proposal network.
More generally, I don't think it makes sense to have a function that is nearly identical to get_density, but that takes in non-standard arguments.

Proposed fix

remove/deprecate density_fn. It can nearly always be replaced with get_density. In places where it is not straightforward to replace it with get_density (e.g. the occupancy grid) use a custom wrapper function.

Problem with the fix

It is a breaking change, but a breaking change will be necessary if you want to support e.g. mipnerf360.
I don't know the policy for breaking changes, but one could always not immediately remove the density_fn and put a deprecation warning on it.

[Ilyabasharov]

@Mxbonn Fully agree! #2242

[brentyi]

Does anybody have a sense for how many (public) external repositories would be impacted by this?

[jkulhanek]

I believe external repos should fix an NS version. The development breaks lots of stuff between versions so I think its fine. Perhaps keeping a deprecation warning for a couple of versions would be nice.

[jkulhanek]

Do you agree to merge this as is, or should we add some deprecation warnings?

[brentyi]

I believe external repos should fix an NS version.

This seems like a reasonable thing to recommend!

Do you agree to merge this as is, or should we add some deprecation warnings?

I skimmed a few repos, and at the very least it seems like the K-Planes extension will break from this PR:
https://github.com/Giodiro/kplanes_nerfstudio/blob/cc5867b14697732bef4dd1de5ba49fafd419b312/kplanes/kplanes.py#L189-L214

That said, I'm okay with merging without deprecation warnings. If we do this we can follow up with an official recommendation for external methods to pin a nerfstudio version/commit in the docs, and perhaps make PRs for fixing versions of external methods that we know are out there? Does this make sense? cc also @kerrj @tancik

[Mxbonn]

So anyone willing to make a decision on this?

[jkulhanek]

@brentyi , do you agree with merging?

[brentyi]

Yes, this has my support!

For moving forward, how about:

• Dropping in an error message if density_fn() is called on the field, which communicates that the method was removed after version 0.3.4. I don't think we can guarantee this always, but for this PR it seems like the effort required would be low.
• Updating the docs + README in the method template repo to recommend pinning nerfstudio versions.
• Making a few PRs or issues for external repos that would be obviously broken.

Any opposition @Mxbonn @jkulhanek? If not I'm also happy to help with some of this.

[jkulhanek]

I agree. I will add the error message soon.