Skip to content

Update Cmake to allow third-party libraries - #2006

Merged
paulromano merged 10 commits into
openmc-dev:developfrom
kkiesling:thirdparty_libs
Mar 18, 2022
Merged

paulromano merged 10 commits into
openmc-dev:developfrom
kkiesling:thirdparty_libs

Conversation

@kkiesling

Copy link
Copy Markdown
Contributor

This addresses #1779. CMake will now search for existing installations of xtensor, xtl, and gsl-lite and use those instead. If not found, it'll use the vendored submodules.

@paulromano paulromano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @kkiesling and congrats on your first PR here! 🎉 Just a few comments regarding how we handle xtl

Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
@kkiesling
kkiesling requested a review from pshriwise as a code owner March 17, 2022 15:58
@kkiesling

kkiesling commented Mar 17, 2022 •

Copy link
Copy Markdown
Contributor Author

I think I have at least one solution to one of the build errors coming from xtensor on CI (we'll see when it is finished running since I can't really reproduce this locally). For xt::flip I explicitly included the header file that it comes from.

For the other build error with xt::col, I think that comes down to a version issue. vectfit builds with xtensor version 0.21.2, which is then found as the already installed version, where as the OpenMC vendored version is 0.21.3. It looks like xt::row and xt::col were introduced in 0.21.3 (see pr). We could probably get around this by requiring a minimum version of xtensor and if it's not satisfied then install the vendored version instead.

@kkiesling

Copy link
Copy Markdown
Contributor Author

One last change and hopefully this makes it work- I updated the xtl and xtensor versions on the CI build script for vectfit to be the versions that are vendored. This new find_package change was finding the versions installed with vectfit, but they were too old and didn't have some of the functions currently used by openmc.

@paulromano paulromano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Thanks for working through the strange build issues 😄 Will merge tomorrow morning if there's no more feedback

@paulromano
paulromano merged commit 0d7a0e1 into openmc-dev:develop Mar 18, 2022
apingegno pushed a commit to apingegno/openmc that referenced this pull request May 7, 2026
Update Cmake to allow third-party libraries
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