ADIOS1: Link Serial & Parallel Separately by ax3l · Pull Request #254 · openPMD/openPMD-api

ax3l · 2018-06-13T11:52:36Z

CMake draft to link the _nompi and MPI libs of ADIOS1 separately.

Close #252

Current Status 1

It's ugly, it's verbose, it's hard to maintain, it excludes compilers that can not hide symbols such as pgc++ when building with ADIOS1, the system must have shared library support, it requires static ADIOS1 libraries (at least that's the default), ... but it works :-D

Current Status 2

It's complex but doable.

To Do

ax3l · 2018-06-14T12:47:00Z

+        POSITION_INDEPENDENT_CODE ON
+        CXX_VISIBILITY_PRESET hidden
+        VISIBILITY_INLINES_HIDDEN ON
+        LINK_FLAGS "-Wl,--exclude-libs,libadios_nompi.a"


or simply -Wl,--exclude-libs,ALL

ax3l · 2018-06-15T10:06:27Z

 if(openPMD_HAVE_ADIOS1)
-    target_link_libraries(openPMD PUBLIC ${ADIOS_LIBRARIES})
-    target_include_directories(openPMD SYSTEM PUBLIC ${ADIOS_INCLUDE_DIRS})
+    add_library(openPMD.ADIOS1.Serial SHARED ${IO_ADIOS1_SEQUENTIAL_SOURCE})


ha, have to link this always xD

ax3l · 2018-06-15T12:01:11Z

@C0nsultant lol, I made it. That's the worst hack to isolate an upstream issue I ever did. I feel ashamed and a bit used 🤕

ax3l · 2018-06-15T12:07:57Z


-# external library: ADIOS1 (optional)
+#   always search for a sequential lib first, so we can mock MPI
+find_package(ADIOS 1.13.1 COMPONENTS sequential QUIET)


todo: the QUIET should be honored in FindADIOS.cmake

ax3l · 2018-06-15T12:08:45Z

+set(ADIOS_LIBRARIES_SEQUENTIAL ${ADIOS_LIBRARIES})
+set(ADIOS_INCLUDE_DIRS_SEQUENTIAL ${ADIOS_INCLUDE_DIRS})
+unset(ADIOS_FOUND CACHE)
+unset(ADIOS_VERSION CACHE)


we could use this and look for the parallel component with EXACT.
but actually it's not two, totally independent backends anyway.
nevertheless, same version makes life easier.

anokfireball · 2018-06-15T12:23:04Z

@ax3l You survived the war. Undoubtedly you have witnessed the bad and the worse, but you made it to the other side. People are going to love you for it.

Great work!

anokfireball · 2018-06-15T12:31:13Z

 endif()

+add_library(openPMD.ADIOS1.Serial SHARED ${IO_ADIOS1_SEQUENTIAL_SOURCE})
+add_library(openPMD.ADIOS1.Parallel SHARED ${IO_ADIOS1_SOURCE})


Just for better understanding:
Are all 4 combinations

SER PAR N N N Y Y N Y Y

possible with this change?

Yes indeed 💃

Even though an ADIOS install of the form

SER PAR N Y

is not build by ADIOS (it always ships at least a serial lib with its lovely mock magic).

Impressive!
... and that only requires building two seperate library files by compiling with different options and linking them with tinkered symbols.

its lovely mock magic

😂

Close: no symbol tinkering, just really aggressive symbols hiding for encapsulation 😈

And note the weird code sharing in the implementation files :-o

anokfireball · 2018-06-15T12:42:33Z

+protected:
+    int64_t m_group;
+    std::string m_groupName;
+    ADIOS_READ_METHOD m_readMethod;


If we don't use that varaible for both backends anymore, we cna probably hard-code the ADIOS_READ_METHOD used in each one.

I don't fully get this but I guess we can do this in a follow-up as well.

The methods are initialized once per handler in init()

openPMD-api/src/IO/ADIOS/ADIOS1IOHandler.cpp

Line 210 in 265828b

m_readMethod = ADIOS_READ_METHOD_BP;

openPMD-api/src/IO/ADIOS/ParallelADIOS1IOHandler.cpp

Lines 221 to 222 in 551ab6d

/** @todo ADIOS_READ_METHOD_BP_AGGREGATE */

m_readMethod = ADIOS_READ_METHOD_BP;

