ADIOS1 backend #126

C0nsultant · 2018-04-11T15:52:26Z

This adds both a serial and parallel ADIOS1 backend.

One potential drawback of this backend is that it can only mutually exclusively build a non-MPI/parallel version. This might induce contraints on other backends as well, depending on how they are built.

The implementation in this PR does the bare minimum to use ADIOS1 effectively:

It considers the typical ADIOS workflow of defining variables before opening a file and tries to reduce the number of times a file has to be (re-)opened. To do this, file opening is deferred until the first write must happen. Two IO queues are used in the backend to discern the two phases of operations (before / after file open). One condition under which re-opening a file can not be avoided is when a manual Series::flush() is called before new a variable is defined (there may be more complex ones that we have not recognized yet).
It bunches reads for every file to schedule a combined read of multiple chunks.
It keeps file handles open as long as is possible. It also reopenes them when required.
It allows to write dynamically sized chunks per dataset. Typically, ADIOS recommends to define a unique variable for every different-sized/-localized chunk for every dataset. We solve this by creating scalar varaibles for every every dimension of the extent and offset (in "/tmp", so it does not conflict with the openPMD standard). After being defined once, these variables are re-used as the extent and offset for any chunk of a dataset. To do this, the scalar variables just have to be overwritten with the desired extent and offset values before performing the chunk write.

ax3l · 2018-04-12T08:22:33Z

CMakeLists.txt

@@ -41,7 +41,7 @@ endfunction()

 openpmd_option(MPI    "Enable MPI support"     AUTO)
 openpmd_option(HDF5   "Enable HDF5 support"    AUTO)
-openpmd_option(ADIOS1 "Enable ADIOS1 support"  OFF)
+openpmd_option(ADIOS1 "Enable ADIOS1 support"  AUTO)


also occurs in README.md, docs/source/dev/buildoptions.rst and docs/source/dev/dependencies.rst

Determine and check the parallel/serial status of a found ADIOS1 lib

ax3l · 2018-04-12T15:57:41Z

CMakeLists.txt

-#endif()
+# TODO we could support more combinations than MPI+pADIOS and noMPI+sADIOS
+if(openPMD_HAVE_MPI AND openPMD_HAVE_ADIOS1 AND ADIOS_HAVE_SEQUENTIAL)
+    message(FATAL_ERROR "Found MPI but requested ADIOS1 is serial. "


are you sure a MPI-parallel ADIOS not not still ship a "sequential" component "as well"?

The library itself might provide both, but with the used FindAdios.cmake only mutually exclusive versions get linked. Since ADIOS has a unified interface (with dummy MPI if serial), I doubt you can link both simultaneously.

ax3l · 2018-04-12T16:06:33Z

cmake/FindADIOS.cmake

@@ -178,6 +179,9 @@ if(ADIOS_FOUND)
    endforeach()
    # we could append ${CMAKE_PREFIX_PATH} now but that is not really necessary

+    # determine whether found library is serial only


I think we can only determine:

is serial and parallel

is serial only

Alright, the vocabulary in this comment is not sufficient. This determines if the library links as serial only with the specified components.
The library itself might be serial + parallel.

ok cool.

want I wanted to say is just: if you build serial, the lib/ dir will contain a regular and a _nompi version, as far as I remember.

If you check that only the noMPI is linked this is fine, but you might want to prefer it someway during search and validate it with two adios installations (/w and /wo MPI), printing all vars to make sure. Also includes (and their #defines) should match.

Bump required ADIOS1 version to 1.13.0. Enable serial ADIOS1 through -D_NOMPI. Enable tests according to used backends.

Move ADIOS1 logic form parallel to serial (dummy MPI allows for mostly the same code).

ax3l · 2018-04-15T16:13:41Z

.travis/spack/packages.yaml

@@ -16,6 +16,9 @@ packages:
      openmpi@1.6.5%gcc@7.2.0 arch=linux-ubuntu14-x86_64: /usr
      openmpi@1.6.5%clang@5.0.0 arch=linux-ubuntu14-x86_64: /usr
    buildable: False
+packages:


oh oh, pls remove this single line

ax3l · 2018-04-15T20:15:46Z

I think there is more to do for the CI than we can do in the next few days.
Just leave it off for now (remove the last three commits) and we will add travis for ADIOS in a follow-up together end of the week.

C0nsultant · 2018-04-16T05:12:17Z

This is probably not a problem with our build process. The errors are caused by timeouts during download of our dependencies (e.g. https://zlib.net was down yesterday). Now, Boost & HDF5 are horribly slow, but making progress.

ax3l · 2018-04-16T05:19:18Z

Yes, also that but I restarted them later on and saw some other issues, also spack-dependency-wise, that I need to investigate properly.

Remove not yet implemented status for ADIOS1. Bump required ADIOS1 version.

ax3l · 2018-04-16T14:46:18Z

include/openPMD/auxiliary/Memory.hpp

@@ -83,6 +99,9 @@ allocatePtr(Datatype dtype, size_t numPoints)
            data = new bool[numPoints];
            del = [](void* p){ delete[] static_cast< bool* >(p); p = nullptr; };
            break;
+        case DT::STRING:
+            /* user assings c_str pointer */


typo: assigns

ax3l · 2018-04-16T15:03:50Z

src/IO/ADIOS/ADIOS1IOHandler.cpp

+                          dims.c_str(),
+                          global_dims.c_str(),
+                          local_offsets.c_str());
+    ASSERT(id >= 0 /* ??? */, "Internal error: Failed to define ADIOS variable during Dataset writing");


good candidate for a VERIFY macro - seen this a lot in the last on startup/env issues.

Very much agreed. We should not exclude these assertions in production code, unless the user really wants to.

ax3l · 2018-04-16T15:09:38Z

src/IO/ADIOS/ADIOS1IOHandler.cpp

+            break;
+        }
+        case DT::BOOL:
+            throw std::runtime_error("No workaround for ADIOS1 bool implemented");


Current PIConGPU representation: ComputationalRadiationPhysics/picongpu#1756

also just as ints, unfortunately.

Currently tinkering with this. Adapting the behaviour of using unsigned bytes for bools means we can store them but not read them back typesafe.
Having every uint8_t of value 0 be reinterpreted to a bool false and every uint8_t of value 1 be reinterpreted to a bool true is undesirable when reading a file.

…opic/adios

Dataset opening, path & dataset listing. Change frontend logic to rely less on HDF5 backend implementation quirks. Working ADIOS test case.

Working ADIOS read test case. ADIOS attribute datatype test case.

To fit ADIOS's workflow better and to avoid redundant file open/file closing, separate the IO operations in ADIOS1 backend into two queues: Defines and file context preperation (m_setup) that gets drained before a file is opened for writing. Actual data transfers that require an open file but do not re-open it ( m_work).

1.13.1 is very mich desired.

ax3l · 2018-04-19T15:58:51Z

include/openPMD/auxiliary/StringManip.hpp

@@ -126,8 +126,9 @@ join(std::vector< std::string > const& vs, std::string const& delimiter)
        default:
            std::ostringstream ss;
            std::copy(vs.begin(),
-                      vs.end(),
+                      vs.end() - 1,


o.0

this changes looks a bit unsave, e.g. for zero-lengths :)

offline discussion: add comment // do not append separator after last element

The empty and size 1 vector cases are handled above ;)

yep, saw this too late :)

ax3l · 2018-04-23T11:45:23Z

@C0nsultant can you pls add a little to-do list in the PR description what is still missing in this PR? :) Just curious :)

C0nsultant · 2018-04-23T18:14:16Z

I added the major two things that have me refrain from merging yet.
Nothing major, at least the testing can be done in a few hours (should have time for that tomorrow).

Adapt the different ADIOS workflow to work with fileBased output. This requires closing and flushing open handles as ADIOS handles are only one-directional (in- or out-put).

ax3l · 2018-04-23T23:06:28Z

