Skip to content

Conversation

@vvish
Copy link
Contributor

@vvish vvish commented Nov 26, 2025

WIP version of the algorithm re-enablement as discussed in #2:

  • code that was removed in Removed stl algorithms from xsimd xsimd#830 is recovered into this repository;
  • cmake project created;
  • tests ported to use doctest instead of gtest (similar to the rest of the stack);
  • initial version of github actions workflow created for linux (followiing xsimd and xtensor pattern);
  • clang-format configuration added.

There are several open questions:

  • naming: the project is called xsimd-algorithm, but previously it was in stl folder and was refered as "stl". So I would appreciate guidelines on naming for cmake project, include folder e.t.c.
  • testing strategy: currently the project has test environment configurations similar to xsimd. This requires similar configuration, including sde installation for avx512 (switched off in the PR). Should it be like this? Should install_sde.sh script be copied into this repo as well?
  • overall progress: what status is required to start reviewing/merging the PR?

@vvish vvish changed the title Vvish/feat/algo recover Algorithm re-enablement Nov 26, 2025
@JohanMabille
Copy link
Member

Thanks for tackling this! Regarding your questions:

  • naming: include/xsimd-algorithm should be the root folder, then we can have an stl folder under it, and then we can have a file per family of algorithms (transform, copy_if, reduce). For now we don't have many algorithms, but I would like to avoid 5k+ LOC files when we add missing algorithms. We can have a xsimd-algorithm.hpp header under xsimd-algorithm including all the headers to make it easier for downstream projets to include this one.
  • testing strategy: I think we should do the same as xsimd. install_sde.sh is a small script, so I find it simpler to copy it here rather than setting up a more complex machinery to get it from elsewhere.
  • overall progress: PR reviews can be asked when you're happy with the implementation, tests have been added and the CI is green. This does not prevent quick early reviews to discuss a point or a general orientation before finishing to implement a feature. Generally it is easier for me to review many short well scoped PRs (one PR per algorithme for instance) than a huge one (containing all algorithms related to a family for instance). I'm not speaking for this one, since it i contains all the machinery for setting tests and the CI, which is pretty mechanical to review.

@vvish vvish force-pushed the vvish/feat/algo_recover branch from 110dea2 to e215e5f Compare November 30, 2025 15:40
@vvish vvish force-pushed the vvish/feat/algo_recover branch from e215e5f to e1b4783 Compare November 30, 2025 16:19
@vvish vvish marked this pull request as ready for review December 1, 2025 20:49
@vvish
Copy link
Contributor Author

vvish commented Dec 1, 2025

@JohanMabille , thank you for the reply. I adjusted the PR according to your comments: headers and tests are split, tests are kept similar to xsimd.

It looks like actions are disabled for this repository as no checks are running. In my fork they are green so I assume we can start review process.

@JohanMabille
Copy link
Member

Awesome!
Regarding the CI, I think it is not run until there is at least a workflow file on the main branch. Let's merge and possibly fix any failure we may see.

Thanks again!

@JohanMabille JohanMabille merged commit 89635a8 into xtensor-stack:main Dec 2, 2025
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.

2 participants