Skip to content

Commit

Permalink
Fix append mode double attributes (#1302)
Browse files Browse the repository at this point in the history
* Add failing test

* Allow creating file-based Series with Append mode

even if the specified directory does not exist

* Add Mpi.hpp with MPI helpers

Needed later for checking file presence in parallel situations

* Add CHECK_FILE IO task and implement in the backends

* Only write top-level attributes when really needed

* Enable Series creation in Append mode

* Add Windows CI workaround

* Windows Workaround: Fallback to Create mode

* Avoid MPI mocking, use ifdefs

Also use parallel logical or for file existence check

* Append mode: test in parallel

* Don't alias sendbfr and recvbfr

* Undo unused changes

* Remove unused Mock_MPI_Comm

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Replace MPI specializations by if constexpr

* Try making this constexpr

I think that this won't work with every MPI implementation

* Link ADIOS2 issue

ornladios/ADIOS2#3358

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 7, 2022
1 parent 273466e commit 230afdd
Show file tree
Hide file tree
Showing 20 changed files with 608 additions and 77 deletions.
10 changes: 9 additions & 1 deletion include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ class ADIOS2IOHandlerImpl
void
createFile(Writable *, Parameter<Operation::CREATE_FILE> const &) override;

void checkFile(Writable *, Parameter<Operation::CHECK_FILE> &) override;

// MPI Collective
bool checkFile(std::string fullFilePath) const;

void
createPath(Writable *, Parameter<Operation::CREATE_PATH> const &) override;

Expand Down Expand Up @@ -226,6 +231,9 @@ class ADIOS2IOHandlerImpl

private:
adios2::ADIOS m_ADIOS;
#if openPMD_HAVE_MPI
std::optional<MPI_Comm> m_communicator;
#endif
/*
* If the iteration encoding is variableBased, we default to using the
* 2021_02_09 schema since it allows mutable attributes.
Expand Down Expand Up @@ -329,7 +337,7 @@ class ADIOS2IOHandlerImpl
// use m_config
std::optional<std::vector<ParameterizedOperator> > getOperators();

std::string fileSuffix() const;
std::string fileSuffix(bool verbose = true) const;

/*
* We need to give names to IO objects. These names are irrelevant
Expand Down
1 change: 1 addition & 0 deletions include/openPMD/IO/ADIOS/CommonADIOS1IOHandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class CommonADIOS1IOHandlerImpl : public AbstractIOHandlerImpl
public:
void
createFile(Writable *, Parameter<Operation::CREATE_FILE> const &) override;
void checkFile(Writable *, Parameter<Operation::CHECK_FILE> &) override;
void
createPath(Writable *, Parameter<Operation::CREATE_PATH> const &) override;
void createDataset(
Expand Down
17 changes: 17 additions & 0 deletions include/openPMD/IO/AbstractIOHandlerImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ class AbstractIOHandlerImpl
deref_dynamic_cast<Parameter<Operation::CREATE_FILE> >(
i.parameter.get()));
break;
case O::CHECK_FILE:
checkFile(
i.writable,
deref_dynamic_cast<Parameter<Operation::CHECK_FILE> >(
i.parameter.get()));
break;
case O::CREATE_PATH:
createPath(
i.writable,
Expand Down Expand Up @@ -220,6 +226,17 @@ class AbstractIOHandlerImpl
virtual void
closeFile(Writable *, Parameter<Operation::CLOSE_FILE> const &) = 0;

/**
* Check if the file specified by the parameter is already present on disk.
* The Writable is irrelevant for this method.
* A backend can choose to ignore this task and specify FileExists::DontKnow
* in the out parameter.
* The consequence will be that some top-level attributes might be defined
* a second time when appending to an existing file, because the frontend
* cannot be sure that the file already has these attributes.
*/
virtual void checkFile(Writable *, Parameter<Operation::CHECK_FILE> &) = 0;

/** Advance the file/stream that this writable belongs to.
*
* If the backend is based around usage of IO steps (especially streaming
Expand Down
10 changes: 10 additions & 0 deletions include/openPMD/IO/HDF5/HDF5IOHandlerImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class HDF5IOHandlerImpl : public AbstractIOHandlerImpl

void
createFile(Writable *, Parameter<Operation::CREATE_FILE> const &) override;
void checkFile(Writable *, Parameter<Operation::CHECK_FILE> &) override;
void
createPath(Writable *, Parameter<Operation::CREATE_PATH> const &) override;
void createDataset(
Expand Down Expand Up @@ -92,6 +93,15 @@ class HDF5IOHandlerImpl : public AbstractIOHandlerImpl
hid_t m_H5T_CDOUBLE;
hid_t m_H5T_CLONG_DOUBLE;

protected:
#if openPMD_HAVE_MPI
/*
* Not defined in ParallelHDF5IOHandlerImpl, so we don't have to write
* some methods twice.
*/
std::optional<MPI_Comm> m_communicator;
#endif

private:
json::TracingJSON m_config;
std::string m_chunks = "auto";
Expand Down
29 changes: 28 additions & 1 deletion include/openPMD/IO/IOTask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ Writable *getWritable(Attributable *);
/** Type of IO operation between logical and persistent data.
*/
OPENPMDAPI_EXPORT_ENUM_CLASS(Operation){
CREATE_FILE, OPEN_FILE, CLOSE_FILE, DELETE_FILE,
CREATE_FILE, CHECK_FILE, OPEN_FILE, CLOSE_FILE,
DELETE_FILE,

CREATE_PATH, CLOSE_PATH, OPEN_PATH, DELETE_PATH,
LIST_PATHS,
Expand Down Expand Up @@ -118,6 +119,32 @@ struct OPENPMDAPI_EXPORT Parameter<Operation::CREATE_FILE>
IterationEncoding encoding = IterationEncoding::groupBased;
};

template <>
struct OPENPMDAPI_EXPORT Parameter<Operation::CHECK_FILE>
: public AbstractParameter
{
Parameter() = default;
Parameter(Parameter const &p)
: AbstractParameter(), name(p.name), fileExists(p.fileExists)
{}

std::unique_ptr<AbstractParameter> clone() const override
{
return std::unique_ptr<AbstractParameter>(
new Parameter<Operation::CHECK_FILE>(*this));
}

std::string name = "";
enum class FileExists
{
DontKnow,
Yes,
No
};
std::shared_ptr<FileExists> fileExists =
std::make_shared<FileExists>(FileExists::DontKnow);
};

template <>
struct OPENPMDAPI_EXPORT Parameter<Operation::OPEN_FILE>
: public AbstractParameter
Expand Down
2 changes: 2 additions & 0 deletions include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl
void
createFile(Writable *, Parameter<Operation::CREATE_FILE> const &) override;

void checkFile(Writable *, Parameter<Operation::CHECK_FILE> &) override;

void
createPath(Writable *, Parameter<Operation::CREATE_PATH> const &) override;

Expand Down
2 changes: 1 addition & 1 deletion include/openPMD/Series.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ OPENPMD_private
bool hasExpansionPattern(std::string filenameWithExtension);
bool reparseExpansionPattern(std::string filenameWithExtension);
void init(std::shared_ptr<AbstractIOHandler>, std::unique_ptr<ParsedInput>);
void initDefaults(IterationEncoding);
void initDefaults(IterationEncoding, bool initAll = false);
/**
* @brief Internal call for flushing a Series.
*
Expand Down
77 changes: 77 additions & 0 deletions include/openPMD/auxiliary/Mpi.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/* Copyright 2022 Franz Poeschel
*
* This file is part of openPMD-api.
*
* openPMD-api is free software: you can redistribute it and/or modify
* it under the terms of of either the GNU General Public License or
* the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* openPMD-api is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License and the GNU Lesser General Public License
* for more details.
*
* You should have received a copy of the GNU General Public License
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#pragma once

#include "openPMD/config.hpp"

#if openPMD_HAVE_MPI
#include <mpi.h>
#endif

#include <type_traits>

namespace openPMD::auxiliary
{
#if openPMD_HAVE_MPI

namespace detail
{
namespace
{
// see https://en.cppreference.com/w/cpp/language/if
template <typename>
inline constexpr bool dependent_false_v = false;
} // namespace
} // namespace detail

namespace
{
template <typename T>
constexpr MPI_Datatype openPMD_MPI_type()
{
using T_decay = std::decay_t<T>;
if constexpr (std::is_same_v<T_decay, char>)
{
return MPI_CHAR;
}
else if constexpr (std::is_same_v<T_decay, unsigned>)
{
return MPI_UNSIGNED;
}
else if constexpr (std::is_same_v<T_decay, unsigned long>)
{
return MPI_UNSIGNED_LONG;
}
else if constexpr (std::is_same_v<T_decay, unsigned long long>)
{
return MPI_UNSIGNED_LONG_LONG;
}
else
{
static_assert(
detail::dependent_false_v<T>,
"openPMD_MPI_type: Unsupported type.");
}
}
} // namespace

#endif
} // namespace openPMD::auxiliary
7 changes: 7 additions & 0 deletions src/IO/ADIOS/ADIOS1IOHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ std::future<void> ADIOS1IOHandlerImpl::flush()
deref_dynamic_cast<Parameter<Operation::CREATE_FILE> >(
i.parameter.get()));
break;
case O::CHECK_FILE:
checkFile(
i.writable,
deref_dynamic_cast<Parameter<Operation::CHECK_FILE> >(
i.parameter.get()));
break;
case O::CREATE_PATH:
createPath(
i.writable,
Expand Down Expand Up @@ -346,6 +352,7 @@ void ADIOS1IOHandler::enqueue(IOTask const &i)
switch (i.operation)
{
case Operation::CREATE_FILE:
case Operation::CHECK_FILE:
case Operation::CREATE_PATH:
case Operation::OPEN_PATH:
case Operation::CREATE_DATASET:
Expand Down
Loading

0 comments on commit 230afdd

Please sign in to comment.