Skip to content
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

running tests against installed ADIOS2 library #3795

Closed
drew-parsons opened this issue Sep 9, 2023 · 18 comments · Fixed by #3906
Closed

running tests against installed ADIOS2 library #3795

drew-parsons opened this issue Sep 9, 2023 · 18 comments · Fixed by #3906
Assignees
Labels
area: build Build issues

Comments

@drew-parsons
Copy link

drew-parsons commented Sep 9, 2023

edit
The original error reported here (deleted function ‘adios2::ADIOS::ADIOS(void*)’) was apparently a misconfiguration. The issue title has been changed to indicate the actual goal: running tests against an installed ADIOS2 library for linux distribution CI testing

Describe the bug

I'm trying to test an MPI build of adios2 by building the examples against the installed library.
bpFWriteCRead from examples/hello fails with the error:

[ 25%] Building CXX object CMakeFiles/CppWriter.dir/CppWriter.o
/usr/bin/mpicxx    -MD -MT CMakeFiles/CppWriter.dir/CppWriter.o -MF CMakeFiles/CppWriter.dir/CppWriter.o.d -o CMakeFiles/CppWriter.dir/CppWriter.o -c /projects/mathlibs/build/adios2/examples/hello/bpFWriteCRead/CppWriter.cpp
/projects/mathlibs/build/adios2/examples/hello/bpFWriteCRead/CppWriter.cpp: In function ‘int main(int, char**)’:
/projects/mathlibs/build/adios2/examples/hello/bpFWriteCRead/CppWriter.cpp:35:39: error: use of deleted function ‘adios2::ADIOS::ADIOS(void*)’
   35 |     adios2::ADIOS adios(MPI_COMM_WORLD);
      |                                       ^
In file included from /usr/include/adios2.h:27,
                 from /projects/mathlibs/build/adios2/examples/hello/bpFWriteCRead/CppWriter.cpp:11:
/usr/include/adios2/cxx11/ADIOS.h:87:5: note: declared here
   87 |     ADIOS(void *define_ADIOS2_USE_MPI_to_use_MPI_constructor) = delete;
      |     ^~~~~
make[2]: *** [CMakeFiles/CppWriter.dir/build.make:76: CMakeFiles/CppWriter.dir/CppWriter.o] Error 1
make[2]: Leaving directory '/projects/mathlibs/build/adios2/examples/hello/bpFWriteCRead/build'

So CppWriter.cpp is trying to use adios2::ADIOS::ADIOS(void*), which is no longer the correct API.

To Reproduce

  1. Go to 'examples/hello/bpFWriteCRead'
  2. Create a new subdir build for the build
  3. cd build; cmake -DCMAKE_CXX_COMPILER=mpicxx ..
  4. make
  5. See error

Expected behavior

Examples should build and run successfully.

Desktop (please complete the following information):

  • OS/Platform: Linux (debian unstable)
  • Build gcc 13.2.0, cmake 3.27.4, OpenMPI 4.1.5, build type: shared
  • adios2 2.8.2

Additional context

When running cmake to configure the example, I had to specify -DCMAKE_CXX_COMPILER=mpicxx since the adios installation only links to OpenMPI's libmpi.so without libmpi_cxx.so. This feels like an error in the installed adios cmake files. If I'm accessing adios2 built with MPI support, it should know it needs to link libmpi_cxx.so without requiring CMAKE_CXX_COMPILER=mpicxx to be specified. Or would it be an error in my build configuration?

@drew-parsons drew-parsons changed the title bpFWriteCRead uses deleted function bpFWriteCRead uses deleted function adios2::ADIOS::ADIOS(void*) Sep 9, 2023
@eisenhauer
Copy link
Member

The items under the examples directory build fine in our continuous integration, so it might take a bit of detective work to see what might be happening. First, the CMakeLists.txt in examples/hello/bpFWriteCRead isn't a standalone CMake file as it is meant to be a leaf in the CMake tree starting with the top-level ADIOS CMakeLists.txt. (The examples are really meant to showcase the API, not the build spec.). Did you edit the CMakeLists.txt to find the ADIOS installation, etc? If so, can you provide it?

Generally, that different versions of MPI use different underlying types as handles to the MPI communicator, coupled with overloaded versions of ADIOS init, causes a ton of grief because invocations that are easily resolvable in one setup look ambiguous to C++ in another. The deletions of certain initializers is part of an attempt to resolve those problems, but clearly there is an issue with this particular combination, it just may take a bit to get to the underlying problem and a resolution.

@vicentebolea This issue might be more in the Kitware wheelhouse...

@drew-parsons
Copy link
Author

That's a good point about standalone builds of the examples. I was actually about to file a separate bug about that. The tests managed by the testing subdir pass fine when run (via ctest) at build time. But what I'm trying to do is run tests (or examples) against the installed adios2 library, i.e. testing at "runtime" not "buildtime".

