bennyguo / instant-nsr-pl

Neural Surface reconstruction based on Instant-NGP. Efficient and customizable boilerplate for your research projects. Train NeuS in 10min!
MIT License
856 stars 84 forks source link

Add indexing parameter to torch meshgrid #49

Closed MvWouden closed 1 year ago

MvWouden commented 1 year ago

Running the code with a recent version of PyTorch leads to:

UserWarning: torch.meshgrid: in an upcoming release, it will be required to pass the indexing argument.
(Triggered internally at ..\aten\src\ATen\native\TensorShape.cpp:3191.)
  return _VF.meshgrid(tensors, **kwargs)  # type: ignore[attr-defined]

Next to that, the default indexing mode of np.meshgrid and torch.meshgrid are different from each other (xy vs ij), see the torch meshgrid documentation which also includes a warning for this.

I've added the xy indexing parameter to the torch.meshgrid call which will:

  1. Ensure the numpy and torch call to meshgrid have the same indexing mode (since we already have indexing='xy' for np.meshgrid), even if future changes are made to the defaults (as suggested in the torch docs).
  2. Resolve the UserWarning that is thrown.

This issue also describes the problem.

If we'd like to use e.g. indexing='ij' or something else needs to be changed, please let me know.

bennyguo commented 1 year ago

Thank you for pointing out this potential problem. I think we better use indexing='ij' here to have the correct coordinate system for the 3D mesh. Note that this is different from the np.meshgrid in ray_utils.py, which ensures the correct coordinate system for the 2D image pixels. Could you please change the indexing method to ij?

MvWouden commented 1 year ago

Sure! I'll do that today.

MvWouden commented 1 year ago

@bennyguo I've changed it to be indexing='ij', so it should be good now. Note that I did not change np.meshgrid in ray_utils.py given your explanation.

bennyguo commented 1 year ago

Looks good! Merged.