cmake refactor #225
cmake refactor #225
Conversation
Updated neso-particles submodule
Changed the FindSYCL.cmake to be a macro
- Can request specific sycl implementation
- looks for intel sycl if intel compilers beign used
- adaptiveCPP/hipsycl otherwise
Removed some using namespace std and added "std::"s where needed
Using the neso-particles find restrict code in NESO - I don't like this
would a header file + some pre-processing be better instead of (or in addition to) this
- My main objection is that if neso-particles is installed, then you
can't export the RESTRICT keyword you found as part of the
neso-particles target as (in theory) you might compile in the
consuming project with a different compiler that doesn't have the
same keyword.
|
These fixes for the namespace std includes fix the compiling issues we have been seeing with gcc 11.3.0, 11.4.0 and gcc 12.3.0. (Apparently Nektar saw this as well and a stray "using namespace std" was the cause). |
We should probably remove all the |
Simplfy SYCL.cmake Stop setting HIPSYCL_TARGET etc update submodule
jwscook
requested review from
cmacmackin
and removed request for
will-saunders-ukaea
|
@cmacmackin would you mind reviewing this and merging if acceptable please? |
|
Sorry for not doing this earlier. There kept being new commits pushed so I wasn't sure when it was actually ready. |
There was a problem hiding this comment.
When I tried building this I got this error again:
The link interface of target "hdf5-shared" contains:
MPI::MPI_C
but the target was not found. Possible reasons include:
I addressed this in ExCALIBUR-NEPTUNE/NESO-Particles#47 but it seems to have been lost in @seimtpow refactoring.
There was a problem hiding this comment.
I've made some changes necessary to make everything build on my system (plus address a flaw I noticed). It's probably worth @will-saunders-ukaea or @oparry-ukaea checking everything still builds for them. Might also be best to wait until ExCALIBUR-NEPTUNE/NESO-Particles#64 is merged.
|
Shall I merge main into this, resolve the conflict with solvers/SimpleSOL/EquationSystems/SOLWithParticlesSystem.cpp and make sure it builds for me? |
|
I get (before merging from main, and having run |
|
Could you send me your |
|
That looks like it ran successfully. I'm not seeing any errors about failing to find NESO-Particles. |
… neso-particles to commit that contains hdf5 fix
…PTUNE/NESO into stp/cmake_refactor_to_merge
Sorry, was configuring with Builds with gcc for me, with Will's latest changes plus merge from main |
|
Right my understanding is that @seimtpow is doing a cleanup of the NP cmake that should not change functionality. Then we can look to actually merge. Edit: I propose that this is last addition to avoid feature creep and if there are any outstanding changes then they are added just before we do a v0.1.0 of NESO. I'm expecting to make a release of NP to coincide with v0.1.0 of NESO and that will be the submodule commit of NP in NESO for the release. |
|
Builds with oneapi for me too |
…PTUNE/NESO into stp/cmake_refactor_to_merge
|
Are we ready to merge now? |
It builds with gcc for me. I should do a intel build as well. |
|
Latest version builds with gcc and intel for me |
|
hipsycl cuda-nvcxx built and passed for me. |
|
Seeing this with oneapi@2022.1.0: Plus about 2k lines of missing oneapi sycl symbols. |
|
Compiles and links outside the spack env with oneapi@2022.1.0 without issue. |
|
Update: main throws the same linking errors for me when using the spack env with oneapi@2022.1.0. |
I've actually been getting that error too. I thought it was just something peculiar to my machine, but guess it's more widespread. I think the problem might arise because somehow I've gotten it building against a different release of MKL than of the compilers. I'm going to try pinning MKL to the same version as the compiler and we'll see if that fixes it. |
|
I've confirmed that the issue is with conflicting MKL versions. I'm going to see if I can modify the package file to prevent that from happening. |
|
The issue with linking for the OneAPI/MKL tool chain isn't really related to this PR. I've updated the NESO package to prevent it happening in future and will create separate PRs for that. In the meantime, I'm going to merge this. |
Description
Updating cmake to use updated neso-particles (now consumed as submodule) + some other minor updates
added AdaptiveCPP and IntelSYCL as options in FindSYCL
fixed some deprecated code that fails to compile with new intel compiler
updated neso-particles submodule to point to compatible version.