With the hard separation of serial and parallel backend, we do not really need the variable any more.

anokfireball · 2018-06-15T12:45:42Z

+
+#define ADIOS1IOHandlerImpl ParallelADIOS1IOHandlerImpl
+#include "ADIOS1IOCommon.cpp"
+#undef ADIOS1IOHandlerImpl


When you think you've seen all the ways of sharing code between classes...

yeah, that's my secret present for you.
did I mention it's the worst hack ever?

Note that inheritance does not work since identical symbols do not mean identical implementation thanks to the MPI mock...

I could inherit from yet another baseclass though, that we then hide again in symbols and compile twice... ahhhh... it must stop.

The reason for the duplicated code is not too diffcult to understand. Just maybe replace ADIOS1IOHandlerImpl with CommonImpl in "ADIOS1IOCommon.cpp" and then do the #defines in both handlers?

Yeah, thought so, too!

anokfireball · 2018-06-19T03:59:14Z

-public:
-#if openPMD_HAVE_MPI
-    ParallelADIOS1IOHandler(std::string const& path, AccessType, MPI_Comm);
+#if openPMD_HAVE_ADIOS1 && openPMD_HAVE_MPI


This check is redundant with the # if openPMD_HAVE_MPI below.

anokfireball · 2018-06-19T04:08:15Z

Separating the two is probably a good thing.
Using the Impl is only the recommended way to use backends, not a constraint. I placed them in the same files up to this point since all backends implemented their own Impls.

Enable serial ADIOS1 backend usage even if MPI is present.

CMake draft to link the `_nompi` and MPI libs of ADIOS1 separately.

ax3l · 2018-06-21T22:52:57Z

@C0nsultant I think we could merge this as well now. It's also hard to rebase if we keep this open for too long.

We can not inhertit, we need unique symbols.

anokfireball · 2018-06-22T03:56:30Z

+        src/IO/AbstractIOHandler.cpp
+        src/IO/AbstractIOHandlerImpl.cpp
+        src/auxiliary/Filesystem.cpp
+#        src/IO/IOTask.cpp


Remove if not required to build this library.

anokfireball · 2018-06-22T03:56:36Z

+        src/IO/AbstractIOHandler.cpp
+        src/IO/AbstractIOHandlerImpl.cpp
+        src/auxiliary/Filesystem.cpp
+#        src/IO/IOTask.cpp


Remove if not required to build this library.

anokfireball · 2018-06-22T03:58:40Z

+            CXX_EXTENSIONS OFF
+            CXX_STANDARD_REQUIRED ON
+            POSITION_INDEPENDENT_CODE ON
+            CXX_VISIBILITY_PRESET "hidden"


Is there a reasong why a string is used for CXX_VISIBILITY_PRESET here and a raw literal in openPMD.ADIOS1.Serial?

That's unfortunately syntax in CMake to set -fvisibility-inlines-hidden (or equivalent)

Ah, now I see. yes, we can skip the quotes. Updated.

anokfireball · 2018-06-22T04:08:51Z

        else()
-            message("ADIOS component ${COMP} is not supported. Please use fortran, readonly, or sequential")
+            if(NOT ADIOS_FIND_QUIETLY)
+                message("ADIOS component ${COMP} is not supported. Please use fortran, readonly, or sequential")


This is potentially a case where we could signal FATAL_ERROR.
The user is requesting a component we will never be able to provide.

Oh, quite the contrary! We are inside a Find*.cmake script. The worst thing it is allowed to do is not find what is requested.

Fatal errors are controlled by setting REQUIRED in find_package or by handling the Package_FOUND variable manually.

So we are all set :-)

anokfireball

LGTM.

Clean up debug messages and honor QUIET in FindADIOS.cmake

Do not flush Attribute value until termination of IOHandler. Variable defines do not overwrite previous definitions. Only flush the very last value as the definition. Enable VERFIY macro for ADIOS backends (was disabled with openPMD#254).

ax3l added bug third party third party libraries that are shipped and/or linked backend: ADIOS1 labels Jun 13, 2018

anokfireball mentioned this pull request Jun 13, 2018

Remove MPI from serial ADIOS interface #258

Merged

ax3l force-pushed the topic-adiosSepLinking branch 7 times, most recently from 07d5824 to 86b51cf Compare June 14, 2018 12:01

ax3l commented Jun 14, 2018

View reviewed changes

ax3l force-pushed the topic-adiosSepLinking branch from 86b51cf to c890f77 Compare June 14, 2018 23:17

ax3l mentioned this pull request Jun 15, 2018

Fix ADIOS Warnings #267

Merged

ax3l commented Jun 15, 2018

View reviewed changes

ax3l force-pushed the topic-adiosSepLinking branch 2 times, most recently from 8618b33 to 5347c1e Compare June 15, 2018 11:59

ax3l force-pushed the topic-adiosSepLinking branch 2 times, most recently from 24ff86a to 33685b1 Compare June 15, 2018 12:04

ax3l commented Jun 15, 2018

View reviewed changes

anokfireball reviewed Jun 15, 2018

View reviewed changes

ax3l mentioned this pull request Jun 15, 2018

Travis-CI: ADIOS1 Tests #190

Merged

ax3l force-pushed the topic-adiosSepLinking branch 3 times, most recently from b336a9b to 9e13f5b Compare June 18, 2018 11:06

anokfireball reviewed Jun 19, 2018

View reviewed changes

ax3l force-pushed the topic-adiosSepLinking branch 4 times, most recently from ba06b23 to 30ec032 Compare June 19, 2018 21:30

ax3l added 2 commits June 22, 2018 00:23

Changelog: ADIOS1 Serial with Parallel

105236a

Enable serial ADIOS1 backend usage even if MPI is present.

ADIOS1: Link Serial & Parallel Separately

7d7d2e3

CMake draft to link the `_nompi` and MPI libs of ADIOS1 separately.

ax3l changed the title ~~[WIP] ADIOS1: Link Serial & Parallel Separately~~ ADIOS1: Link Serial & Parallel Separately Jun 21, 2018

ax3l force-pushed the topic-adiosSepLinking branch from 30ec032 to 2e9dec8 Compare June 21, 2018 22:52

ax3l assigned anokfireball Jun 21, 2018

Symbol detangling

fc3d9b8

We can not inhertit, we need unique symbols.

ax3l force-pushed the topic-adiosSepLinking branch from 2e9dec8 to e8c81d8 Compare June 21, 2018 22:57

anokfireball reviewed Jun 22, 2018

View reviewed changes

anokfireball approved these changes Jun 22, 2018

View reviewed changes

ax3l force-pushed the topic-adiosSepLinking branch from e8c81d8 to e9e80a5 Compare June 22, 2018 06:18

CMake: Less Verbose

c54f6a1

Clean up debug messages and honor QUIET in FindADIOS.cmake

ax3l force-pushed the topic-adiosSepLinking branch from e9e80a5 to c54f6a1 Compare June 22, 2018 06:19

ax3l merged commit b85a155 into openPMD:dev Jun 22, 2018

ax3l deleted the topic-adiosSepLinking branch June 22, 2018 07:38

anokfireball mentioned this pull request Aug 29, 2018

Linker error when switching off MPI (and hence ADIOS1) #317

Closed

ax3l mentioned this pull request Jan 5, 2019

ADIOS1 Serial Link Trick Violates ODR for vtable #413

Closed

ax3l mentioned this pull request Jan 16, 2019

IOTask Parameters: Export Symbols #442

Merged

ax3l mentioned this pull request Feb 16, 2019

CMake: Optional ADIOS1 Wrapper Libs #472

Merged

	/** @todo ADIOS_READ_METHOD_BP_AGGREGATE */
	m_readMethod = ADIOS_READ_METHOD_BP;

Uh oh!

Conversation

ax3l commented Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current Status 1

Current Status 2

To Do

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax3l commented Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax3l Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anokfireball commented Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anokfireball Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax3l Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax3l Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax3l Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anokfireball commented Jun 19, 2018

Uh oh!

ax3l commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anokfireball Jun 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ax3l commented Jun 13, 2018 •

edited

Loading

ax3l commented Jun 15, 2018 •

edited

Loading

ax3l Jun 15, 2018 •

edited

Loading

anokfireball commented Jun 15, 2018 •

edited

Loading

anokfireball Jun 15, 2018 •

edited

Loading

ax3l Jun 15, 2018 •

edited

Loading

ax3l Jun 15, 2018 •

edited

Loading

ax3l Jun 15, 2018 •

edited

Loading

ax3l commented Jun 21, 2018 •

edited

Loading

anokfireball Jun 22, 2018 •

edited

Loading