You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Avoiding temporary fails with xt::concatenate #2819
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:
xgenerator::assign_to resizes a, so it can hold seven elements. It re-allocates memory for a, without initializing the elements.
The function in the xgenerator, which has type concatenate_access, still wants to concatenate a and b. It uses the new value for a.
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.
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.
Activity
mnijhuis-tos commentedon Dec 27, 2024
@vakokako Since you created the optimization of avoiding temporaries, you're probably interested in this issue.
risa2000 commentedon Apr 19, 2025
I just got bitten by the same behavior when migrating from
xtensor-0.24.0toxtensor-0.26.0, where it seems the defineXTENSOR_FORCE_TEMPORARY_MEMORY_IN_ASSIGNMENTSis by default undefined.Falling back to
xtensor-0.25.0seems 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.
audiovention commentedon Jun 5, 2025
Been debugging the exact same issue now for a couple of hours, after I was forced to migrate by clang incompatibility.
Fix xgenerator memory overlap in assignment (issue xtensor-stack#2819)