Ultimately, what I'm aiming to do is set up Linux distribution CI testing for packages built for debian (the general debian CI context is https://ci.debian.net/), to confirm that adios2 packages will continue to work correctly when other packages (such as gcc, openmpi) get updated. Ideally I would use the testing subdir against installed packages, but as you say, the ctest organisation there is tightly bound to the original cmake build. In particular standalone tests fail since ADIOSFunctions is not included in the installed library. I suspect an independent test build could succeed if it were possible to independently tell cmake where ADIOSFunctions.cmake. So far I haven't succeeded doing this. cmake is not simple if you try to do something "outside the box" (like running ctest against installed libraries rather than built libraries)

What I've got at this point is a direct build of the example files (not via testing), and I can run them manually one by one, apart from the bpFWriteCRead case reported here. As you guessed, I needed to hack examples/CMakeLists.txt to build against the installed libraries. The hack is simple, inject

cmake_minimum_required(VERSION 3.12)
ENABLE_LANGUAGE(Fortran)
find_package(adios2 REQUIRED)

at the top of examples/CMakeLists.txt (it fails without a minimum cmake version. Other versions like 3.5 will work, but I figure it's best to be consistent with toplevel CMakeLists.txt. It fails with no minimum version at all). I needed to explicitly enable Fortran since otherwise only C and CXX is recongnised as loaded (CMAKE_${lang}_COMPILER_LOADED) by adios2-config-common.cmake. is After that, cmake -DCMAKE_CXX_COMPILER=mpicxx .. works in examples/build, but fails without the -DCMAKE_CXX_COMPILER=mpicxx option, as reported in the "Additional context" above.

The same 3 lines of patch do enable bpFWriteCRead to build (standalone) when applied directly to examples/hello/bpFWriteCRead/CMakeLists.txt. It looks like my cache might have been inconsistent when I got the deleted function error before. I can't reproduce it now.

So it looks like my problem as-reported was the result of a confused build state (this is the first time we're building debian packages for adios2, so ironing out the issues one by one. I apologise for the noise in this regard.

Rather than keeping this issue as a problem with a deleted function, could we repivot the issue to discuss enabling testing of an installed library? How should ADIOSFunctions.cmake be handled if the aim is to run ctest in the testing subdir, building against an installed library (adding find_package(adios2 REQUIRED))

@eisenhauer
Copy link
Member

eisenhauer commented Sep 10, 2023

So, ADIOS actually does (or tries to do) testing of installation, linking against installed libraries, etc. The fellow that did that CMake magic for us has left for sunnier climes, but I'm wondering if the setup that does that can be adapted to your needs. Can you take a look under ADIOS2/testing/install? If there are issues, @vicentebolea is taking the lead on the CMake bits of ADIOS these days and is probably the best contact.

Ah, reading more closely I see you had already found the testing/install subdir. I'm going to punt this to @vicentebolea because I'm not familiar with ADIOSFunctions.cmake and it'd take me a while to get up to speed.

@drew-parsons
Copy link
Author

drew-parsons commented Sep 10, 2023

I tried adding find_package(adios2 REQUIRED) etc to testing/install/CMakeLists.txt, but that's not enough on its own. cmake passes but it doesn't configure any tests to build and run. It'll need something more to run standalone without the toplevel CMakeLists.txt, unless there's an option to use at toplevel to only build tests using an existing installation without building the whole library.

@drew-parsons drew-parsons changed the title bpFWriteCRead uses deleted function adios2::ADIOS::ADIOS(void*) running tests against installed ADIOS2 library Sep 10, 2023
@vicentebolea
Copy link
Collaborator

@drew-parsons many thanks for taking care of the ADIOS2 debian pkg.

this is the first time we're building debian packages for adios2

This is great, one of our goals is to have an ADIOS2 debian package. We have been doing some work in this regard, I wonder if we could chat further about this to see how can we help with this with the work already done. @scottwittenburg

X-ref: #2539

With regards exporting/installing ADIOSFunctions.cmake in a installation prefix. This will not work since the "testing" cmake bits of ADIOS2 are: Located in more cmake files than ADIOSFunctions.cmake`, its written assuming that you are running cmake/ctest from a build directory.

As per running the ADIOS2 tests against installed ADIOS2 library, in principle this can be as easy as building tests normally in an ADIOS2 build and then change their RPATH to point to a directory where an adios2 install tree (libdir) is located. This will work as long as you are using ABI compatible adios2 versions. To try to build the adios2 unit tests from an adios2 installed tree will be an arduous task if not impossible in the current state.

As per the testing/install contract tests. That can be something since they have their own "standalone cmakelists.txt", meaning that you could possibly extract from there the example applications.

but it doesn't configure any tests to build and run

This is odd, normally each of this install tests will configure and make independently

I think that the following can be a good course of actions for these contract testings for the ADIOS2 debian package:

  • As a first step try to build/run: testing/install/C/ and testing/install/CXX11. This will be a sort of an smoke tests.
  • Later, we are in the works of adding a few more examples in the ADIOS2-examples project. This examples are for the purpose of a tutorial meaning that each of them will have their standalone cmakelists.txt so that the user can also see how to build in cmake against adios2.
  • It might be worth to have our adios2/examples (no adios2-examples) have their own standalone Cmakelists.txt so that future users can plug-and-play their source code and also cmake files.

@drew-parsons
Copy link
Author

As far as the packaging goes, it seems to be successful via https://salsa.debian.org/science-team/adios2/-/tree/master/debian . I built on work started earlier by Kyle Edwards from Kitware at https://gitlab.kitware.com/debian/adios2

I set it up to build and run selected examples (checking that they run without segfaults, rather than checking correctness of results). We can keep working on getting the actual tests running. I started packaging with adios2 2.8, so I don't yet have any fixes that might have been made to to the Cmakelists.txt in adios2 2.9.

@drew-parsons
Copy link
Author

I see what you mean about testing/install/C/ and testing/install/CXX11 . They both configure, make and run successfully (ctest runs and passes) without needing to hack their CMakeLists.txt.

testing/install/Fortran
testing/install/EncryptionOperator
testing/install/EnginePlugin
all pass as well.

From the install subdir, it's only testing/install/CMakeLists.txt that configures and builds nothing.

@vicentebolea
Copy link
Collaborator

@drew-parsons is that package in any Debian repository, if not is there any plan of submitting the package to Debian? And when that happen would it be in Debian Experimenta/Unstable before being in a numbered release?

@drew-parsons
Copy link
Author

drew-parsons commented Sep 11, 2023

It's now submitted, it's in the NEW queue. The ftp masters will check it for licence and general quality control before aceepting. It'll go to unstable and migrate soon after to testing if no problems are discovered.

@drew-parsons
Copy link
Author

drew-parsons commented Sep 17, 2023

xsimd has a simple pattern for running its test suite against its installed library, https://github.com/xtensor-stack/xsimd/blob/master/test/CMakeLists.txt

if (CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
    project(xsimd-test)

    enable_testing()

    find_package(xsimd REQUIRED CONFIG)
    set(XSIMD_INCLUDE_DIR ${xsimd_INCLUDE_DIRS})
endif ()

@vicentebolea
Copy link
Collaborator

Thanks for pointing out an example of howto have the tests as an standalone app. We are attempting to do this with the adios2-examples.

For the adios2 unit tests while doable it will not be as easy as shown in the snippet since the adios unit tests component is strongly intertwined with the rest of the components (in many levels), we would have to workout this and make adios2 to properly export these components in its installation, we would also have to add CI support for this so that it keeps this way.

I still think it will be beneficial to have this in the future. For the time being, we can soon use the examples as a smoke test without any change. (we do this for other packages managers: Spack;SLES). In Conda we do just run the unit tests.

@drew-parsons
Copy link
Author

drew-parsons commented Sep 18, 2023

It will work at the level of testing/install at least, since these install tests run standalone via ctest.
I tried it, adding the subdirs to one of these if blocks patched into testing/install/CMakeLists.txt. It works.
(need to set it up as if()... else() or else the existing custom tests fail).

@vicentebolea
Copy link
Collaborator

It will work at the level of testing/install at least, since these install tests run standalone via ctest.

That its great, yes they are standalone test anyways. I was referring to the whole unit tests.

@spyridon97
Copy link
Contributor

@vicentebolea is this issue closed?

@vicentebolea
Copy link
Collaborator

@spyridon97 thanks for promptly pointing this but not yet. Currently in the CI, we build the examples as part of ADIOS2, we need to do some changes later on to make the CI to build the examples outside of ADIOS2 build and use an existing adios2 installation.

@spyridon97
Copy link
Contributor

@spyridon97 thanks for promptly pointing this but not yet. Currently in the CI, we build the examples as part of ADIOS2, we need to do some changes later on to make the CI to build the examples outside of ADIOS2 build and use an existing adios2 installation.

So we basically need to create another CI target that builds all the tutorials using an external ADIOS installation?

@vicentebolea
Copy link
Collaborator

So we basically need to create another CI target that builds all the tutorials using an external ADIOS installation?

Yep, something similar to tau or scorpion build, you can have a look at the everything.yml

@vicentebolea
Copy link
Collaborator

Note that we also want to run the examples in that build too we do not test things there but we can make sure that they do not crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants