Skip to content

Conversation

@ericmehl
Copy link
Collaborator

@ericmehl ericmehl commented Dec 9, 2025

This adds a new lightweight buffer object to IECorePython which supports Python's buffer protocol.

The buffer can be obtained from numeric-based *VectorData types with read-only or read-write access. Read-only buffers are advantageous because they will not need to make copies of the underlying data that read-write buffer may need to do for TypedData copy-on-write functionality.

Types that act as a consumer for Python's buffer protocol include Python's native memoryview object and Numpy's ndarray object.

Checklist

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have tested my change(s) in the test suite, and added new test cases where necessary.
  • My code follows the Cortex project's prevailing coding style and conventions.

@ericmehl
Copy link
Collaborator Author

ericmehl commented Dec 9, 2025

I don't have a solution for the test failures in the Linux debug build, but I do have some information about it. If I comment out

#include "IECore/HalfTypeTraits.h"
Cortex will actually build and the tests that are failing on CI pass.

A bunch of others will break from removing that include, but it's something to go on. I also tried commenting out individual specializations in HalfTypeTraits.h, and at least among those that didn't lead to build failures, none helped with the tests.

I'm guessing it's some kind of optimization issue between debug and release builds, but beyond that hunch I'll have to keep poking at it and see what happens.

@ericmehl
Copy link
Collaborator Author

ericmehl commented Dec 9, 2025

Oh, one more piece of information. If I go back to only the first commit, 20550fb, the tests pass. If I simply add #include "TypeTraits.h" to VectorTypedDataBinding.cpp`, they fail.

Then if I remove one of the two pairs of specializations at

template<>
struct is_unsigned<half> : public false_type{};
template<>
struct is_unsigned<const half> : public false_type{};
template<>
struct is_unsigned<volatile half> : public false_type{};
template<>
struct is_unsigned<const volatile half> : public false_type{};

it passes again.

But if I try that trick with the full set of commits, where TypeTraits is actually used and not just included, the failures are still present.

@ericmehl
Copy link
Collaborator Author

It's a Christmas miracle! I think I've sorted out the errors we were getting on Linux debug builds with VectorData.HalfVectorData tests. My solution for now is in 650eb9b, though there may well be a better solution if we figure out more clearly what's happening.

It was easier to isolate the test down to constructing a v = IECore.HalfVectorData( [ 1.0, 2.0, 3.0 ] ) and testing that v[0] == 1.0. In fact, if you went into a c++ debugger or used some debug prints, you don't even need to do the test, just constructing the vector data results in bogus values. It's also a bit simpler to go to RB-10.6 branch and just add

namespace boost
{

template<>
struct is_arithmetic<half> : public true_type{};

};

to the top of VectorTypedDataBinding.inl.

That's enough to reproduce the error.

The thought train goes :

  1. In the VectorTypedDataBinding::VectorTypedDataFunctions constructor that accepts a list of values, we call
    boost::python::container_utils::extend_container( r->writable(), v );
  2. That extracts the value and adds it to the container at https://github.com/boostorg/python/blob/47d5bc76f69e20625214381c930a2fad5765e2b3/include/boost/python/suite/indexing/container_utils.hpp#L44
  3. The extraction, x(), in step 2 calls https://github.com/boostorg/python/blob/47d5bc76f69e20625214381c930a2fad5765e2b3/include/boost/python/extract.hpp#L67-L83
  4. python::detail::copy_ctor_mutates_rhs<T> is false, so result_type becomes typename call_traits<T>::param_type.
  5. That type is defined at https://github.com/boostorg/utility/blob/656b5397c527e321f1c7d2be4518833dc021da72/include/boost/detail/call_traits.hpp#L86-L91
  6. Track that a couple of template layers above that line in the same file and it's choosing between
template <typename T, bool small_>
struct ct_imp2
{
   typedef const T& param_type;
};

and

template <typename T>
struct ct_imp2<T, true>
{
   typedef const T param_type;
};
  1. half is smaller in size that void *, so we get the latter.
  2. But only if is_arithmetic<half>::value == true.
  3. Which it is now that we're including HalfTypeTraits.h.

A little more info :

  1. Modify Boost at https://github.com/boostorg/python/blob/47d5bc76f69e20625214381c930a2fad5765e2b3/include/boost/python/suite/indexing/container_utils.hpp#L44 to add auto a = x().
  2. The definition of extract_rvalue()() being used in step 3 above is at https://github.com/boostorg/python/blob/47d5bc76f69e20625214381c930a2fad5765e2b3/include/boost/python/extract.hpp#L176-L186.
  3. Modify that to expand the ternary into a regular if() statement and add some auto variables to hold temporary values that can be inspected in the debugger.
  4. The value step 3 (immediate above) intends to return is different from that received in 2.

Defining our own specialization of call_traits<half> where we make param_type into const half & instead of const half fixes the problem.

Why exactly this is causing bad data to be returned, I'm not entirely sure. Ditto for why there are no problems with optimizations enabled. My fuzzy guess is something to do with the copy / move constructors, RVO, etc.

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.

1 participant