Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix user backend flag bad assignment #706

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LvisRoot
Copy link

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

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

Signed-off-by: lvisroot <[email protected]>
@LvisRoot LvisRoot force-pushed the csmitt/fix_extrinsics_constructor_user_specified_backend_flag branch from 5a07a55 to 0f3e934 Compare April 10, 2023 10:05
@Caenorst
Copy link
Collaborator

Hi @LvisRoot , what are you trying to resolve?

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

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.

- Revert renaming of backend argument in `from_view_matrix`
- Create aux variable for the backend object
- Set `user_requested_backend` using the backend name argument

Signed-off-by: lvisroot <[email protected]>
@LvisRoot
Copy link
Author

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

The issue is that in from_view_matrix:

extrinsics._shared_fields['user_requested_backend'] = backend is not None

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants