Adios2 warn groupbased encoding (#1498)

* Warning: group-based iteration encoding in ADIOS2

* Remove adios2.usesteps, set it always to true

This uncovered loads of bugs

* Remove requireActiveStep

* Remove StreamStatus::Parsing

* Remove usesteps = true/false from tests

* Fix CI error

* Testing: ADIOS2 < v2.9 compatibility

* Less misleading warning message

* Unify struct/class

* Transition a bit more leniently (part 1)

* Transition a bit more leniently (part 2)

* Remove usesteps option from documentation

* Cleanup and fixes

* Fix tests

* Fix the warning text

Author: franzpoeschel

CMakeLists.txt

@@ -1360,16 +1360,18 @@ if(openPMD_BUILD_TESTING)
                         ${MPI_TEST_EXE} ${Python_EXECUTABLE}                       \
                             ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe       \
                             --infile ../samples/git-sample/data00000100.h5         \
-                            --outfile ../samples/git-sample/single_iteration.bp && \
+                            --outfile                                              \
+                                ../samples/git-sample/single_iteration_%T.bp &&    \
                                                                                    \
                         ${MPI_TEST_EXE} ${Python_EXECUTABLE}                       \
                             ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe       \
                             --infile ../samples/git-sample/thetaMode/data%T.h5     \
-                            --outfile ../samples/git-sample/thetaMode/data.bp &&   \
+                            --outfile                                              \
+                                ../samples/git-sample/thetaMode/data_%T.bp &&      \
                                                                                    \
                         ${Python_EXECUTABLE}                                       \
                             ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe       \
-                            --infile ../samples/git-sample/thetaMode/data.bp       \
+                            --infile ../samples/git-sample/thetaMode/data_%T.bp    \
                             --outfile ../samples/git-sample/thetaMode/data%T.json  \
                         "
                     WORKING_DIRECTORY ${openPMD_RUNTIME_OUTPUT_DIRECTORY}
@@ -1378,17 +1380,17 @@ if(openPMD_BUILD_TESTING)
                 add_test(NAME CLI.pipe.py
                     COMMAND sh -c
                         "${Python_EXECUTABLE}                                      \
-                            ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe         \
+                            ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe       \
                             --infile ../samples/git-sample/data%T.h5               \
                             --outfile ../samples/git-sample/data%T.bp &&           \
                                                                                    \
                         ${Python_EXECUTABLE}                                       \
-                            ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe         \
+                            ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe       \
                             --infile ../samples/git-sample/thetaMode/data%T.h5     \
                             --outfile ../samples/git-sample/thetaMode/data%T.bp && \
                                                                                    \
                         ${Python_EXECUTABLE}                                       \
-                            ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe         \
+                            ${openPMD_RUNTIME_OUTPUT_DIRECTORY}/openpmd-pipe       \
                             --infile ../samples/git-sample/thetaMode/data%T.bp     \
                             --outfile ../samples/git-sample/thetaMode/data%T.json  \
                         "

docs/source/backends/adios2.rst

@@ -56,7 +56,6 @@ In order to activate steps, it is imperative to use the :ref:`Streaming API <usa
 
 ADIOS2 release 2.6.0 contained a bug (fixed in ADIOS 2.7.0, see `PR #2348 <https://github.com/ornladios/ADIOS2/pull/2348>`_) that disallows random-accessing steps in file-based engines.
 With this ADIOS2 release, files written with steps may only be read using the streaming API.
-In order to keep compatibility with older codes reading ADIOS2 files, step-based processing must currently be opted in to via use of the :ref:`JSON parameter<backendconfig>` ``adios2.engine.usesteps = true`` when using a file-based engine such as BP3 or BP4 (usesteps).
 
 Upon reading a file, the ADIOS2 backend will automatically recognize whether it has been written with or without steps, ignoring the JSON option mentioned above.
 Steps are mandatory for streaming-based engines and trying to switch them off will result in a runtime error.
@@ -183,7 +182,6 @@ This feature can be activated via the JSON/TOML key ``adios2.use_group_table = t
 It is fully backward-compatible with the old layout of openPMD in ADIOS2 and mostly forward-compatible (except the support for steps).
 
 The variable-based encoding of openPMD automatically activates the group table feature.
-The group table feature automatically activates the use of ADIOS2 steps (which until version 0.15 was an opt-in feature via ``adios2.engine.usesteps = true``).
 
 Memory usage
 ------------

docs/source/details/backendconfig.rst

@@ -121,7 +121,6 @@ Explanation of the single keys:
 * ``adios2.engine.parameters``: An associative array of string-formatted engine parameters, passed directly through to ``adios2::IO::SetParameters``.
   Please refer to the `official ADIOS2 documentation <https://adios2.readthedocs.io/en/latest/engines/engines.html>`_ for the available engine parameters.
   The openPMD-api does not interpret these values and instead simply forwards them to ADIOS2.
-* ``adios2.engine.usesteps``: Described more closely in the documentation for the :ref:`ADIOS2 backend<backends-adios2>` (usesteps).
 * ``adios2.engine.preferred_flush_target`` Only relevant for BP5 engine, possible values are ``"disk"`` and ``"buffer"`` (default: ``"disk"``).
 
   * If ``"disk"``, data will be moved to disk on every flush.

include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp

@@ -225,12 +225,6 @@ class ADIOS2IOHandlerImpl
 #if openPMD_HAVE_MPI
     std::optional<MPI_Comm> m_communicator;
 #endif
-    /*
-     * If the iteration encoding is variableBased, we default to using a group
-     * table, since it is the only reliable way to recover currently active
-     * groups.
-     */
-    IterationEncoding m_iterationEncoding = IterationEncoding::groupBased;
     /**
      * The ADIOS2 engine type, to be passed to adios2::IO::SetEngine
      */
@@ -439,6 +433,8 @@ namespace ADIOS2Defaults
         "__openPMD_internal/openPMD2_adios2_schema";
     constexpr const_str str_isBoolean = "__is_boolean__";
     constexpr const_str str_activeTablePrefix = "__openPMD_groups";
+    constexpr const_str str_groupBasedWarning =
+        "__openPMD_internal/warning_bugprone_groupbased_encoding";
 } // namespace ADIOS2Defaults
 
 namespace detail
@@ -914,7 +910,6 @@ namespace detail
         UseGroupTable detectGroupTable();
 
         adios2::Engine &getEngine();
-        adios2::Engine &requireActiveStep();
 
         template <typename BA>
         void enqueue(BA &&ba);
@@ -976,15 +971,9 @@ namespace detail
          * @brief Begin or end an ADIOS step.
          *
          * @param mode Whether to begin or end a step.
-         * @param calledExplicitly True if called due to a public API call.
-         *     False if called from requireActiveStep.
-         *     Some engines (BP5) require that every interaction happens within
-         *     an active step, meaning that we need to call advance()
-         *     implicitly at times. When doing that, do not tag the dataset
-         *     with __openPMD_internal/useSteps (yet).
          * @return AdvanceStatus
          */
-        AdvanceStatus advance(AdvanceMode mode, bool calledExplicitly);
+        AdvanceStatus advance(AdvanceMode mode);
 
         /*
          * Delete all buffered actions without running them.
@@ -1069,34 +1058,7 @@ namespace detail
              *    without steps. This is not a workaround since not using steps,
              *    while inefficient in ADIOS2, is something that we support.
              */
-            NoStream,
-            /**
-             * Rationale behind this state:
-             * When user code opens a Series, series.iterations should contain
-             * all available iterations.
-             * If accessing a file without opening a step, ADIOS2 will grant
-             * access to variables and attributes from all steps, allowing us
-             * to parse the complete dump.
-             * This state indicates that no step should be opened for parsing
-             * purposes (which is necessary in streaming engines, hence they
-             * are initialized with the OutsideOfStep state).
-             * A step should only be opened if an explicit ADVANCE task arrives
-             * at the backend.
-             *
-             * @todo If the streaming API is used on files, parsing the whole
-             *       Series up front is unnecessary work.
-             *       Our frontend does not yet allow to distinguish whether
-             *       parsing the whole series will be necessary since parsing
-             *       happens upon construction time of Series,
-             *       but the classical and the streaming API are both activated
-             *       afterwards from the created Series object.
-             *       Hence, improving this requires refactoring in our
-             *       user-facing API. Ideas:
-             *       (1) Delayed lazy parsing of iterations upon accessing
-             *           (would bring other benefits also).
-             *       (2) Introduce a restricted class StreamingSeries.
-             */
-            Parsing,
+            ReadWithoutStream,
             /**
              * The stream status of a file-based engine will be decided upon
              * opening the engine if in read mode. Up until then, this right
@@ -1157,8 +1119,8 @@ namespace detail
         void create_IO();
 
         void configure_IO(ADIOS2IOHandlerImpl &impl);
-        void configure_IO_Read(std::optional<bool> userSpecifiedUsesteps);
-        void configure_IO_Write(std::optional<bool> userSpecifiedUsesteps);
+        void configure_IO_Read();
+        void configure_IO_Write();
     };
 
 } // namespace detail

include/openPMD/IO/AbstractIOHandler.hpp

@@ -23,6 +23,7 @@
 #include "openPMD/IO/Access.hpp"
 #include "openPMD/IO/Format.hpp"
 #include "openPMD/IO/IOTask.hpp"
+#include "openPMD/IterationEncoding.hpp"
 #include "openPMD/config.hpp"
 
 #if openPMD_HAVE_MPI
@@ -168,6 +169,11 @@ namespace internal
     }
 } // namespace internal
 
+namespace detail
+{
+    struct BufferedActions;
+}
+
 /** Interface for communicating between logical and physically persistent data.
  *
  * Input and output operations are channeled through a task queue that is
@@ -179,8 +185,12 @@ namespace internal
 class AbstractIOHandler
 {
     friend class Series;
+    friend class ADIOS2IOHandlerImpl;
+    friend struct detail::BufferedActions;
 
 private:
+    IterationEncoding m_encoding = IterationEncoding::groupBased;
+
     void setIterationEncoding(IterationEncoding encoding)
     {
         /*
@@ -193,6 +203,8 @@ class AbstractIOHandler
             // do we really want to have those as const members..?
             *const_cast<Access *>(&m_backendAccess) = Access::CREATE;
         }
+
+        m_encoding = encoding;
     }
 
 public:

include/openPMD/IO/IOTask.hpp

@@ -125,7 +125,6 @@ struct OPENPMDAPI_EXPORT Parameter<Operation::CREATE_FILE>
     }
 
     std::string name = "";
-    IterationEncoding encoding = IterationEncoding::groupBased;
 };
 
 template <>
@@ -172,12 +171,6 @@ struct OPENPMDAPI_EXPORT Parameter<Operation::OPEN_FILE>
     }
 
     std::string name = "";
-    /*
-     * The backends might need to ensure availability of certain features
-     * for some iteration encodings, e.g. availability of ADIOS steps for
-     * variableBased encoding.
-     */
-    IterationEncoding encoding = IterationEncoding::groupBased;
     using ParsePreference = internal::ParsePreference;
     std::shared_ptr<ParsePreference> out_parsePreference =
         std::make_shared<ParsePreference>(ParsePreference::UpFront);