Skip to content

Commit 420bc89

Browse files
committed
docs: Add warning about dangling references with lazy reducers (fixes #2871)
This commit addresses GitHub issue #2871 where using 'auto' with xtensor reducer functions (like amax, sum with keep_dims) in functions causes crashes or incorrect results in optimized builds. The issue is that lazy expressions hold references to local variables. When intermediate results are stored with 'auto', these references become dangling when the function returns, leading to undefined behavior. Fixes #2871
1 parent cbb9a36 commit 420bc89

File tree

2 files changed

+94
-0
lines changed

2 files changed

+94
-0
lines changed

docs/source/pitfall.rst

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,54 @@ in the returned expression.
7070
Replacing ``auto tmp`` with ``xt::xarray<double> tmp`` does not change anything, ``tmp``
7171
is still an lvalue and thus captured by reference.
7272

73+
.. warning::
74+
75+
This issue is particularly subtle with reducer functions like :cpp:func:`xt::amax`,
76+
:cpp:func:`xt::sum`, etc. Consider the following function:
77+
78+
.. code::
79+
80+
template <typename T>
81+
xt::xtensor<T, 2> logSoftmax(const xt::xtensor<T, 2> &matrix)
82+
{
83+
xt::xtensor<T, 2> maxVals = xt::amax(matrix, {1}, xt::keep_dims);
84+
auto shifted = matrix - maxVals;
85+
auto expVals = xt::exp(shifted);
86+
auto sumExp = xt::sum(expVals, {1}, xt::keep_dims);
87+
return shifted - xt::log(sumExp);
88+
}
89+
90+
This function may produce incorrect results or crash, especially in optimized builds.
91+
The issue is that ``shifted``, ``expVals``, and ``sumExp`` are all lazy expressions
92+
that hold references to local variables. When the function returns, these local
93+
variables are destroyed, and the returned expression contains dangling references.
94+
95+
The fix is to use explicit container types to force evaluation:
96+
97+
.. code::
98+
99+
template <typename T>
100+
xt::xtensor<T, 2> logSoftmax(const xt::xtensor<T, 2> &matrix)
101+
{
102+
xt::xtensor<T, 2> maxVals = xt::amax(matrix, {1}, xt::keep_dims);
103+
xt::xtensor<T, 2> shifted = matrix - maxVals;
104+
xt::xtensor<T, 2> expVals = xt::exp(shifted);
105+
xt::xtensor<T, 2> sumExp = xt::sum(expVals, {1}, xt::keep_dims);
106+
return shifted - xt::log(sumExp);
107+
}
108+
109+
Alternatively, you can use :cpp:func:`xt::eval` to force evaluation:
110+
111+
.. code::
112+
113+
auto shifted = xt::eval(matrix - maxVals);
114+
115+
Or use the immediate evaluation strategy for reducers:
116+
117+
.. code::
118+
119+
auto sumExp = xt::sum(expVals, {1}, xt::evaluation_strategy::immediate | xt::keep_dims);
120+
73121
Random numbers not consistent
74122
-----------------------------
75123

test/test_xmath.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,4 +969,50 @@ namespace xt
969969
EXPECT_TRUE(xt::allclose(expected, unwrapped));
970970
}
971971
}
972+
973+
// Test for GitHub issue #2871: Proper handling of intermediate results
974+
// This test documents the correct way to use reducers with keep_dims
975+
// when intermediate expressions are needed.
976+
TEST(xmath, issue_2871_intermediate_result_handling)
977+
{
978+
// This test verifies the correct pattern for using reducers with
979+
// intermediate results. Using 'auto' with lazy expressions can lead
980+
// to dangling references when the function returns.
981+
982+
// The CORRECT way: use explicit container types for intermediate results
983+
auto logSoftmax_correct = [](const xt::xtensor<double, 2>& matrix)
984+
{
985+
xt::xtensor<double, 2> maxVals = xt::amax(matrix, {1}, xt::keep_dims);
986+
xt::xtensor<double, 2> shifted = matrix - maxVals;
987+
xt::xtensor<double, 2> expVals = xt::exp(shifted);
988+
xt::xtensor<double, 2> sumExp = xt::sum(expVals, {1}, xt::keep_dims);
989+
return xt::xtensor<double, 2>(shifted - xt::log(sumExp));
990+
};
991+
992+
// Alternative CORRECT way: use xt::eval for intermediate results
993+
auto logSoftmax_eval = [](const xt::xtensor<double, 2>& matrix)
994+
{
995+
auto maxVals = xt::eval(xt::amax(matrix, {1}, xt::keep_dims));
996+
auto shifted = xt::eval(matrix - maxVals);
997+
auto expVals = xt::eval(xt::exp(shifted));
998+
auto sumExp = xt::eval(xt::sum(expVals, {1}, xt::keep_dims));
999+
return xt::xtensor<double, 2>(shifted - xt::log(sumExp));
1000+
};
1001+
1002+
// Test data
1003+
xt::xtensor<double, 2> input = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}};
1004+
1005+
// Both implementations should produce the same result
1006+
auto result1 = logSoftmax_correct(input);
1007+
auto result2 = logSoftmax_eval(input);
1008+
1009+
EXPECT_TRUE(xt::allclose(result1, result2));
1010+
1011+
// Verify the result is a valid log-softmax (rows sum to 0 in log space)
1012+
// exp(log_softmax).sum(axis=1) should equal 1
1013+
auto exp_result = xt::exp(result1);
1014+
auto row_sums = xt::sum(exp_result, {1});
1015+
xt::xtensor<double, 1> expected_sums = {1.0, 1.0};
1016+
EXPECT_TRUE(xt::allclose(row_sums, expected_sums));
1017+
}
9721018
}

0 commit comments

Comments
 (0)