sounds good.
leave the CI out for now, it looks still buggy and I had no time to progress on it.

Treat booleans as unsigned bytes. This allows us to write them, but not read them back as bool (they are uint8_t).

Include new matcher functionality in tutorial.

ADIOS1 does not offer deletion inside files, so handle those request non-gracefully.

ax3l · 2018-04-26T15:06:51Z

can only mutually exclusively build a serial/parallel version

@C0nsultant but if I build the parallel version and satisfy the MPI dependency e.g. when running in mpiexec - can I still create a serial Series from a single rank? This should work, no?

ax3l · 2018-04-26T15:10:02Z

CMakeLists.txt

@@ -301,6 +303,10 @@ if(openPMD_HAVE_ADIOS1)
    target_link_libraries(openPMD PUBLIC ${ADIOS_LIBRARIES})
    target_include_directories(openPMD SYSTEM PUBLIC ${ADIOS_INCLUDE_DIRS})
    target_compile_definitions(openPMD PUBLIC "-DopenPMD_HAVE_ADIOS1=1")
+    if(ADIOS_HAVE_SEQUENTIAL)
+        # TODO might be smarter to get ALL definitions from adios-config -s & parse it


Yes, ideally in FindADIOS.cmake we should populate ADIOS_DEFINITIONS. We then use that var here.

ax3l · 2018-04-26T15:12:01Z

CMakeLists.txt

-    if(${examplename} MATCHES ".+parallel$")
-        if(openPMD_HAVE_MPI)
+# Current examples all use HDF5, elaborate if other backends are used
+if(openPMD_HAVE_HDF5)


don't exclude the build, just the test below is fine.
the examples should still be build and only throw errors at runtime if they try to use a backend that does not exist.

I included the examples as tests in CI. With this approach, CI tests without HDF5 will always fail.

no, the add_tests are in the block below and properly guarded. Those are the runtimeand require HDF5 files and backend. but the compile should still be done.

ax3l · 2018-04-26T15:14:56Z

include/openPMD/IO/ADIOS/ADIOS1Auxiliary.hpp

+        case DT::UNDEFINED:
+            throw std::runtime_error("Unknown Attribute datatype");
+        default:
+            throw std::runtime_error("Datatype not implemented in HDF5 IO");


ax3l · 2018-04-26T15:17:44Z

include/openPMD/Series.hpp

+/** Create a functor to determine if a file can be of a format given the filename on disk.
+ *
+ * @param   name        String containing desired filename without filename extension.
+ * @param   f           File format to check plausibility for.


Maybe write "backend applicability" instead of "plausibility"?

ax3l · 2018-04-26T15:32:34Z

src/IO/ADIOS/ADIOS1IOHandler.cpp

+            break;
+        case DT::UNDEFINED:
+        case DT::DATATYPE:
+            throw std::runtime_error("Unknown Attribute datatype");


I am wondering if one has still to break after a throw...?
probably, no?

Execution at that point aborts until you reach the next mathcing catch (if it reaches the top level, the process aborts). And even inside that catch, you can not possilbly recover to the point of failure (unless you use goto-fuckery).

So no, there is no break required after a throw just like after a return.

ok, I was more wondering if the default can be reached without the break.

ax3l · 2018-04-26T15:34:04Z

src/IO/ADIOS/ADIOS1IOHandler.cpp

+                for( int i = 0; i < size; ++i )
+                {
+                    vs[i] = auxiliary::strip(std::string(c[i], std::strlen(c[i])), {'\0'});
+                    /* TODO pointer should be freed, but this causes memory curruption */


make this a doxygen todo please: /** @todo ...

ax3l · 2018-04-26T15:36:31Z

src/IO/ADIOS/ParallelADIOS1IOHandler.cpp

+    char const* c = params.str().c_str();
+
+    int status;
+    /* TODO ADIOS_READ_METHOD_BP_AGGREGATE */


//! @todo ...

ax3l · 2018-04-26T15:39:46Z

test/SerialIOTest.cpp

+    REQUIRE(s.getAttribute("vecDouble").get< std::vector< double > >() == std::vector< double >({0., 1.79769e+308}));
+    REQUIRE(s.getAttribute("vecLongdouble").get< std::vector< long double > >() == std::vector< long double >({0.L, 1.18973e+4932L}));
+    REQUIRE(s.getAttribute("vecString").get< std::vector< std::string > >() == std::vector< std::string >({"vector", "of", "strings"}));
+    REQUIRE(s.getAttribute("bool").get< uint8_t >() == static_cast< uint8_t >(true));


can you add a comment on bool reads please?
maybe we also want to write this in the manual in a section on "backends: features & limitations" or so.

ax3l · 2018-04-26T15:40:56Z

test/SerialIOTest.cpp

+
+TEST_CASE( "hzdr_adios1_sample_content_test", "[serial][adios1]" )
+{
+    // since this file might not be publicly available, gracefully handle errors


please add /** @todo add bp example files to https://github.com/openPMD/openPMD-example-datasets */

C0nsultant · 2018-04-26T17:00:04Z

but if I build the parallel version and satisfy the MPI dependency e.g. when running in mpiexec - can I still create a serial Series from a single rank? This should work, no?

Yes. Yes, that always works with single-size MPI communicators.
Let's rephrase that to non-MPI and parallel versions.

Use adios_config -c to get compile definitions.

…opic/adios

ax3l · 2018-05-08T07:11:42Z

Funny, I just saw this: https://github.com/ornladios/ADIOS/blob/v1.13.1/KNOWN_BUGS

That means writing a var for each chunk's dimensions is indeed the right way to go in ADIOS1 ;)

Build Skeleton of ADIOS1 backend

b34628c

C0nsultant added backend: ADIOS1 MPI labels Apr 11, 2018

ax3l self-requested a review April 12, 2018 08:19

ax3l reviewed Apr 12, 2018

View reviewed changes

ax3l mentioned this pull request Apr 12, 2018

[WIP] Travis: ADIOS1 #127

Closed

C0nsultant added 2 commits April 12, 2018 13:49

Get desired ADIOS1 version through CMake

ec8ee1a

Determine and check the parallel/serial status of a found ADIOS1 lib

ADIOS1 path & dataset creation (untested)

3b449d2

ax3l reviewed Apr 12, 2018

View reviewed changes

C0nsultant added 2 commits April 13, 2018 11:47

Serial ADIOS1 integration

d0783a8

Bump required ADIOS1 version to 1.13.0. Enable serial ADIOS1 through -D_NOMPI. Enable tests according to used backends.

ADIOS1 attribute writing

7b7e291

Move ADIOS1 logic form parallel to serial (dummy MPI allows for mostly the same code).

ax3l self-assigned this Apr 15, 2018

ax3l added this to To do in First Stable Release via automation Apr 15, 2018

ax3l reviewed Apr 15, 2018

View reviewed changes

C0nsultant moved this from To do to In progress in First Stable Release Apr 15, 2018

C0nsultant force-pushed the topic/adios branch from 8b16750 to 7b7e291 Compare April 16, 2018 08:31

C0nsultant and others added 4 commits April 16, 2018 12:03

ADIOS dataset/chunk writing

624b8a2

Adapt error messages to context

213784f

Update documentation

8da2f56

Remove not yet implemented status for ADIOS1. Bump required ADIOS1 version.

Merge branch 'dev' into topic/adios

5e69e22

ax3l reviewed Apr 16, 2018

View reviewed changes

C0nsultant added 5 commits April 16, 2018 20:46

ADIOS attribute reading

67f7da3

Merge branch 'topic/adios' of github.com:C0nsultant/libopenPMD into t…

26b5ab8

…opic/adios

ADIOS attribute listing, path opening

17a4e1e

ADIOS file reading

77d476e

Dataset opening, path & dataset listing. Change frontend logic to rely less on HDF5 backend implementation quirks. Working ADIOS test case.

ADIOS dataset reading

3c5dde5

Working ADIOS read test case. ADIOS attribute datatype test case.

C0nsultant added 6 commits April 19, 2018 12:02

Bump required ADIOS version

48e0d48

1.13.1 is very mich desired.

Copy behaviour to Parallel ADIOS1

d11a33b

Fix compilation when ADIOS is missing

44d21c3

Switch to MPI_AGGREGATE

916c2ca

ADIOS test cases

043ea05

ax3l reviewed Apr 19, 2018

View reviewed changes

Fix ADIOS fileBased Series logic

ea7fe5e

Adapt the different ADIOS workflow to work with fileBased output. This requires closing and flushing open handles as ADIOS handles are only one-directional (in- or out-put).

ADIOS bool workaround

ccd9351

Treat booleans as unsigned bytes. This allows us to write them, but not read them back as bool (they are uint8_t).

C0nsultant changed the title ~~[WIP] ADIOS1 backend~~ ADIOS1 backend Apr 26, 2018

C0nsultant and others added 5 commits April 26, 2018 11:40

Use Tranform parameters on Datasets

193fefb

Correct error text

42127ac

Update Howto backend documentation

e8fecb9

Include new matcher functionality in tutorial.

Add non-functional ADIOS backend commands

56386af

ADIOS1 does not offer deletion inside files, so handle those request non-gracefully.

Merge branch 'dev' into topic/adios

048758d

ax3l reviewed Apr 26, 2018

View reviewed changes

C0nsultant added 3 commits April 27, 2018 14:04

Parse ADIOS compile definitions

c363f46

Use adios_config -c to get compile definitions.

Cleanup

c78a314

Merge branch 'topic/adios' of github.com:C0nsultant/libopenPMD into t…

c1b9ed4

…opic/adios

C0nsultant merged commit 0b17f15 into openPMD:dev Apr 27, 2018

First Stable Release automation moved this from In progress to Done Apr 27, 2018

C0nsultant deleted the topic/adios branch April 27, 2018 13:12

This was referenced Jun 22, 2018

NetCDF backend #33

Open

Add ADIOS example Datasets openPMD/openPMD-example-datasets#13

Open

ADIOS1 backend #126

ADIOS1 backend #126

Conversation

C0nsultant commented Apr 11, 2018 • edited

Choose a reason for hiding this comment

ax3l Apr 12, 2018 • edited

Choose a reason for hiding this comment

C0nsultant Apr 13, 2018 • edited

Choose a reason for hiding this comment

ax3l Apr 12, 2018 • edited

Choose a reason for hiding this comment

C0nsultant Apr 13, 2018 • edited

Choose a reason for hiding this comment

ax3l Apr 13, 2018 • edited

Choose a reason for hiding this comment

ax3l Apr 15, 2018 • edited

Choose a reason for hiding this comment

ax3l commented Apr 15, 2018 • edited

C0nsultant commented Apr 16, 2018

ax3l commented Apr 16, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C0nsultant Apr 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C0nsultant Apr 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ax3l commented Apr 23, 2018

C0nsultant commented Apr 23, 2018

ax3l commented Apr 23, 2018

ax3l commented Apr 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ax3l Apr 27, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C0nsultant Apr 26, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C0nsultant commented Apr 26, 2018

ax3l commented May 8, 2018 • edited

C0nsultant commented Apr 11, 2018 •

edited

ax3l Apr 12, 2018 •

edited

C0nsultant Apr 13, 2018 •

edited

ax3l Apr 12, 2018 •

edited

C0nsultant Apr 13, 2018 •

edited

ax3l Apr 13, 2018 •

edited

ax3l Apr 15, 2018 •

edited

ax3l commented Apr 15, 2018 •

edited

C0nsultant Apr 24, 2018 •

edited

C0nsultant Apr 20, 2018 •

edited

ax3l Apr 27, 2018 •

edited

C0nsultant Apr 26, 2018 •

edited

ax3l commented May 8, 2018 •

edited