Scope out whether Albany works with C++17
Once we get a working build, will want to have a C++17 nightly build, probably.
This works fine on my sems-gcc/8.3.0 build but my clang build fails. I think because it was built with an old gcc. We can try turning it on in our nightly trilinos builds and see what happens?
-D CMAKE_CXX_STANDARD=17 \
@jewatkins : thanks for testing this. Please let me know what build you'd like to enable the C++17, and I'm happy to do that. I'd suggest to start with 1-2 builds to verify that things are OK.
Maybe mockba or camobap since they have newer compilers? There might be issues with blake/weaver since the compilers are older. I could debug those next week if so.
I can do that. I assume this option goes in Trilinos, and then Albany will inherit it?
Yes, you can add it to Trilinos and Albany will inherit it. Also you can remove: -D Trilinos_ENABLE_CXX11:BOOL=ON \
I updated camobap. I'll do the same for mockba if there are no issues with the camobap nightlies.
c++17 works on the weaver build
@jewatkins : do you want me to enable it in the weaver nightlies? Or, maybe wait for all the other weaver issues to be fixed first?
Yes, I think I copied it over to the right directory but maybe I missed something.
Great! I checked the nightlies and I see that the weaver one is using the C++17 flags, so I think we are good on weaver.
I created a google sheet that summarizes the nightly builds, so that we can keep track of what has C++17 enabled and what doesn't: https://docs.google.com/spreadsheets/d/1MO6huLBmZr3VKbo27_iQ6F8VYqmItxy0PdbByoub1Nc/edit?usp=sharing . @jewatkins can you please check that it's accurate? I think only blake, weaver and some of the camobap builds have C++17 enabled. I can start working on propagating the options to the other nightlies, namely CEE and skybridge. I will need to figure out / make a decision about CALI to port the remaining camobap and the mockba albany nightlies (and Cori as well).
I don't think blake-gcc has it yet so I removed those. The rest look good.
Yes you are right. Can we activate it in that build, or there is a reason not to? Also, were you able to get things to work with clang? One of the CEE builds is clang, so if that one doens't work with C++17, we may want to defer changing it over.
Maybe we can try turning it on all runs for one nightly and see how it goes? Then revert if things fail? I haven't tested the blake-gcc build. I think we'll have to upgrade that build too since it's gcc 7.2
So I tried changing the Trilinos (intel) and CEE intel + clang builds to turn on C++17, and it did not work. Errors look as follows:
https://sems-cdash-son.sandia.gov/cdash/viewBuildError.php?buildid=41141 https://sems-cdash-son.sandia.gov/cdash/viewBuildError.php?buildid=41161 https://sems-cdash-son.sandia.gov/cdash/viewBuildError.php?buildid=41134
The errors are related to Kokkos and look as follows:
/.../repos/Trilinos/packages/kokkos/core/src/Kokkos_MathematicalConstants.hpp(70): error: namespace "std" has no member "is_floating_point_v"
The skybridge one is different and makes me wonder if there's an issue with the compiler being used.
I've had similar issues with clang inside vim (using it as a background compiler to spot compile-time errors as I write). Usually, this points to the incorrect libstdc++ headers being used by the compiler. Since clang relies on gcc headers, this might be an issue of clang-gcc compatibility.
One thing that might help with our cdash builds debugging is if we used -DCMAKE_VERBOSE_MAKEFILE=ON, so that we could see in the warnings/error message the exact compile line used.
I suspect all of them are due to old gcc compilers since the clang/intel compilers seem new enough. Those things should exist in std if the compiler supports c++17. The intel build seems to use gcc/6.1.0? We'll have to go through each one and update the build. Perhaps we can discuss which builds we want to maintain at the next meeting so we're not reviving builds that we end up removing?
I will put this on the agenda for the next meeting (although we may not get to it due to the spack discussion that is planned).
It would be great if someone (@jewatkins ?) could volunteer to figure out a new set of modules / build scripts that work with intel and clang on CEE and intel on skybridge. The skybridge build is particularly out of date, I think - it is using a set of modules, cmake script and workflow created a very long time ago for LCM I believe by Dan Ibanez. We could rework this nightly to look more like the others if this is desired, once a new set of modules / configure scripts that work are identified. Happy to discuss this in the near future at one of the Albany meetings.
FYI, I have converted all the nightlies to use C++17 except the ones where it does not work, which is CEE clang, CEE intel and Skybridge intel. I am pretty sure the problem with these is that the modules / compilers are too old. I actually tried using newer ones but ran into issues. We will need to have some volunteer to rework these nightlies so that they can work with C++17. We can discuss this at the next Albany meeting.
@ikalash I just added cee-clang, cee-gcc configurations here: https://github.com/sandialabs/Albany/tree/master/doc/LandIce/cee skybridge/attaway are here: https://github.com/sandialabs/Albany/tree/master/doc/LandIce/skybridge All tests pass with the configuration I have in those directories. Some of the skybridge/attaway tests were a little slow so maybe these could be tuned.
We should also have a conversation about what Trilinos/Albany configuration we should be running with at some point. I'm sure I left some things out and maybe some of the options could be removed.
It was easy enough to try a cee-intel build but I ran into several issues with the various intel compilers available. I have a ticket up with sems. If I find something that works, I'll let you know.
Also on cee, you might need to change Kokkos_ARCH_SKX depending on which compute node you're using.
Thanks @jewatkins ! Did we want to update the CEE gcc build then? I can do this but wanted to double check. The current CEE gcc build is working with C++17.
On what cee-compute nodes does Kokkos_ARCH_SKX need to change, and to what? Currently the CEE gcc builds run on cee-compute007 and cee-compute025, and the clang one is on cee-compute010 and cee-compute024 (there are 2 of each b/c one is debug, one is release). I can add the note about a conversation about Trilinos/Albany configurations to future Albany weekly meeting notes.
Yeah, I was thinking it might be best to update the gcc build so that it's similar to the clang build.
cee-compute007: broadwell -> Kokkos_ARCH_BDW cee-compute010: haswell -> Kokkos_ARCH_HSW cee-compute024, 025: Ivy Bridge -> Kokkos_ARCH_SNB Though these might change? They may have swapped cpus in the past. It might be safer to remove the "ARCH" option. It should still run fine.
@jewatkins : does the Albany built with your clang run for you? It doesn't for me - here is the error: https://sems-cdash-son.sandia.gov/cdash/test/3084809 . It happens when I build the usual way not just with the CDash scripts.
Also, we should probably add the option to use C++17 in the spack build, right?
It worked on ecw but I didn't test it on a compute node. I'll try it on compute024 today.
Yes, we should add C++17 to that build too.
Thanks @jewatkins , that would be great. The clang nightlies run on cee-compute024 and cee-compute010. You can see the errors on the CDash site.
I have updated spack with the change, am doing some testing before I merge it.
I tested cee-compute024 w/ and w/o Kokkos_ARCH_SNB and it works. Maybe you forgot to remove Kokkos_ARCH_SKX?
I did indeed forget to remove Kokkos_ARCH_SKX in the clang build (I did it in the gcc). I'll make the change and hopefully the issue will go away tomorrow. Thanks!
It might be safer to remove the "ARCH" option. It should still run fine.
Imho, we should keep the arch option. In kokkos, it should trigger the correct AVX flags as well as other (potentially) relevant optimization flags. In production runs, we likely do want those flags on, so we should test the code with the correct arch enabled.