kaolin icon indicating copy to clipboard operation
kaolin copied to clipboard

Fix user backend flag bad assignment

Open LvisRoot opened this issue 2 years ago • 2 comments

Fixes user_requested_backend bad assignment when backend is not specified

  • Changed argument backend to backend_name
  • Check backend_name to set user_requested_backend instead of the created backend object

LvisRoot avatar Apr 10 '23 10:04 LvisRoot

Hi @LvisRoot , what are you trying to resolve?

I can see some backend_name in the code base but all the constructors use backend:

  • from_camera_pose: https://github.com/NVIDIAGameWorks/kaolin/blob/master/kaolin/render/camera/extrinsics.py#L278
  • from_lookat: https://github.com/NVIDIAGameWorks/kaolin/blob/master/kaolin/render/camera/extrinsics.py#L331
  • from_view_matrix: https://github.com/NVIDIAGameWorks/kaolin/blob/master/kaolin/render/camera/extrinsics.py#L417

Since it's not inconsistent (I did find an inconsistency here https://github.com/NVIDIAGameWorks/kaolin/blob/master/kaolin/render/camera/extrinsics.py#L245), and is not causing any known issue I would suggest to avoid breaking change.

Caenorst avatar Apr 13 '23 20:04 Caenorst

Hi @Caenorst , thanks for the remark, here's what I'm trying to fix:

The issue is that in from_view_matrix: https://github.com/NVIDIAGameWorks/kaolin/blob/a1b2ad427a580c10f4a3667c31565f244a911aaf/kaolin/render/camera/extrinsics.py#L474 both the passed backend argument (which is a backend name as a string) has the same variable name as the backend instanciated in the function.

So extrinsics._shared_fields['user_requested_backend'] gets always set, since you just constructed a backend object and is always not None, regardless of a backend name being passed to the function or not. This breaks auto backend selection for optimization when you run, say camera.requires_grad_(True) if you constructed the extrinsics with from_view_matrix.

I see changing the argument name would break other stuff, so I'll create an aux variable to store the backend instance, keep the backend argument name.

would you do it differently?

LvisRoot avatar Apr 14 '23 07:04 LvisRoot