mitsuba3 icon indicating copy to clipboard operation
mitsuba3 copied to clipboard

BoundingSphere/Box Python binding ambiguity

Open dvicini opened this issue 2 years ago • 5 comments

Hi,

I came across an edge case for BoundingBox3f and BoundingSphere3f in Python. If we first set the llvm_ad_rgb variant, changing to scalar_rgb will not recover the correct bindings for scalar mode (and vice versa). I came across this when running the Mitsuba test with llvm_ad_rgb set as a default variant. In this case, the bounding shape tests fail due to this incompatibility.

The problem is that the bounding shapes have a ray intersection function that is templated, and the binding differ between scalar mode and the scalar versions bound for the vectorized variants, e.g., ScalarBoundingSphere3f.

The following snippet illustrates the issue. The type mi.scalar_rgb.BoundingSphere3f will resolve to mi.llvm_ad_rgb.ScalarBoundingSphere3f, which will not accept scalar rays anymore.

import mitsuba as mi
mi.set_variant('llvm_ad_rgb')
mi.set_variant('scalar_rgb')

bsphere = mi.BoundingSphere3f()
print(type(bsphere))
bsphere.ray_intersect(mi.Ray3f([-5, 0, 0], [1, 0, 0]))

Or alternatively, first loading the scalar_rgb module:

import mitsuba as mi
mi.set_variant('scalar_rgb')
bsphere = mi.BoundingSphere3f()
mi.set_variant('llvm_ad_rgb')

bsphere = mi.ScalarBoundingSphere3f()
print(type(bsphere))
bsphere.ray_intersect(mi.Ray3f([-5, 0, 0], [1, 0, 0]))

Not sure what's the best solution here that doesn't require too many other changes. The Problem is inherently that we are trying to bind BoundingSphere<Point<float, 3>> twice, but with different arguments for the ray_intersect function. It seems that to properly solve this, the type of Ray would need to be a template parameter of BoundingSphere itself, rather than the function. Alternatively, maybe we could inject the missing function if we detect that the class was bound previously. Not sure how easy it would be to do that though

dvicini avatar Apr 18 '23 11:04 dvicini

Nice catch! In that case could we bind a lambda that routes the function call to the right template at run time? (so that we don't bind a different function depending on the variant)

Speierers avatar Apr 18 '23 12:04 Speierers

Interesting idea. We would still need some mechanism to deal with the fact that the bindings might be called in either order. Depending on the Python code, we might either bind scalar_rgb.BoundingSphere3f or llvm_ad_rgb.ScalarBoundingSphere3f first.

One "brute force" solution could be to keep a central list/map of py::class_ objects with aliases. In that case, the second time the binding is invoked, we could retrieve the previously created py::class_ and simply add the overload definition to it? It feels like there should be some kind of built in Pybind11 mechanism for this though and I am not a fan of adding more global state...

The problem seems to be specific to these bounding shapes, as far as I can tell these are the only classes with aliases and different method implementations.

dvicini avatar Apr 18 '23 12:04 dvicini

We could also overload the ray_intersect method so that it can handle both Ray3f and ScalarRay3f. Or did I misunderstand the issue here?

Speierers avatar Apr 18 '23 12:04 Speierers

Yes, that would be the goal. But the question is where and when do we do that. The problem is that the variants are compiled independently, so we don't necessarily know which other variants are enabled. And as I said, we also don't necessarily have control over module import order. Ideally, the binding code would detect and extend a previously created binding. Currently, it only detects the binding and creates an alias.

dvicini avatar Apr 18 '23 13:04 dvicini

I suppose it should be possible to bind a "private" static ray intersection function in each variant compilation unit (e.g. BBox.__ray_intersect(bbox, ray) and then use the following at runtime to fetch it:

[](const BBox &self, const Ray &ray) {
      // Go fetch the intersection routine from the other compilate unit if necessary
      auto mitsuba_variant_lib = py::module::import("mitsuba.VARIANT_NAME");
      auto func = mitsuba_variant_lib.attr('BoundingBox3f').attr('__ray_intersect');
      return func(bbox, ray);
}

Speierers avatar Apr 18 '23 13:04 Speierers