georghess / neurad-studio

[CVPR2024] NeuRAD: Neural Rendering for Autonomous Driving
https://research.zenseact.com/publications/neurad/
Apache License 2.0
356 stars 25 forks source link

Chamfer distance, why is min_dist pow(2) and also why does no chunk size does not normalize ? #55

Open JulienStanguennec-Leddartech opened 1 week ago

JulienStanguennec-Leddartech commented 1 week ago

Hi,

When I compare with other definition and results provided by other implementation it seems that chamfer_distance does not seems to use .pow(2) while it does in Neurad implementation. Maybe it's more a question than a bug...

In the code for chamfer distance, when I replace the calculation for dist in _chamfer_dist to remove pow(2) (https://github.com/georghess/neurad-studio/blob/main/nerfstudio/utils/math.py#L770C1-L771C107)

This:

    def _chamfer_dist(source, target):
        dist = torch.cdist(source, target, p=2, compute_mode="use_mm_for_euclid_dist_if_necessary").pow(2)

Becomes this:

    def _chamfer_dist(source, target):
        dist = torch.cdist(source, target, p=2, compute_mode="use_mm_for_euclid_dist_if_necessary")

I get results that are inline with for example the point_cloud_utils (https://fwillims.info/point-cloud-utils/sections/shape_metrics/)

import point_cloud_utils as pcu
import torch
device =  torch.device('cuda' if torch.cuda.is_available() else 'cpu')
xyz1 = torch.rand(100000, 3, device=device)
xyz2 = xyz1 + 0.1
print(f"original_chamfer distance without normalization: {chamfer_distance_original(xyz1,xyz2, normalize_with_target=False, chunk_size=1000)}")
print(f"original_chamfer distance with normalization: {chamfer_distance_original(xyz1,xyz2, normalize_with_target=True, chunk_size=1000)}")
print(f"modified chamfer distance without normalization: {chamfer_distance_modified(xyz1,xyz2, normalize_with_target=False, chunk_size=1000)}")
print(f"modified chamfer distance with normalization: {chamfer_distance_modified(xyz1,xyz2, normalize_with_target=True, chunk_size=1000)}")
print(f"point_cloud_utils chamfer distance: {pcu.chamfer_distance(xyz1.float().cpu().numpy(),xyz2.float().cpu().numpy())}")

Gives: original_chamfer distance without normalization: 269.503662109375 original_chamfer distance with normalization: 0.0026950370520353317 modified chamfer distance without normalization: 5048.60107421875 modified chamfer distance with normalization: 0.050485990941524506 point_cloud_utils chamfer distance: 0.05048602819442749

Question: is there a choice that drove you to use the square value of dist for chamfer distance calculation ?

Also, if chamfer distance is provided without chunk, normalization has no effect:- https://github.com/georghess/neurad-studio/blob/main/nerfstudio/utils/math.py#L777

    if chunk_size is None:
        min_dist_source_to_target, min_dist_target_to_source = _chamfer_dist(source_pc, target_pc)

The chamfer distance function is "partialled" with chunk_size=1000 and normalization=True so this function is never called without chunk_size=None. So probably that's why it did not came out.

georghess commented 6 days ago

Hi,

Thanks for bringing this up! As I remember it, we used the squared distance to have the same unit as for our depth metric (median squared depth error). We used median squared depth error mainly to have comparable numbers to UniSim. However, this should probably be clarified.

As for the normalization not having an impact when not chunking, that indeed looks like a bug. I'll fix it as soon as I have some spare time.

anishmadan23 commented 6 days ago

Given the squared median depth error and similarly for Chamfer distance, the numbers reported would be in square meters, and not in meters(as reported in the paper), right?

georghess commented 6 days ago

@anishmadan23 yes, I think our table caption might be missing a ^2 in that case

anishmadan23 commented 5 days ago

Congrats on your SplatAD work, the results look great. I wanted to confirm if the SplatAD metrics (chamfer distance and median depth) are also computed using the squared term?

georghess commented 5 days ago

Thank you @anishmadan23! Yes, we use the same code to calculate the metrics so the numbers are comparable.

anishmadan23 commented 5 days ago

gotcha! Thanks for the prompt response. When can we expect the code release for SplatAD btw?

JulienStanguennec-Leddartech commented 4 days ago

@georghess, thanks for the clarification, it make sens. As anishmadan23 mentionned, congratulation for SplatAD ! A great contribution once again. Do you plan to release the code at some point ? Maybe we could add support for wod dataset :)

carlinds commented 4 days ago

@JulienStanguennec-Leddartech All CUDA-code will be released in our gsplat-fork once the paper has been peer reviewed and published. The model itself will likely be released either as a model in neurad-studio, or as an external method (like we did with unisim).

JulienStanguennec-Leddartech commented 4 days ago

Very good news ! Is it possible to know which conference it is ? As said above, if you want us to do the evaluation on waymo dataset, do not hesitate. As the rolling shutter direction is different in waymo dataset, it could be interesting to see if it has an impact.

carlinds commented 2 days ago

Unfortunately, the conference has some policies regarding that. That would be interesting, we will reach out once the code is released :)