Skip to content

Resolve xtl in user cmake#1351

Merged
AntoinePrv merged 1 commit into
xtensor-stack:masterfrom
AntoinePrv:cmake-xtl
May 20, 2026
Merged

Resolve xtl in user cmake#1351
AntoinePrv merged 1 commit into
xtensor-stack:masterfrom
AntoinePrv:cmake-xtl

Conversation

@AntoinePrv
Copy link
Copy Markdown
Contributor

Instead of having the packager (Debian, Conda recipe) decide on xtl support, let the final user decide on wheter to use it.

The current behaviour is maintained and overridable until xsim 15.
In xsimd 15, the configure time option has no effect for installed targets.

Close #1338

@AntoinePrv AntoinePrv requested a review from JohanMabille May 19, 2026 11:29
@AntoinePrv
Copy link
Copy Markdown
Contributor Author

@drew-parsons does that work for you?

@serge-sans-paille serge-sans-paille self-requested a review May 19, 2026 13:09
Copy link
Copy Markdown
Contributor

@serge-sans-paille serge-sans-paille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too, thanks

@AntoinePrv AntoinePrv merged commit b707616 into xtensor-stack:master May 20, 2026
88 checks passed
@AntoinePrv AntoinePrv deleted the cmake-xtl branch May 20, 2026 07:12
@drew-parsons
Copy link
Copy Markdown

drew-parsons commented May 22, 2026

I'm not certain how to judge this PR. It explicitly preserves the current behaviour of xsimd 14, i.e. reproduces the "inconsistent" behaviour that I reported, adding -DXSIMD_ENABLE_XTL_COMPLEX=1 with or without ENABLE_XTL_COMPLEX. I guess it will become more clear once xsimd 15 is released whether this patch works the way that I was thinking.

edit: wait, no, I think it's working fine for me. I didn't have the tests in my demo test organised as clearly as it could be. The "2 tests" it reported were for type float vs complex, not test_load_store vs test_load_store_xtl. doctest didn't report the SUBCASES clearly. Placing test_load_store_xtl in a separate TEST_CASE_TEMPLATE rather than a SUBCASE shows the patch is working. Compile flag -DXSIMD_ENABLE_XTL_COMPLEX=1 gets added by default in my configuration, but can be deactivated by configuring cmake -DXSIMD_ENABLE_XTL_COMPLEX=Off ... I'll update my bug report with the updated demo test.

So, looks like this patch is working fine with xsimd 14.

If I understood right, the ENABLE_XTL_COMPLEX handling for tests at

if (ENABLE_XTL_COMPLEX)
could be removed or refactored in the future with xsimd 15.

@drew-parsons
Copy link
Copy Markdown

Incidentally, I added an extra test run to the debian CI tests, "test xsimd-test-float" (and test "xsimd-test-clang-float"), which runs the tests with this patch with an explicit -DXSIMD_ENABLE_XTL_COMPLEX=Off. You can see in the test logs at https://ci.debian.net/packages/x/xsimd/testing/amd64/71383346/#S8
that it causes the -DXSIMD_ENABLE_XTL_COMPLEX=1 to be dropped from compile flags, as intended from this PR.

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.

inconsistency in configuration of XTL complex number support

4 participants