From e668e8629c07b9d94c7bad573def3df12974ccbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 22 Dec 2023 15:59:59 +0100 Subject: [PATCH] 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 --- CMakeLists.txt | 14 +- docs/source/backends/adios2.rst | 2 - docs/source/details/backendconfig.rst | 1 - include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 50 +--- include/openPMD/IO/AbstractIOHandler.hpp | 12 + include/openPMD/IO/IOTask.hpp | 7 - src/IO/ADIOS/ADIOS2IOHandler.cpp | 294 ++++++++----------- src/IO/AbstractIOHandlerImpl.cpp | 18 +- src/ReadIterations.cpp | 1 - src/Series.cpp | 4 - test/ParallelIOTest.cpp | 24 +- test/SerialIOTest.cpp | 159 ++++------ 12 files changed, 237 insertions(+), 349 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 20673b1c50..1d81e94d83 100644 --- a/CMakeLists.txt +++ b/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 \ " diff --git a/docs/source/backends/adios2.rst b/docs/source/backends/adios2.rst index 13e357022c..a6161ed9dc 100644 --- a/docs/source/backends/adios2.rst +++ b/docs/source/backends/adios2.rst @@ -56,7 +56,6 @@ In order to activate steps, it is imperative to use the :ref:`Streaming API `_) 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` ``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 ------------ diff --git a/docs/source/details/backendconfig.rst b/docs/source/details/backendconfig.rst index 57d577af10..ae2a2d4f63 100644 --- a/docs/source/details/backendconfig.rst +++ b/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 `_ 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` (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. diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index cdd7983312..269d908360 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp @@ -225,12 +225,6 @@ class ADIOS2IOHandlerImpl #if openPMD_HAVE_MPI std::optional 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 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 userSpecifiedUsesteps); - void configure_IO_Write(std::optional userSpecifiedUsesteps); + void configure_IO_Read(); + void configure_IO_Write(); }; } // namespace detail diff --git a/include/openPMD/IO/AbstractIOHandler.hpp b/include/openPMD/IO/AbstractIOHandler.hpp index 1106f78f16..71a0587b0f 100644 --- a/include/openPMD/IO/AbstractIOHandler.hpp +++ b/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(&m_backendAccess) = Access::CREATE; } + + m_encoding = encoding; } public: diff --git a/include/openPMD/IO/IOTask.hpp b/include/openPMD/IO/IOTask.hpp index 7ca9e490a3..d2fc05f379 100644 --- a/include/openPMD/IO/IOTask.hpp +++ b/include/openPMD/IO/IOTask.hpp @@ -125,7 +125,6 @@ struct OPENPMDAPI_EXPORT Parameter } std::string name = ""; - IterationEncoding encoding = IterationEncoding::groupBased; }; template <> @@ -172,12 +171,6 @@ struct OPENPMDAPI_EXPORT Parameter } 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 out_parsePreference = std::make_shared(ParsePreference::UpFront); diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index 534e364f53..708fbbdef0 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -26,6 +26,7 @@ #include "openPMD/IO/ADIOS/ADIOS2Auxiliary.hpp" #include "openPMD/IO/ADIOS/ADIOS2FilePosition.hpp" #include "openPMD/IO/ADIOS/ADIOS2IOHandler.hpp" +#include "openPMD/IterationEncoding.hpp" #include "openPMD/auxiliary/Environment.hpp" #include "openPMD/auxiliary/Filesystem.hpp" #include "openPMD/auxiliary/Mpi.hpp" @@ -38,6 +39,7 @@ #include #include #include +#include #include #include @@ -543,6 +545,33 @@ ADIOS2IOHandlerImpl::flush(internal::ParsedFlushParams &flushParams) return res; } +/* + * 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. + * If group-based encoding is used without group table, then + * READ_LINEAR is forbidden as it will be unreliable in reporting + * currently available data. + * Use AbstractIOHandler::m_encoding for implementing this logic. + */ + +static constexpr char const *warningADIOS2NoGroupbasedEncoding = &R"( +[Warning] Use of group-based encoding in ADIOS2 is discouraged as it can lead +to drastic performance issues, no matter if I/O steps are used or not. + +* If not using I/O steps: A crash will corrupt all data since there is only + one atomic logical write operation upon closing the file. + Memory performance can be pathological depending on the setup. +* If using I/O steps: Each step will add new variables and attributes instead + of reusing those from earlier steps. ADIOS2 is not optimized for this and + especially the BP5 engine will show a quadratic increase in metadata size + as the number of steps increase. +We advise you to pick either file-based encoding or variable-based encoding +(variable-based encoding is not yet feature-complete in the openPMD-api). +For more details, refer to +https://openpmd-api.readthedocs.io/en/latest/usage/concepts.html#iteration-and-series)" + [1]; + void ADIOS2IOHandlerImpl::createFile( Writable *writable, Parameter const ¶meters) { @@ -578,7 +607,6 @@ void ADIOS2IOHandlerImpl::createFile( VERIFY(success, "[ADIOS2] Could not create directory."); } - m_iterationEncoding = parameters.encoding; associateWithFile(writable, shared_name); this->m_dirty.emplace(shared_name); @@ -586,7 +614,26 @@ void ADIOS2IOHandlerImpl::createFile( writable->abstractFilePosition = std::make_shared(); // enforce opening the file // lazy opening is deathly in parallel situations - getFileData(shared_name, IfFileNotOpen::OpenImplicitly); + auto &fileData = + getFileData(shared_name, IfFileNotOpen::OpenImplicitly); + + if (!printedWarningsAlready.noGroupBased && + m_writeAttributesFromThisRank && + m_handler->m_encoding == IterationEncoding::groupBased) + { + // For a peaceful phase-out of group-based encoding in ADIOS2, + // print this warning only in the new layout (with group table) + if (m_useGroupTable.value_or(UseGroupTable::No) == + UseGroupTable::Yes) + { + std::cerr << warningADIOS2NoGroupbasedEncoding << std::endl; + printedWarningsAlready.noGroupBased = true; + } + fileData.m_IO.DefineAttribute( + ADIOS2Defaults::str_groupBasedWarning, + std::string("Consider using file-based or variable-based " + "encoding instead in ADIOS2.")); + } } } @@ -832,7 +879,6 @@ void ADIOS2IOHandlerImpl::openFile( writable->written = true; writable->abstractFilePosition = std::make_shared(); - m_iterationEncoding = parameters.encoding; // enforce opening the file // lazy opening is deathly in parallel situations auto &fileData = getFileData(file, IfFileNotOpen::OpenImplicitly); @@ -1109,8 +1155,6 @@ void ADIOS2IOHandlerImpl::getBufferView( break; } - ba.requireActiveStep(); - if (parameters.update) { detail::I_UpdateSpan &updater = @@ -1145,7 +1189,6 @@ void ADIOS2IOHandlerImpl::readAttribute( auto file = refreshFileFromParent(writable, /* preferParentFile = */ false); auto pos = setAndGetFilePosition(writable); detail::BufferedActions &ba = getFileData(file, IfFileNotOpen::ThrowError); - ba.requireActiveStep(); auto name = nameOfAttribute(writable, parameters.name); auto type = detail::attributeInfo(ba.m_IO, name, /* verbose = */ true); @@ -1183,7 +1226,6 @@ void ADIOS2IOHandlerImpl::listPaths( * from variables and attributes. */ auto &fileData = getFileData(file, IfFileNotOpen::ThrowError); - fileData.requireActiveStep(); std::unordered_set subdirs; /* @@ -1315,7 +1357,6 @@ void ADIOS2IOHandlerImpl::listDatasets( */ auto &fileData = getFileData(file, IfFileNotOpen::ThrowError); - fileData.requireActiveStep(); std::unordered_set subdirs; for (auto var : fileData.availableVariablesPrefixed(myName)) @@ -1351,7 +1392,6 @@ void ADIOS2IOHandlerImpl::listAttributes( attributePrefix = ""; } auto &ba = getFileData(file, IfFileNotOpen::ThrowError); - ba.requireActiveStep(); // make sure that the attributes are present std::vector attrs = ba.availableAttributesPrefixed(attributePrefix); @@ -1371,8 +1411,7 @@ void ADIOS2IOHandlerImpl::advance( { auto file = m_files.at(writable); auto &ba = getFileData(file, IfFileNotOpen::ThrowError); - *parameters.status = - ba.advance(parameters.mode, /* calledExplicitly = */ true); + *parameters.status = ba.advance(parameters.mode); } void ADIOS2IOHandlerImpl::closePath( @@ -1416,8 +1455,8 @@ void ADIOS2IOHandlerImpl::availableChunks( std::string varName = nameOfVariable(writable); auto engine = ba.getEngine(); // make sure that data are present auto datatype = detail::fromADIOS2Type(ba.m_IO.VariableType(varName)); - bool allSteps = m_handler->m_frontendAccess != Access::READ_LINEAR && - ba.streamStatus == detail::BufferedActions::StreamStatus::NoStream; + bool allSteps = ba.streamStatus == + detail::BufferedActions::StreamStatus::ReadWithoutStream; switchAdios2VariableType( datatype, parameters, @@ -1786,7 +1825,6 @@ namespace detail auto &filedata = impl->getFileData( file, ADIOS2IOHandlerImpl::IfFileNotOpen::ThrowError); - filedata.requireActiveStep(); filedata.invalidateAttributesMap(); adios2::IO IO = filedata.m_IO; impl->m_dirty.emplace(std::move(file)); @@ -1946,7 +1984,6 @@ namespace detail { auto &fileData = impl->getFileData( file, ADIOS2IOHandlerImpl::IfFileNotOpen::ThrowError); - fileData.requireActiveStep(); auto &IO = fileData.m_IO; adios2::Variable var = IO.InquireVariable(varName); if (!var) @@ -2230,9 +2267,7 @@ namespace detail // might have been closed previously if (engine) { - if (streamStatus == StreamStatus::DuringStep || - (streamStatus == StreamStatus::NoStream && - m_mode == adios2::Mode::Write)) + if (streamStatus == StreamStatus::DuringStep) { engine.EndStep(); } @@ -2329,36 +2364,6 @@ namespace detail } return false; } - - bool useStepsInWriting( - UseGroupTable groupTable, std::string const &engineType) - { - if (engineType == "bp5") - { - /* - * BP5 does not require steps when reading, but it requires - * them when writing. - */ - return true; - } - switch (supportsPerstepParsing(Access::CREATE, engineType)) - { - case PerstepParsing::Required: - return true; - case PerstepParsing::Supported: - switch (groupTable) - { - case UseGroupTable::No: - return false; - case UseGroupTable::Yes: - return true; - } - break; - case PerstepParsing::Unsupported: - return false; - } - return false; // unreachable - } } // namespace size_t BufferedActions::currentStep() @@ -2373,18 +2378,8 @@ namespace detail } } - void BufferedActions::configure_IO_Read( - std::optional userSpecifiedUsesteps) + void BufferedActions::configure_IO_Read() { - if (userSpecifiedUsesteps.has_value() && - m_impl->m_handler->m_backendAccess != Access::READ_WRITE) - { - std::cerr << "Explicitly specified `adios2.usesteps` in Read mode. " - "Usage of steps will be determined by what is found " - "in the file being read." - << std::endl; - } - bool upfrontParsing = supportsUpfrontParsing( m_impl->m_handler->m_backendAccess, m_engineType); PerstepParsing perstepParsing = supportsPerstepParsing( @@ -2411,18 +2406,9 @@ namespace detail m_IO.SetParameter("StreamReader", "On"); break; case PerstepParsing::Unsupported: - streamStatus = StreamStatus::NoStream; - parsePreference = ParsePreference::UpFront; - /* - * Note that in BP4 with linear access mode, we set the - * StreamReader option, disabling upfrontParsing capability. - * So, this branch is only taken by niche engines, such as - * BP3 or HDF5, or by BP5 without group table and normal read - * mode. Need to fall back to random access parsing. - */ -#if openPMD_HAS_ADIOS_2_8 - m_mode = adios2::Mode::ReadRandomAccess; -#endif + throw error::Internal( + "Internal control flow error: Per-Step parsing cannot be " + "unsupported when access type is READ_LINEAR"); break; } break; @@ -2441,7 +2427,7 @@ namespace detail } if (upfrontParsing) { - streamStatus = StreamStatus::NoStream; + streamStatus = StreamStatus::ReadWithoutStream; parsePreference = ParsePreference::UpFront; } else @@ -2464,8 +2450,7 @@ namespace detail } } - void BufferedActions::configure_IO_Write( - std::optional userSpecifiedUsesteps) + void BufferedActions::configure_IO_Write() { optimizeAttributesStreaming = // Also, it should only be done when truly streaming, not @@ -2473,20 +2458,7 @@ namespace detail // streaming engine (otherwise attributes might vanish) nonpersistentEngine(m_engineType); - bool useSteps = useStepsInWriting(useGroupTable(), m_engineType); - if (userSpecifiedUsesteps.has_value()) - { - useSteps = userSpecifiedUsesteps.value(); - if (!useSteps && nonpersistentEngine(m_engineType)) - { - throw error::WrongAPIUsage( - "Cannot switch off IO steps for non-persistent stream " - "engines in ADIOS2."); - } - } - - streamStatus = - useSteps ? StreamStatus::OutsideOfStep : StreamStatus::NoStream; + streamStatus = StreamStatus::OutsideOfStep; } void BufferedActions::configure_IO(ADIOS2IOHandlerImpl &impl) @@ -2501,8 +2473,12 @@ namespace detail #if openPMD_HAS_ADIOS_2_9 if (!m_impl->m_useGroupTable.has_value()) { - switch (m_impl->m_iterationEncoding) + switch (m_impl->m_handler->m_encoding) { + /* + * For variable-based encoding, this does not matter as it is + * new and requires >= v2.9 features anyway. + */ case IterationEncoding::variableBased: m_impl->m_useGroupTable = UseGroupTable::Yes; break; @@ -2516,7 +2492,8 @@ namespace detail if (m_impl->m_modifiableAttributes == ADIOS2IOHandlerImpl::ModifiableAttributes::Unspecified) { - m_impl->m_modifiableAttributes = m_impl->m_iterationEncoding == + m_impl->m_modifiableAttributes = + m_impl->m_handler->m_encoding == IterationEncoding::variableBased ? ADIOS2IOHandlerImpl::ModifiableAttributes::Yes : ADIOS2IOHandlerImpl::ModifiableAttributes::No; @@ -2567,7 +2544,6 @@ namespace detail // set engine parameters std::set alreadyConfigured; - std::optional userSpecifiedUsesteps; bool wasTheFlushTargetSpecifiedViaJSON = false; auto engineConfig = impl.config(ADIOS2Defaults::str_engine); if (!engineConfig.json().is_null()) @@ -2599,8 +2575,10 @@ namespace detail impl.config(ADIOS2Defaults::str_usesteps, engineConfig); if (!_useAdiosSteps.json().is_null() && writeOnly(m_mode)) { - userSpecifiedUsesteps = - std::make_optional(_useAdiosSteps.json().get()); + std::cerr << "[ADIOS2 backend] WARNING: Parameter " + "`adios2.engine.usesteps` is deprecated since use " + "of steps is now always enabled." + << std::endl; } if (engineConfig.json().contains(ADIOS2Defaults::str_flushtarget)) @@ -2643,21 +2621,21 @@ namespace detail { case Access::READ_LINEAR: case Access::READ_ONLY: - configure_IO_Read(userSpecifiedUsesteps); + configure_IO_Read(); break; case Access::READ_WRITE: if (readOnly(m_mode)) { - configure_IO_Read(userSpecifiedUsesteps); + configure_IO_Read(); } else { - configure_IO_Write(userSpecifiedUsesteps); + configure_IO_Write(); } break; case Access::APPEND: case Access::CREATE: - configure_IO_Write(userSpecifiedUsesteps); + configure_IO_Write(); break; } @@ -2828,11 +2806,8 @@ namespace detail // the streaming API was used. m_engine = std::make_optional( adios2::Engine(m_IO.Open(m_file, tempMode))); - if (streamStatus == StreamStatus::NoStream) - { - // Write everything into one big step - m_engine->BeginStep(); - } + m_engine->BeginStep(); + streamStatus = StreamStatus::DuringStep; break; } #if openPMD_HAS_ADIOS_2_8 @@ -2911,10 +2886,45 @@ namespace detail "anything in an engine that supports " "up-front parsing."); } - streamStatus = StreamStatus::Parsing; + streamStatus = StreamStatus::ReadWithoutStream; } else { + // If the iteration encoding is group-based and + // no group table is used, we're now at a dead-end. + // Step-by-Step parsing is unreliable in that mode + // since groups might be reported that are not + // there. + // But we were only able to find this out by opening + // the ADIOS2 file with an access mode that was + // possibly wrong, so we would have to close and + // reopen here. + // Since group-based encoding is a bag of trouble in + // ADIOS2 anyway, we just don't support this + // particular use case. + // This failure will only arise when the following + // conditions are met: + // + // 1) group-based encoding + // 2) no group table (i.e. old "ADIOS2 schema") + // 3) LINEAR access mode + // + // This is a relatively lenient restriction compared + // to forbidding group-based encoding in ADIOS2 + // altogether. + if (m_impl->m_useGroupTable.value() == + UseGroupTable::No && + m_IO.InquireAttribute( + ADIOS2Defaults::str_groupBasedWarning)) + { + throw error::OperationUnsupportedInBackend( + "ADIOS2", + "Trying to open a group-based ADIOS2 file " + "that does not have a group table with " + "LINEAR access type. That combination is " + "very buggy, so please use " + "READ_ONLY/READ_RANDOM_ACCESS instead."); + } if (!openedANewStep && m_engine.value().BeginStep() != adios2::StepStatus::OK) @@ -2932,11 +2942,11 @@ namespace detail * If openedANewStep is true, then the file consists * of one large step, we just leave it open. */ - streamStatus = StreamStatus::NoStream; + streamStatus = StreamStatus::ReadWithoutStream; } break; } - case StreamStatus::NoStream: + case StreamStatus::ReadWithoutStream: // using random-access mode break; case StreamStatus::DuringStep: @@ -2972,31 +2982,6 @@ namespace detail return m_engine.value(); } - adios2::Engine &BufferedActions::requireActiveStep() - { - adios2::Engine &eng = getEngine(); - /* - * If streamStatus is Parsing, do NOT open the step. - */ - if (streamStatus == StreamStatus::OutsideOfStep) - { - switch ( - advance(AdvanceMode::BEGINSTEP, /* calledExplicitly = */ false)) - { - case AdvanceStatus::OVER: - throw std::runtime_error( - "[ADIOS2] Operation requires active step but no step is " - "left."); - case AdvanceStatus::OK: - case AdvanceStatus::RANDOMACCESS: - // pass - break; - } - streamStatus = StreamStatus::DuringStep; - } - return eng; - } - template void BufferedActions::enqueue(BA &&ba) { @@ -3068,10 +3053,6 @@ namespace detail } return; } - else - { - requireActiveStep(); - } } for (auto &ba : m_buffer) { @@ -3207,48 +3188,23 @@ namespace detail /* flushUnconditionally = */ false); } - AdvanceStatus - BufferedActions::advance(AdvanceMode mode, bool calledExplicitly) + AdvanceStatus BufferedActions::advance(AdvanceMode mode) { if (streamStatus == StreamStatus::Undecided) { - // stream status gets decided on upon opening an engine - getEngine(); + throw error::Internal( + "[BufferedActions::advance()] StreamStatus Undecided before " + "beginning/ending a step?"); } // sic! no else - if (streamStatus == StreamStatus::NoStream) + if (streamStatus == StreamStatus::ReadWithoutStream) { - if (writeOnly(m_mode) && - !m_IO.InquireAttribute( - ADIOS2Defaults::str_usesstepsAttribute) && - m_impl->m_writeAttributesFromThisRank) - { - m_IO.DefineAttribute( - ADIOS2Defaults::str_usesstepsAttribute, 0); - } flush( ADIOS2FlushParams{FlushLevel::UserFlush}, /* writeLatePuts = */ false); return AdvanceStatus::RANDOMACCESS; } - /* - * If advance() is called implicitly (by requireActiveStep()), the - * Series is not necessarily using steps (logically). - * But in some ADIOS2 engines, at least one step must be opened - * (physically) to do anything. - * The usessteps tag should only be set when the Series is *logically* - * using steps. - */ - if (calledExplicitly && writeOnly(m_mode) && - !m_IO.InquireAttribute( - ADIOS2Defaults::str_usesstepsAttribute) && - m_impl->m_writeAttributesFromThisRank) - { - m_IO.DefineAttribute( - ADIOS2Defaults::str_usesstepsAttribute, 1); - } - switch (mode) { case AdvanceMode::ENDSTEP: { @@ -3270,6 +3226,15 @@ namespace detail "opened."); } } + + if (writeOnly(m_mode) && m_impl->m_writeAttributesFromThisRank && + !m_IO.InquireAttribute( + ADIOS2Defaults::str_usesstepsAttribute)) + { + m_IO.DefineAttribute( + ADIOS2Defaults::str_usesstepsAttribute, 1); + } + flush( ADIOS2FlushParams{FlushLevel::UserFlush}, [](BufferedActions &, adios2::Engine &eng) { eng.EndStep(); }, @@ -3412,7 +3377,6 @@ namespace detail { if (writeOnly(m_mode) && m_impl->m_writeAttributesFromThisRank) { - requireActiveStep(); auto currentStepBuffered = currentStep(); do { diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index 9a12a45b35..bbab360b4d 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/src/IO/AbstractIOHandlerImpl.cpp @@ -25,6 +25,7 @@ #include "openPMD/backend/Writable.hpp" #include +#include namespace openPMD { @@ -317,7 +318,22 @@ std::future AbstractIOHandlerImpl::flush() auto ¶meter = deref_dynamic_cast>( i.parameter.get()); writeToStderr( - "[", i.writable->parent, "->", i.writable, "] ADVANCE"); + "[", + i.writable->parent, + "->", + i.writable, + "] ADVANCE ", + [&]() { + switch (parameter.mode) + { + + case AdvanceMode::BEGINSTEP: + return "BEGINSTEP"; + case AdvanceMode::ENDSTEP: + return "ENDSTEP"; + } + throw std::runtime_error("Unreachable!"); + }()); advance(i.writable, parameter); break; } diff --git a/src/ReadIterations.cpp b/src/ReadIterations.cpp index 74e963eb55..fff398c8cd 100644 --- a/src/ReadIterations.cpp +++ b/src/ReadIterations.cpp @@ -74,7 +74,6 @@ void SeriesIterator::initSeriesInLinearReadMode() case IE::variableBased: { Parameter fOpen; fOpen.name = series.get().m_name; - fOpen.encoding = series.iterationEncoding(); series.IOHandler()->enqueue(IOTask(&series, fOpen)); series.IOHandler()->flush(internal::defaultFlushParams); using PP = Parameter::ParsePreference; diff --git a/src/Series.cpp b/src/Series.cpp index e89841db7d..5d698ffbcf 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -924,7 +924,6 @@ void Series::flushGorVBased( } Parameter fCreate; fCreate.name = series.m_name; - fCreate.encoding = iterationEncoding(); IOHandler()->enqueue(IOTask(this, fCreate)); } @@ -1004,7 +1003,6 @@ void Series::readFileBased() auto &series = get(); Parameter fOpen; Parameter aRead; - fOpen.encoding = iterationEncoding(); if (!auxiliary::directory_exists(IOHandler()->directory)) throw error::ReadError( @@ -1320,7 +1318,6 @@ auto Series::readGorVBased( auto &series = get(); Parameter fOpen; fOpen.name = series.m_name; - fOpen.encoding = iterationEncoding(); IOHandler()->enqueue(IOTask(this, fOpen)); IOHandler()->flush(internal::defaultFlushParams); series.m_parsePreference = *fOpen.out_parsePreference; @@ -2107,7 +2104,6 @@ void Series::openIteration(IterationIndex_t index, Iteration iteration) auto &series = get(); // open the iteration's file again Parameter fOpen; - fOpen.encoding = iterationEncoding(); fOpen.name = iterationFilename(index); IOHandler()->enqueue(IOTask(this, fOpen)); diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index ce7d6cc565..a82a300b0e 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -1403,7 +1403,8 @@ void append_mode( std::string const &extension, bool variableBased, ParseMode parseMode, - std::string const &jsonConfig = "{}") + std::string const &jsonConfig = "{}", + bool test_read_linear = true) { std::string filename = (variableBased ? "../samples/append/append_variablebased." @@ -1500,6 +1501,7 @@ void append_mode( } }; + if (test_read_linear) { switch (parseMode) { @@ -1624,6 +1626,8 @@ void append_mode( write.flush(); } MPI_Barrier(MPI_COMM_WORLD); + + if (test_read_linear) { Series read(filename, Access::READ_LINEAR, MPI_COMM_WORLD); switch (parseMode) @@ -1689,22 +1693,14 @@ TEST_CASE("append_mode", "[serial]") { "adios2": { - "use_group_table": false, - "engine": - { - "usesteps" : true - } + "use_group_table": false } })END"; std::string jsonConfigNew = R"END( { "adios2": { - "use_group_table": true, - "engine": - { - "usesteps" : true - } + "use_group_table": true } })END"; if (t == "bp" || t == "bp4" || t == "bp5") @@ -1718,7 +1714,11 @@ TEST_CASE("append_mode", "[serial]") */ #if HAS_ADIOS_2_8 append_mode( - t, false, ParseMode::LinearWithoutSnapshot, jsonConfigOld); + t, + false, + ParseMode::LinearWithoutSnapshot, + jsonConfigOld, + /* test_read_linear = */ false); #endif #if HAS_ADIOS_2_9 append_mode(t, false, ParseMode::WithSnapshot, jsonConfigNew); diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 19cad91155..7f126e104f 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -4260,8 +4260,6 @@ TEST_CASE("adios2_bp5_flush", "[serial][adios2]") [adios2] [adios2.engine] -# Check that BP5 can also be used without steps -usesteps = false type = "bp5" preferred_flush_target = "disk" @@ -4280,7 +4278,6 @@ BufferChunkSize = 2147483646 # 2^31 - 2 [adios2] [adios2.engine] -usesteps = true type = "bp5" preferred_flush_target = "buffer" @@ -4298,7 +4295,6 @@ BufferChunkSize = 2147483646 # 2^31 - 2 [adios2] [adios2.engine] -usesteps = true type = "bp5" # preferred_flush_target = @@ -4321,7 +4317,6 @@ BufferChunkSize = 2147483646 # 2^31 - 2 [adios2] [adios2.engine] -usesteps = true type = "bp5" preferred_flush_target = "buffer_override" @@ -4339,7 +4334,6 @@ BufferChunkSize = 2147483646 # 2^31 - 2 [adios2] [adios2.engine] -usesteps = true type = "bp5" preferred_flush_target = "disk_override" @@ -4859,8 +4853,7 @@ this = "should not warn" void bp4_steps( std::string const &file, std::string const &options_write, - std::string const &options_read, - Access access = Access::READ_ONLY) + std::optional access = Access::READ_ONLY) { { Series writeSeries(file, Access::CREATE, options_write); @@ -4880,12 +4873,12 @@ void bp4_steps( } } - if (options_read.empty()) + if (!access.has_value()) { return; } - Series readSeries(file, access, options_read); + Series readSeries(file, *access); size_t last_iteration_index = 0; for (auto iteration : readSeries.readIterations()) @@ -4911,104 +4904,62 @@ void bp4_steps( TEST_CASE("bp4_steps", "[serial][adios2]") { - std::string useSteps = R"( + std::string bp4 = json::merge( + R"( { "ADIOS2": { "engine": { - "type": "bp4", - "usesteps": true + "type": "bp4" } } } - )"; + )", +#if openPMD_HAS_ADIOS_2_9 + R"({"ADIOS2":{"use_group_table": true}})" +#else + R"({"ADIOS2":{"use_group_table": false}})" +#endif + ); std::string nullcore = R"( { "adios2": { - "type": "nullcore", - "ENGINE": { - "type": "bp4", - "usesteps": true - } + "type": "nullcore" } } )"; - std::string dontUseSteps = R"( - # let's use TOML for this one - [adios2.engine] - type = "bp4" - UseSteps = false - )"; - // sing the yes no song - bp4_steps("../samples/bp4steps_yes_yes.bp", useSteps, useSteps); - bp4_steps("../samples/bp4steps_no_yes.bp", dontUseSteps, useSteps); - bp4_steps("../samples/bp4steps_yes_no.bp", useSteps, dontUseSteps); - bp4_steps("../samples/bp4steps_no_no.bp", dontUseSteps, dontUseSteps); - bp4_steps("../samples/nullcore.bp", nullcore, ""); - bp4_steps("../samples/bp4steps_default.bp", "{}", "{}"); - - // bp4_steps( - // "../samples/newlayout_bp4steps_yes_yes.bp", - // useSteps, - // useSteps, - // Access::READ_LINEAR); - // bp4_steps( - // "../samples/newlayout_bp4steps_yes_no.bp", - // useSteps, - // dontUseSteps, - // Access::READ_LINEAR); + bp4_steps("../samples/bp4steps.bp", bp4); + bp4_steps("../samples/nullcore.bp", nullcore, std::nullopt); + bp4_steps("../samples/bp4steps_default.bp", "{}"); + // Can use READ_LINEAR with ADIOS2 v2.9 because then we have the group table + // feature and can sensibly parse group-based encoding in step-based mode + bp4_steps( + "../samples/bp4steps.bp", + bp4, +#if openPMD_HAS_ADIOS_2_9 + Access::READ_LINEAR +#else + Access::READ_ONLY +#endif + ); #if openPMD_HAS_ADIOS_2_9 /* * Do this whole thing once more, but this time use the new attribute * layout. */ - useSteps = R"( - { - "adios2": { - "use_group_table": true, - "engine": { - "type": "bp4", - "usesteps": true - } - } - } - )"; - dontUseSteps = R"( + bp4 = R"( { "adios2": { "use_group_table": true, "engine": { - "type": "bp4", - "usesteps": false + "type": "bp4" } } } )"; - // sing the yes no song - bp4_steps( - "../samples/newlayout_bp4steps_yes_yes.bp", - useSteps, - useSteps, - Access::READ_LINEAR); - bp4_steps("../samples/newlayout_bp4steps_yes_yes.bp", useSteps, useSteps); - bp4_steps( - "../samples/newlayout_bp4steps_yes_no.bp", useSteps, dontUseSteps); - bp4_steps( - "../samples/newlayout_bp4steps_no_yes.bp", dontUseSteps, useSteps); - bp4_steps( - "../samples/newlayout_bp4steps_no_no.bp", dontUseSteps, dontUseSteps); - - bp4_steps( - "../samples/newlayout_bp4steps_yes_yes.bp", - useSteps, - useSteps, - Access::READ_LINEAR); - bp4_steps( - "../samples/newlayout_bp4steps_yes_no.bp", - useSteps, - dontUseSteps, - Access::READ_LINEAR); + bp4_steps("../samples/newlayout_bp4steps.bp", bp4, Access::READ_LINEAR); + bp4_steps("../samples/newlayout_bp4steps.bp", bp4); #endif } #endif @@ -6478,10 +6429,7 @@ void chaotic_stream(std::string filename, bool variableBased) std::string jsonConfig = R"( { "adios2": { - "use_group_table": true, - "engine": { - "usesteps": true - } + "use_group_table": true } })"; @@ -6552,7 +6500,8 @@ TEST_CASE("chaotic_stream", "[serial]") void unfinished_iteration_test( std::string const &ext, IterationEncoding encoding, - std::string const &config = "{}") + std::string const &config = "{}", + bool test_linear_access = true) { std::cout << "\n\nTESTING " << ext << "\n\n" << std::endl; std::string file = std::string("../samples/unfinished_iteration") + @@ -6641,8 +6590,11 @@ void unfinished_iteration_test( } }; - tryReading(Access::READ_LINEAR); - tryReading(Access::READ_LINEAR, R"({"defer_iteration_parsing": true})"); + if (test_linear_access) + { + tryReading(Access::READ_LINEAR); + tryReading(Access::READ_LINEAR, R"({"defer_iteration_parsing": true})"); + } if (encoding != IterationEncoding::variableBased) { /* @@ -6661,7 +6613,10 @@ TEST_CASE("unfinished_iteration_test", "[serial]") { #if openPMD_HAVE_ADIOS2 unfinished_iteration_test( - "bp", IterationEncoding::groupBased, R"({"backend": "adios2"})"); + "bp", + IterationEncoding::groupBased, + R"({"backend": "adios2"})", + /* test_linear_access = */ false); #if openPMD_HAS_ADIOS_2_9 unfinished_iteration_test( "bp5", @@ -6822,7 +6777,8 @@ void append_mode( std::string const &filename, bool variableBased, ParseMode parseMode, - std::string const &jsonConfig = "{}") + std::string const &jsonConfig = "{}", + bool test_read_linear = true) { if (auxiliary::directory_exists("../samples/append")) { @@ -6905,6 +6861,7 @@ void append_mode( } }; + if (test_read_linear) { switch (parseMode) { @@ -7027,6 +6984,7 @@ void append_mode( write.writeIterations(), std::vector{4, 5}); write.flush(); } + if (test_read_linear) { Series read(filename, Access::READ_LINEAR); switch (parseMode) @@ -7091,22 +7049,14 @@ TEST_CASE("append_mode", "[serial]") { "adios2": { - "use_group_table": false, - "engine": - { - "usesteps" : true - } + "use_group_table": false } })END"; std::string jsonConfigNew = R"END( { "adios2": { - "use_group_table": true, - "engine": - { - "usesteps" : true - } + "use_group_table": true } })END"; if (t == "bp" || t == "bp4" || t == "bp5") @@ -7115,7 +7065,8 @@ TEST_CASE("append_mode", "[serial]") "../samples/append/append_groupbased." + t, false, ParseMode::LinearWithoutSnapshot, - jsonConfigOld); + jsonConfigOld, + /* test_read_linear = */ false); #if openPMD_HAS_ADIOS_2_9 append_mode( "../samples/append/append_groupbased." + t, @@ -7152,11 +7103,7 @@ void append_mode_filebased(std::string const &extension) { "adios2": { - "use_group_table": true, - "engine": - { - "usesteps" : true - } + "use_group_table": true } })END"; auto writeSomeIterations = [](WriteIterations &&writeIterations,