From 420bc8908e8ebfa4ab92a7523b2c0514023f0500 Mon Sep 17 00:00:00 2001 From: Samaresh Kumar Singh Date: Thu, 18 Dec 2025 10:50:33 -0600 Subject: [PATCH 1/3] 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 --- docs/source/pitfall.rst | 48 +++++++++++++++++++++++++++++++++++++++++ test/test_xmath.cpp | 46 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/docs/source/pitfall.rst b/docs/source/pitfall.rst index 0c404007f..e044be07b 100644 --- a/docs/source/pitfall.rst +++ b/docs/source/pitfall.rst @@ -70,6 +70,54 @@ in the returned expression. Replacing ``auto tmp`` with ``xt::xarray tmp`` does not change anything, ``tmp`` is still an lvalue and thus captured by reference. +.. warning:: + + This issue is particularly subtle with reducer functions like :cpp:func:`xt::amax`, + :cpp:func:`xt::sum`, etc. Consider the following function: + + .. code:: + + template + xt::xtensor logSoftmax(const xt::xtensor &matrix) + { + xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); + auto shifted = matrix - maxVals; + auto expVals = xt::exp(shifted); + auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); + return shifted - xt::log(sumExp); + } + + This function may produce incorrect results or crash, especially in optimized builds. + The issue is that ``shifted``, ``expVals``, and ``sumExp`` are all lazy expressions + that hold references to local variables. When the function returns, these local + variables are destroyed, and the returned expression contains dangling references. + + The fix is to use explicit container types to force evaluation: + + .. code:: + + template + xt::xtensor logSoftmax(const xt::xtensor &matrix) + { + xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); + xt::xtensor shifted = matrix - maxVals; + xt::xtensor expVals = xt::exp(shifted); + xt::xtensor sumExp = xt::sum(expVals, {1}, xt::keep_dims); + return shifted - xt::log(sumExp); + } + + Alternatively, you can use :cpp:func:`xt::eval` to force evaluation: + + .. code:: + + auto shifted = xt::eval(matrix - maxVals); + + Or use the immediate evaluation strategy for reducers: + + .. code:: + + auto sumExp = xt::sum(expVals, {1}, xt::evaluation_strategy::immediate | xt::keep_dims); + Random numbers not consistent ----------------------------- diff --git a/test/test_xmath.cpp b/test/test_xmath.cpp index 17bc57895..edaafb344 100644 --- a/test/test_xmath.cpp +++ b/test/test_xmath.cpp @@ -969,4 +969,50 @@ namespace xt EXPECT_TRUE(xt::allclose(expected, unwrapped)); } } + + // Test for GitHub issue #2871: Proper handling of intermediate results + // This test documents the correct way to use reducers with keep_dims + // when intermediate expressions are needed. + TEST(xmath, issue_2871_intermediate_result_handling) + { + // This test verifies the correct pattern for using reducers with + // intermediate results. Using 'auto' with lazy expressions can lead + // to dangling references when the function returns. + + // The CORRECT way: use explicit container types for intermediate results + auto logSoftmax_correct = [](const xt::xtensor& matrix) + { + xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); + xt::xtensor shifted = matrix - maxVals; + xt::xtensor expVals = xt::exp(shifted); + xt::xtensor sumExp = xt::sum(expVals, {1}, xt::keep_dims); + return xt::xtensor(shifted - xt::log(sumExp)); + }; + + // Alternative CORRECT way: use xt::eval for intermediate results + auto logSoftmax_eval = [](const xt::xtensor& matrix) + { + auto maxVals = xt::eval(xt::amax(matrix, {1}, xt::keep_dims)); + auto shifted = xt::eval(matrix - maxVals); + auto expVals = xt::eval(xt::exp(shifted)); + auto sumExp = xt::eval(xt::sum(expVals, {1}, xt::keep_dims)); + return xt::xtensor(shifted - xt::log(sumExp)); + }; + + // Test data + xt::xtensor input = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}}; + + // Both implementations should produce the same result + auto result1 = logSoftmax_correct(input); + auto result2 = logSoftmax_eval(input); + + EXPECT_TRUE(xt::allclose(result1, result2)); + + // Verify the result is a valid log-softmax (rows sum to 0 in log space) + // exp(log_softmax).sum(axis=1) should equal 1 + auto exp_result = xt::exp(result1); + auto row_sums = xt::sum(exp_result, {1}); + xt::xtensor expected_sums = {1.0, 1.0}; + EXPECT_TRUE(xt::allclose(row_sums, expected_sums)); + } } From e994fc0d94bfcf73c0eef9ea07e460c5fda51929 Mon Sep 17 00:00:00 2001 From: Samaresh Kumar Singh Date: Fri, 3 Apr 2026 08:10:56 -0500 Subject: [PATCH 2/3] docs: Address review feedback on lazy reducer pitfall section - Only force evaluation of the returned expression; intermediate lazy expressions are safe as long as they do not outlive the function - Use auto for shifted/expVals/sumExp in the doc example and tests - Remove misleading "Alternatively" sections (eval on intermediates is not required; evaluation_strategy::immediate does not fix the issue) - Fix logSoftmax_eval test variant to drop unnecessary xt::eval wrappers --- docs/source/pitfall.rst | 23 ++++++----------------- test/test_xmath.cpp | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/docs/source/pitfall.rst b/docs/source/pitfall.rst index e044be07b..a0ef4a206 100644 --- a/docs/source/pitfall.rst +++ b/docs/source/pitfall.rst @@ -92,7 +92,8 @@ is still an lvalue and thus captured by reference. that hold references to local variables. When the function returns, these local variables are destroyed, and the returned expression contains dangling references. - The fix is to use explicit container types to force evaluation: + The fix is to force evaluation of only the returned expression — intermediate + lazy expressions are safe as long as they do not outlive the function: .. code:: @@ -100,24 +101,12 @@ is still an lvalue and thus captured by reference. xt::xtensor logSoftmax(const xt::xtensor &matrix) { xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); - xt::xtensor shifted = matrix - maxVals; - xt::xtensor expVals = xt::exp(shifted); - xt::xtensor sumExp = xt::sum(expVals, {1}, xt::keep_dims); - return shifted - xt::log(sumExp); + auto shifted = matrix - maxVals; + auto expVals = xt::exp(shifted); + auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); + return xt::xtensor(shifted - xt::log(sumExp)); } - Alternatively, you can use :cpp:func:`xt::eval` to force evaluation: - - .. code:: - - auto shifted = xt::eval(matrix - maxVals); - - Or use the immediate evaluation strategy for reducers: - - .. code:: - - auto sumExp = xt::sum(expVals, {1}, xt::evaluation_strategy::immediate | xt::keep_dims); - Random numbers not consistent ----------------------------- diff --git a/test/test_xmath.cpp b/test/test_xmath.cpp index edaafb344..6782eb849 100644 --- a/test/test_xmath.cpp +++ b/test/test_xmath.cpp @@ -976,26 +976,26 @@ namespace xt TEST(xmath, issue_2871_intermediate_result_handling) { // This test verifies the correct pattern for using reducers with - // intermediate results. Using 'auto' with lazy expressions can lead - // to dangling references when the function returns. + // intermediate results. Returning a lazy expression from a function can lead + // to dangling references — only the returned expression must be evaluated. - // The CORRECT way: use explicit container types for intermediate results + // The CORRECT way: use auto for intermediates, force evaluation only at return auto logSoftmax_correct = [](const xt::xtensor& matrix) { xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); - xt::xtensor shifted = matrix - maxVals; - xt::xtensor expVals = xt::exp(shifted); - xt::xtensor sumExp = xt::sum(expVals, {1}, xt::keep_dims); + auto shifted = matrix - maxVals; + auto expVals = xt::exp(shifted); + auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); return xt::xtensor(shifted - xt::log(sumExp)); }; - // Alternative CORRECT way: use xt::eval for intermediate results + // Alternative CORRECT way: use xt::eval on the reducer result auto logSoftmax_eval = [](const xt::xtensor& matrix) { auto maxVals = xt::eval(xt::amax(matrix, {1}, xt::keep_dims)); - auto shifted = xt::eval(matrix - maxVals); - auto expVals = xt::eval(xt::exp(shifted)); - auto sumExp = xt::eval(xt::sum(expVals, {1}, xt::keep_dims)); + auto shifted = matrix - maxVals; + auto expVals = xt::exp(shifted); + auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); return xt::xtensor(shifted - xt::log(sumExp)); }; From 2b9cb783d760855acbce57e94b7a17d669b3c00e Mon Sep 17 00:00:00 2001 From: Samaresh Kumar Singh Date: Fri, 3 Apr 2026 11:51:13 -0500 Subject: [PATCH 3/3] docs: Fix lazy reducer pitfall example reducer results must be evaluated A lazy reducer (e.g. xt::sum with keep_dims) cannot be safely used as an operand in a subsequent element-wise expression without first materializing it. Element-wise lazy expressions (shifted, expVals) are safe as auto, but the reducer result (sumExp) must be assigned to xt::xtensor or wrapped in xt::eval before use. This fixes the CI test failure in issue_2871_intermediate_result_handling. --- docs/source/pitfall.rst | 8 +++++--- test/test_xmath.cpp | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/source/pitfall.rst b/docs/source/pitfall.rst index a0ef4a206..b9e40a2a3 100644 --- a/docs/source/pitfall.rst +++ b/docs/source/pitfall.rst @@ -92,8 +92,10 @@ is still an lvalue and thus captured by reference. that hold references to local variables. When the function returns, these local variables are destroyed, and the returned expression contains dangling references. - The fix is to force evaluation of only the returned expression — intermediate - lazy expressions are safe as long as they do not outlive the function: + The fix is to evaluate reducer results and the returned expression explicitly. + Element-wise lazy expressions (like ``shifted`` and ``expVals``) are safe to + leave as ``auto``, but reducer results (like ``sumExp``) must be materialized + before being used in a subsequent element-wise expression: .. code:: @@ -103,7 +105,7 @@ is still an lvalue and thus captured by reference. xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); auto shifted = matrix - maxVals; auto expVals = xt::exp(shifted); - auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); + xt::xtensor sumExp = xt::sum(expVals, {1}, xt::keep_dims); return xt::xtensor(shifted - xt::log(sumExp)); } diff --git a/test/test_xmath.cpp b/test/test_xmath.cpp index 6782eb849..8f56e93b9 100644 --- a/test/test_xmath.cpp +++ b/test/test_xmath.cpp @@ -979,23 +979,24 @@ namespace xt // intermediate results. Returning a lazy expression from a function can lead // to dangling references — only the returned expression must be evaluated. - // The CORRECT way: use auto for intermediates, force evaluation only at return + // The CORRECT way: reducer results must be evaluated; element-wise lazy + // expressions are safe to leave as auto auto logSoftmax_correct = [](const xt::xtensor& matrix) { xt::xtensor maxVals = xt::amax(matrix, {1}, xt::keep_dims); auto shifted = matrix - maxVals; auto expVals = xt::exp(shifted); - auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); + xt::xtensor sumExp = xt::sum(expVals, {1}, xt::keep_dims); return xt::xtensor(shifted - xt::log(sumExp)); }; - // Alternative CORRECT way: use xt::eval on the reducer result + // Alternative CORRECT way: use xt::eval for reducer results auto logSoftmax_eval = [](const xt::xtensor& matrix) { auto maxVals = xt::eval(xt::amax(matrix, {1}, xt::keep_dims)); auto shifted = matrix - maxVals; auto expVals = xt::exp(shifted); - auto sumExp = xt::sum(expVals, {1}, xt::keep_dims); + auto sumExp = xt::eval(xt::sum(expVals, {1}, xt::keep_dims)); return xt::xtensor(shifted - xt::log(sumExp)); };