mitsuba-renderer / drjit

Dr.Jit — A Just-In-Time-Compiler for Differentiable Rendering
BSD 3-Clause "New" or "Revised" License
603 stars 45 forks source link

Fixes the conversion of Matrix43f and similar types to NumPy #193

Closed dvicini closed 1 year ago

dvicini commented 1 year ago

There is a small bug when converting Matrix43f (e.g., an RGB Mueller matrix) to NumPy.

In the current master, such a conversion doesn't quite work. Printing an identity matrix produces the following output:

>>> import drjit as dr
>>> import numpy as np
>>> print(np.array(dr.llvm.ad.Matrix43f(1.0)))
[[[[1. 0. 0. 0.]
   [1. 0. 0. 0.]
   [1. 0. 0. 0.]
   [0. 0. 0. 1.]]

  [[1. 0. 0. 0.]
   [1. 0. 0. 0.]
   [0. 0. 0. 1.]
   [0. 0. 0. 1.]]

  [[1. 0. 0. 0.]
   [0. 0. 0. 1.]
   [0. 0. 0. 1.]
   [0. 0. 0. 1.]]]]
>>> 

But it should be

[[[[1. 0. 0. 0.]
   [0. 1. 0. 0.]
   [0. 0. 1. 0.]
   [0. 0. 0. 1.]]

  [[1. 0. 0. 0.]
   [0. 1. 0. 0.]
   [0. 0. 1. 0.]
   [0. 0. 0. 1.]]

  [[1. 0. 0. 0.]
   [0. 1. 0. 0.]
   [0. 0. 1. 0.]
   [0. 0. 0. 1.]]]]

This commit fixes this by slightly generalizing the existing stride recomputation.

wjakob commented 1 year ago

LGTM, thanks! All of this special-casing around matrices will disappear in the nanobind_v2 rewrite. (they switched to row-major order and are therefore just like any another recursively nested array).

dvicini commented 1 year ago

Sounds good, I was already expecting that but thought it's still good to have this preliminary fix in the meantime :)