search

ExCALIBUR-NEPTUNE / NESO-Spack

Spack repository for installing NESO components and dependencies.
MIT License
2 stars 2 forks source link

Build nektar in standard Spack locations #17

Closed cmacmackin closed 1 year ago

cmacmackin commented 1 year ago

For some reason, the Nektar++ source was being copied to a directory in the install-tree and built there. This meant the installation was unnecessarily large (as it contained build artefacts) and also caused problems when trying to build Nektar++ using the development workflow. For that, CMake was being run in the top-level directory of the repository, before the copying took place (for some reason). This caused problems when it was rerun with a different compiler/toolchain.

jwscook commented 1 year ago

There was a good reason for this in https://github.com/ExCALIBUR-NEPTUNE/NESO-Spack/pull/1. Specifically this was a work around to:

  1. stop Nektar from building its own OpenBLAS / LAPACK in addition to whatever spack has.
  2. make sure NekPy works

Are these two reasons for the workaround now obsolete?

@will-saunders-ukaea might remember more details about https://github.com/ExCALIBUR-NEPTUNE/NESO-Spack/commit/ddc682e2e9fd0690d4b82f2aa8509806f785632c?

will-saunders-ukaea commented 1 year ago
  1. We wanted the objects for the solvers that ship with Nektar++ so that we didn't have copies of these sources in NESO. Ideally these equation solvers would be linked into a library as part of the nektar++ build (maybe @oparry-ukaea is looking into this?). Hence the copy of the solver objects.
  2. Nektar++'s cmake files (i.e. the ones we use to build against nektar++) were pointing to third party libraries that were built as part of the nektar++ build (e.g. blas) and the paths in these cmake files were to the build directory (not the install directory). Hence the build in the spack directory (I'm sure there is a better solution here we really need to get up an running at the time).
  3. Similar issue with NekPy build not getting copied to install directory I think.

Hence the hopefully temporary fix to build nektar in the spack install directory.

cmacmackin commented 1 year ago

I think I fixed the issue with linking against third-party libraries built as part of Nektar already. I'll have a check. The other problems would be better fixed by creating a patch to the CMake, as @oparry-ukaea did in #13, with the idea of trying to get these merged upstream eventually.

On Wed, Jan 11, 2023 at 3:47 PM Will Saunders @.***> wrote:

  1. We wanted the objects for the solvers that ship with Nektar++ so that we didn't have copies of these sources in NESO. Ideally these equation solvers would be linked into a library as part of the nektar++ build (maybe @oparry-ukaea https://github.com/oparry-ukaea is looking into this?). Hence the copy of the solver objects.
  2. Nektar++'s cmake files (i.e. the ones we use to build against nektar++) were pointing to third party libraries that were built as part of the nektar++ build (e.g. blas) and the paths in these cmake files were to the build directory (not the install directory). Hence the build in the spack directory (I'm sure there is a better solution here we really need to get up an running at the time).
  3. Similar issue with NekPy build not getting copied to install directory I think.

Hence the hopefully temporary fix to build nektar in the spack install directory.

— Reply to this email directly, view it on GitHub https://github.com/ExCALIBUR-NEPTUNE/NESO-Spack/pull/17#issuecomment-1378994356, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6ESPN7HPOFRH5ZRLTL5B3WR3IZRANCNFSM6AAAAAATYD7XRY . You are receiving this because you authored the thread.Message ID: @.***>

-- Chris MacMackin

oparry-ukaea commented 1 year ago

1. was fixed by #13 for Nektar's CompressibleFlowSolver, which I think is all we're using at the moment. That PR also added a CMake macro that makes it trivial to build libraries for Nektar's other built-in solvers, if we need them.

oparry-ukaea commented 1 year ago

For 3. Nektar has a make target called 'nekpy-install-system' which might do what we want.

cmacmackin commented 1 year ago

So @oparry-ukaea say's we've fixed (1). I've confirmed that I fixed (2) by getting Nektar to link against spack-installed versions of those libraries, rather than building them itself (see #3). To be sure, I ran ldd on the Nektar shared libraries and some of the executables to confirm they weren't linking to anything in the build directory. I also confirmed that the LinearAlgebraUnitTests-rg executable could run.

Issue (3) still appears to be an outstanding problem. I will look into that tomorrow. Probably it does just require running an extra make command or patching the CMakeLists files.

cmacmackin commented 1 year ago

I've now updated the Spack package to ensure that NekPy gets installed. That should sort out all of the problems which prompted the decision to build Nektar in the install tree. @jwscook, could you please review this now (or delegate to someone to review).