-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Failing Albany configures due to incorrect Trilinos version #982
Comments
Actually it looks like the issue is not the version but finding Trilinos. In LCM, we don't ask for a specific Trilinos version (https://github.com/sandialabs/LCM/blob/main/CMakeLists.txt#L40) but still get the same error of Trilinos not being found (https://sems-cdash-son.sandia.gov/cdash/build/52529/configure) even though there is a successful install of Trilinos that is being pointed to. |
I think cmake is looking in the wrong path. If you check the error log of this build, you can see that we specify this trilinos path
but CMake looks for trilinos here
I'm not sure how that path got searched. Perhaps it is set inside some script that runs in the nightlies? Either way, |
@bartgol : thanks for looking at it. I think the problem is it's not finding the right Trilinos even though the path is set. The other Trilinos that is being pointed to exists in my home dir, but it does not appear in any of the test scripts. Please see also my previous comment about what is happening in LCM (no version is requested, just Trilinos is not found). I think the broader question is why is |
Since this happened only once, I would wait a bit. Since it affected several builds, across several platforms, it is likely to not be a fluke. Alas, cmake correctly finds trilinos on my laptop (cmake 3.26) and my workstation (cmake 3.24), so I don't think we have anything wrong in our cmake logic. |
Are you sure you are using an up-do-date develop branch of Trilinos on your laptop? I just pulled everything from scratch and rebuilt, and am still seeing the problem. I printed CMAKE_PREFIX_PATH and it is correct, so I don't get why Trilinos is not being found. I am also using cmake 3.26. @bartlettroscoe , did anything change recently in the Trilinos build system that would cause the issues described here? Basically Albany is no longer finding Trilinos starting today. Here is how the CMakeLists.txt file looks like: https://github.com/sandialabs/Albany/blob/master/CMakeLists.txt |
This was merged yesterday: trilinos/Trilinos#12258 Specifically this one: trilinos/Trilinos#12104 (comment) seems to indicate that trilinos installs libs in lib64 now. But TLDR |
Could be this too: https://github.com/sandialabs/Albany/blob/master/CMakeLists.txt#L67 maybe change that to lib64 |
I thought CMake would look in several subfolders of the search paths, when it looked for the package config file. This section of their docs seems to suggest that both
should match As for that line in our cmake list, it should be executed after we find trilinos, so it should not be the culprit here. I also think they should not be used, since we link CMake targets, and should not need to keep track of lib/include dirs manually... |
It's not this - the issue is Trilinos_DIR is not being set. |
It actually looks like the above Trilinos configure option does resolve this! I can try to change some of the nightlies to use this to see what happens with the nightlies. |
CC: @sebrowne, @rppawlo, @ccober6
@ikalash et.al., FYI, it is likely that using GNUInstallDirs will not be optional in the future as per: This is just the tip of the iceberg when moving TriBITS and Trilinos to modern CMake (i.e. buckle up). The days of all backward compatible changes in TriBITS/Trilinos CMake refactorings are over. (Talk with Trilinos management about this.) If you want recommendations on how to set up Albany's CMake usage of Trilinos to be more modern CMake standard's compliant, let me know. |
Yes, recommendations for CMake for Albany would be great. |
Thanks, @bartlettroscoe. It would be good if you ping us when you are about to merge a PR that breaks backward compatibility, so that we have a bit more time to fix Albany. |
@mperego, my recommendation is to update Albany to use |
@bartlettroscoe it appears that having that option ON makes Albany not find Trilinos anymore. You said we can talk regarding including Trilinos in a more cmake-y way (I thought |
We'll do. I was referring to other future changes to TriBITS/Trilinos CMake. |
CC: @jwillenbring, @sebrowne
First recommendation, stop assuming you can get compilers from the Trilinos install with find_package(Trilinos). That does not work for CUDA as discovered and discussed in: and is likely to be removed from TriBITS and Trilinos in the future. And moving to GNUInstallDirs.cmake for Trilinos breaks find_package(Trilinos) as per: So get rid of all of the code like: Lines 98 to 135 in 683b9e9
Albany needs to have its compilers explicitly passed in and defined in CMake before calling I know it was really convenient to be able to get compilers from find_package(Trilinos) but that can not be supported when moving to modern CMake and issues with CUDA and Kokkos. NOTE: We may be able to continue support for providing compilers used with installed versions of Trilinos but it can't be through find_package(Trilinos). If that is important to Albany, please request that in: |
I was never a fan of relying on a TPL to get the compilers, so this is a welcome suggestion. :) |
@mperego, this is a good question. I will defer to @sebrowne, @ccober6, and @jwillenbring on how and when to pre-notify Trilinos customers of potentially backwards compatibility breaks to Trilinos 'develop'. Personally, I am happy to send out emails to the various Trilinos lists but it is hard to know how much notification is too much (and that is what RELEASE_NOTES are for, right?). |
Using the permanent link: Line 67 in 683b9e9
this needs to be removed because you can't assume the name of the library directory going forward with modern CMake installs of packages (see https://gitlab.kitware.com/cmake/cmake/-/issues/25157#note_1418009). |
Can we still do
? That is, assuming @kliegeois The only place where we use this is in pyAlbany. Does pyAlbany really need it? |
Fair. I'll bring this up at next Trilinos leadership meeting. |
Thanks for all the tips, @bartlettroscoe . Could someone please volunteer to work with Ross to try to clean up Albany's CMakeLists.txt to work with the recent Trilinos changes? @bartgol perhaps you would be willing to do this? In the meantime, we can use |
@bartgol, that does not work in general because the value of
which will be used instead of The only way that Albany would get the value of |
@ikalash I'll take care of crafting a long term solution. I enjoy cmake stuff. |
It appears the issue is the order in which Works:
Doesn't work:
@bartlettroscoe I looked at the CMake docs, as well as the professional CMake book, but could not find anythink specifying the order of ops for project and find_package. I think we had Notice that the case that did not work failed very early, to the point where even adding Also notice that I tried with several approaches for the specification of the pkg folder (env var, |
@bartgol, yup, that is exactly what I was trying to point out above which points to: You have to define the compilers before calling If you don't get compilers from Trilinos, there is no reason to not define the compilers before calling Sorry you had to discover this on your own again |
@bartlettroscoe Ah, sorry, I didn't catch what you were hinting at. Ok, great, it's nice to know this is not just some weird coincidence, but the way things are supposed to be. Follow up question: we should not assume that libraries are in the Edit: I'd be happy to rely on stuff like |
@bartgol, to be modern CMake compliant, I would not assume the libraries are in the The documentation for that module is: which tries to follow the GNU standard: (But the latter does not mention
I don't know how to address this but one option might be to extend TriBITS to write the paths used with GNUInstallDirs.cmake when Trilinos was configured into the Trilinos-installed package config files. For example,
and then downstream customers like Albany could get these by calling set(SEACAS_bin "${Trilinos_CMAKE_INSTALL_BINDIR}") # Defined by find_package(Trilinos ...) Thoughts? |
Yes, I think that would be a good idea. For libs, it's probably not that important (we link them as targets, which are already found by
but I think that's probably already done and safe to assume. Btw, I saw some hard-coded relative paths in
I guess that's fine if they are not going to change, but it's odd to see this when everything else seems to be put in a path prescribed by a var. I'm not interested to change this, just brought it up in case this was a not-intended cmake usage in trilinos. |
@bartgol, you should post a GitHub issue to: suggesting it call
Using GNUInstallDirs.cmake is considered modern CMake and is recommend in "Professional CMake". |
All the Albany and Albany-LCM nightlies are failing as a result of Albany finding the "wrong" Trilinos version. I checked the version of Trilinos that was built and it is 14.5. I then tried to chance Albany/CMakeLists.txt to look for 14.5 Trilinos instead of 14.1, but it did not fix the problem. My guess is this is due to a Trilinos change. Does anyone have any insight into this? Perhaps @jewatkins @bartgol @mcarlson801 @mperego @kliegeois ?
Tagging @lxmota since this impact LCM too.
The text was updated successfully, but these errors were encountered: