Skip to content

Conversation

@bangerth
Copy link
Contributor

Xtensor has moved all of its header files into new locations -- see xtensor-stack/xtensor#2829. This is of course a colossal hassle, but it can be worked around in a way so that it should work with both the old and new header locations.

This patch seems to work for me, and I hope it also still works for the ones of you who still use Xtensor 0.25.0.

@benbovy
Copy link
Member

benbovy commented Jun 13, 2025

Thanks @bangerth for tackling this.

I agree. Like others I don't really understand why this change in xtensor has been made abruptly without much notice.

I'd be fine to bump the minimum required version of xtensor and xtensor-python here in fastscapelib, instead of having the burden of supporting xtensor versions both before and after 0.26. Fastscapelib users are mostly using it from within Python, for which xtensor is not a runtime dependency (I can easily update the Python wheels and conda packages). However, please let me know if on the ASPECT side you'd need to support multiple xtensor and/or fastscapelib versions.

@benbovy
Copy link
Member

benbovy commented Jun 13, 2025

Looking at the failing tests, I'm afraid that we'll need much more #if statements checking xtensor header files across the whole Fastscapelib codebase, to the point where this will start looking ugly.

I think I'd prefer to bump the minimum required version of xtensor to 0.26 here. Would you mind that? For ASPECT I think that it'll be safer to pin the version of fastscapelib and xtensor anyway. This is what we do for the Fastscapelib package on conda-forge: https://github.com/conda-forge/fastscapelib-feedstock/blob/95a7483c739b26329b7105e5e78037217ef13b55/recipe/meta.yaml#L31.

For xtensor it is likely that next versions will introduce other breaking changes. Likewise in Fastscapelib, which is still in early development, although here we definitely want to avoid too much disruption.

@bangerth
Copy link
Contributor Author

I must admit that I'm confused by the error messages. When the compiler refers to, for example, /home/runner/micromamba/envs/fastscapelib-python-dev/include/xtensor/containers/xstorage.hpp, then is this a file copied over from the Xtensor installation? What version does that reference? 0.25 had files only under include/xtensor/*.hpp, whereas 0.26 has files in include/xtensor/containers/*.hpp, for example. So I had assumed that the tester still uses 0.25, but perhaps that's not true?

@bangerth
Copy link
Contributor Author

The tests that fail to compile seem to have a similar issue: I didn't touch them, but they now no longer find Xtensor -- perhaps because they look for files in the 0.25 directories, but now they are in the 0.26 locations?

@benbovy
Copy link
Member

benbovy commented Jun 13, 2025

The tests are using xtensor 0.26, installed using the conda (micromamba) package manager. You can check that, e.g., there: https://github.com/fastscape-lem/fastscapelib/actions/runs/15620883204/job/44032444103?pr=174#step:4:144.

/home/runner/micromamba/envs/fastscapelib-python-dev/ is the path to the conda environment used for the (Python) tests run in CI.

In #176 I pinned xtensor to 0.25, which is the version now installed for CI workflows run in the main branch and all subsequent PRs. I'll add a test in CI installing the master branch of xtensor, this will be useful here and in general to anticipate breaking changes.

@bangerth
Copy link
Contributor Author

I think I'm confused how this worked before. If you have 0.26 installed, how did the tests run before given that Fastscape couldn't compile against 0.26?

@benbovy
Copy link
Member

benbovy commented Jun 14, 2025

how did the tests run before given that Fastscape couldn't compile against 0.26?

They run successfully in CI before because the installed version of xtensor was the (unpinned) last available one at that time and 0.26 wasn't released yet.

Since #176 the installed version of xtensor for CI tests is explicitly set to >=0.25,<0.26 as a temporary fix. The idea is to now update that pin to >=0.26,<0.27 and fix all header includes.

Since #180 there's an additional CI test that installs xtensor's master branch (to be triggered manually so we avoid red crosses everywhere).

@bangerth
Copy link
Contributor Author

Oh, I see now. I had not realized that your CI had not been updated since 0.26 came out. Anyway, I'm not entirely sure how to proceed here. Can I leave this to you as a starting point for actually making it work?

@benbovy
Copy link
Member

benbovy commented Jun 14, 2025

Yes I can take over on what you've done here, thanks!

@benbovy
Copy link
Member

benbovy commented Jun 16, 2025

Closed in #182.

Thanks @bangerth for submitting this one.

@benbovy benbovy closed this Jun 16, 2025
@bangerth bangerth deleted the headers branch June 16, 2025 15:47
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