Skip to content

Avoiding temporary fails with xt::concatenate #2819

Open

Description

@mnijhuis-tos

When using -DXTENSOR_FORCE_TEMPORARY_MEMORY_IN_ASSIGNMENTS=Off, the following simple test fails:

    TEST(xbuilder, concatenate_assign_to_argument)
    {
        xt::xarray<int> a = xt::ones<int>({ 4 });
        xt::xarray<int> b = xt::ones<int>({ 3 });

        a = xt::concatenate(xt::xtuple(a, b));
        ASSERT_EQ(a, xt::ones<int>({7}));
    }

Instead of containing 7 ones, a will contain garbage.

I have searched for the root cause of the issue, and found the following:

  • Changing overlapping_memory_checker_traits for xgenerator, so check_overlap returns true instead of false, solves the issue. The change is in line 90 of xgenerator.hpp.

  • The problem occurs because of an assumption that an xgenerator always generates fresh values using some generator function. In this case, memory overlap indeed never occurs.

  • xt::concatenate creates an xgenerator that reads values from the arrays in its input tuple, which may overlap with the output array.

The following happens in the test:

  1. xgenerator::assign_to resizes a, so it can hold seven elements. It re-allocates memory for a, without initializing the elements.
  2. The function in the xgenerator, which has type concatenate_access, still wants to concatenate a and b. It uses the new value for a.
  3. Since a now has length 7, concatenate_access::access only uses values from a. It thus copies the uninitialized values from a to a.

Although patching xgenerator works around the issue, a proper fix would be calling check_overlap on the functor inside the xgenerator, and then calling check_overlap on the tuples inside the functor, in case the functor holds a tuple.

Activity

  1. mnijhuis-tos commented on Dec 27, 2024

    @mnijhuis-tos
    ContributorAuthor

    @vakokako Since you created the optimization of avoiding temporaries, you're probably interested in this issue.

  2. risa2000 commented on Apr 19, 2025

    @risa2000

    I just got bitten by the same behavior when migrating from xtensor-0.24.0 to xtensor-0.26.0, where it seems the define XTENSOR_FORCE_TEMPORARY_MEMORY_IN_ASSIGNMENTS is by default undefined.
    Falling back to xtensor-0.25.0 seems to work fine.
    I wonder if it would not be safer to have this defined (or use an inverse logic) instead of forcing this on users and breaking existing code silently.

  3. audiovention commented on Jun 5, 2025

    @audiovention

    Been debugging the exact same issue now for a couple of hours, after I was forced to migrate by clang incompatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions