Skip to content

Commit 9505017

Browse files
Minor fixes, mostly obstructive warnings (#1806)
* Do not flush a read series that did not do any IO yet * Do not read rankTable in file encoding * Remove <variant> include from Variant.hpp Does not make a big difference; other headers still include it and use variant for small helper types internally. * Avoid some wrong compiler warnings (gcc14) * Do not warn on parallel flush params in serial
1 parent b276c88 commit 9505017

File tree

9 files changed

+116
-43
lines changed

9 files changed

+116
-43
lines changed

examples/8b_benchmark_read_parallel.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,14 @@ class TestInput
456456
return;
457457
Timer blockTime(s.str(), m_MPIRank);
458458

459-
Offset off(meshExtent.size(), 0);
460-
Extent ext(meshExtent.size(), 1);
459+
// Avoid the below forms due to a compiler bug on gcc14
460+
// warning: 'void operator delete(void*, std::size_t)' called on pointer
461+
// '<unknown>' with nonzero offset [1, 9223372036854775800] Offset
462+
// off(meshExtent.size(), 0); Extent ext(meshExtent.size(), 1);
463+
Offset off(meshExtent.size());
464+
Extent ext(meshExtent.size());
465+
std::fill(off.begin(), off.end(), 0);
466+
std::fill(ext.begin(), ext.end(), 1);
461467

462468
for (unsigned int i = 0; i < meshExtent.size(); i++)
463469
{
@@ -486,7 +492,7 @@ class TestInput
486492
}
487493
}
488494

489-
auto prettyLambda = [&](Offset oo, Extent cc) {
495+
auto prettyLambda = [&](Offset const &oo, Extent const &cc) {
490496
std::ostringstream o;
491497
o << "[ ";
492498
std::ostringstream c;

include/openPMD/Series.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,18 @@ class Series : public Attributable
457457
*/
458458
Series &setMeshesPath(std::string const &meshesPath);
459459

460+
/**
461+
* @return True if there is a rankTable dataset defined for this Series.
462+
* False in write-only access modes.
463+
* Will indiscriminately return false in file encoding:
464+
* rankTable is not explicitly supported in file encoding,
465+
* finding out if one is defined would require opening all available
466+
* files. You may still call Series::rankTable() to retrieve the
467+
* rank table if you know that there is one, but note that it will
468+
* return the rank table from the file that was last opened.
469+
*/
470+
bool hasRankTableRead();
471+
460472
/**
461473
* @throw no_such_attribute_error If optional attribute is not present.
462474
* @param collective Run this read operation collectively.
@@ -1008,6 +1020,8 @@ OPENPMD_private
10081020
* steps?
10091021
*/
10101022
[[nodiscard]] bool randomAccessSteps() const;
1023+
1024+
std::vector<std::string> availableDatasets();
10111025
}; // Series
10121026

10131027
namespace debug

include/openPMD/auxiliary/Variant.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include <any>
2424
#include <cstddef>
2525
#include <type_traits>
26-
#include <variant> // IWYU pragma: export
2726

2827
namespace openPMD
2928
{

include/openPMD/snapshots/ContainerImpls.hpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,15 @@ class StatefulSnapshotsContainer : public AbstractSnapshotsContainer
3434
* make_reading_stateful_iterator.
3535
* The iterator is resolved upon calling get() below.
3636
*/
37-
std::variant<std::function<StatefulIterator *()>, StatefulIterator *>
38-
m_bufferedIterator;
37+
38+
// Need to put the deferred function behind a shared_ptr to avoid a
39+
// gcc14 compiler bug
40+
// warning: '*(std::_Function_base*)((char*)this +
41+
// 8).std::_Function_base::_M_manager' may be used uninitialized
42+
using Deferred_t = std::shared_ptr<std::function<StatefulIterator *()>>;
43+
using Evaluated_t = StatefulIterator *;
44+
using BufferedIterator_t = std::variant<Deferred_t, Evaluated_t>;
45+
BufferedIterator_t m_bufferedIterator = nullptr;
3946
};
4047
Members members;
4148

src/IO/HDF5/HDF5IOHandler.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3373,20 +3373,23 @@ std::future<void> HDF5IOHandlerImpl::flush(internal::ParsedFlushParams &params)
33733373
if (params.backendConfig.json().contains("hdf5"))
33743374
{
33753375
auto hdf5_config = params.backendConfig["hdf5"];
3376+
auto init_json_shadow = nlohmann::json::parse(flush_cfg_mask);
3377+
json::merge_internal(
3378+
hdf5_config.getShadow(), init_json_shadow, /* do_prune = */ false);
33763379

33773380
if (auto shadow = hdf5_config.invertShadow(); shadow.size() > 0)
33783381
{
33793382
switch (hdf5_config.originallySpecifiedAs)
33803383
{
33813384
case json::SupportedLanguages::JSON:
3382-
std::cerr << "Warning: parts of the backend configuration for "
3383-
"HDF5 remain unused:\n"
3385+
std::cerr << "Warning: parts of the backend flush "
3386+
"configuration for HDF5 remain unused:\n"
33843387
<< shadow << std::endl;
33853388
break;
33863389
case json::SupportedLanguages::TOML: {
33873390
auto asToml = json::jsonToToml(shadow);
3388-
std::cerr << "Warning: parts of the backend configuration for "
3389-
"HDF5 remain unused:\n"
3391+
std::cerr << "Warning: parts of the backend flush "
3392+
"configuration for HDF5 remain unused:\n"
33903393
<< json::format_toml(asToml) << std::endl;
33913394
break;
33923395
}

src/Series.cpp

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,38 @@ Series &Series::setMeshesPath(std::string const &mp)
215215
return *this;
216216
}
217217

218+
std::vector<std::string> Series::availableDatasets()
219+
{
220+
if (iterationEncoding() == IterationEncoding::variableBased &&
221+
IOHandler()->m_backendAccess == Access::READ_RANDOM_ACCESS)
222+
{
223+
Parameter<Operation::ADVANCE> advance;
224+
advance.mode =
225+
Parameter<Operation::ADVANCE>::StepSelection{std::nullopt};
226+
IOHandler()->enqueue(IOTask(this, std::move(advance)));
227+
}
228+
Parameter<Operation::LIST_DATASETS> listDatasets;
229+
IOHandler()->enqueue(IOTask(this, listDatasets));
230+
IOHandler()->flush(internal::defaultFlushParams);
231+
return std::move(*listDatasets.datasets);
232+
}
233+
234+
bool Series::hasRankTableRead()
235+
{
236+
if (access::writeOnly(IOHandler()->m_frontendAccess))
237+
{
238+
return false;
239+
}
240+
auto &series = get();
241+
if (series.m_rankTable.m_bufferedRead.has_value())
242+
{
243+
return true;
244+
}
245+
auto datasets = availableDatasets();
246+
return std::find(datasets.begin(), datasets.end(), "rankTable") !=
247+
datasets.end();
248+
}
249+
218250
#if openPMD_HAVE_MPI
219251
chunk_assignment::RankMeta Series::rankTable(bool collective)
220252
#else
@@ -245,21 +277,9 @@ chunk_assignment::RankMeta Series::rankTable([[maybe_unused]] bool collective)
245277
IOHandler()->enqueue(IOTask(this, openFile));
246278
#endif
247279
}
248-
if (iterationEncoding() == IterationEncoding::variableBased &&
249-
IOHandler()->m_backendAccess == Access::READ_RANDOM_ACCESS)
250-
{
251-
Parameter<Operation::ADVANCE> advance;
252-
advance.mode =
253-
Parameter<Operation::ADVANCE>::StepSelection{std::nullopt};
254-
IOHandler()->enqueue(IOTask(this, std::move(advance)));
255-
}
256-
Parameter<Operation::LIST_DATASETS> listDatasets;
257-
IOHandler()->enqueue(IOTask(this, listDatasets));
258-
IOHandler()->flush(internal::defaultFlushParams);
259-
if (std::none_of(
260-
listDatasets.datasets->begin(),
261-
listDatasets.datasets->end(),
262-
[](std::string const &str) { return str == "rankTable"; }))
280+
auto datasets = availableDatasets();
281+
if (std::find(datasets.begin(), datasets.end(), "rankTable") ==
282+
datasets.end())
263283
{
264284
rankTable.m_bufferedRead = chunk_assignment::RankMeta{};
265285
return {};
@@ -3119,18 +3139,25 @@ namespace internal
31193139
{
31203140
this->m_sharedStatefulIterator->close();
31213141
}
3122-
/*
3123-
* Scenario: A user calls `Series::flush()` but does not check for
3124-
* thrown exceptions. The exception will propagate further up,
3125-
* usually thereby popping the stack frame that holds the `Series`
3126-
* object. `Series::~Series()` will run. This check avoids that the
3127-
* `Series` is needlessly flushed a second time. Otherwise, error
3128-
* messages can get very confusing.
3129-
*/
31303142
Series impl;
31313143
impl.setData({this, [](auto const *) {}});
3132-
if (auto IOHandler = impl.IOHandler();
3133-
IOHandler && IOHandler->m_lastFlushSuccessful)
3144+
if (auto IOHandler = impl.IOHandler(); IOHandler &&
3145+
/*
3146+
* Scenario: A user calls `Series::flush()` but does not check for
3147+
* thrown exceptions. The exception will propagate further up,
3148+
* usually thereby popping the stack frame that holds the `Series`
3149+
* object. `Series::~Series()` will run. This check avoids that the
3150+
* `Series` is needlessly flushed a second time. Otherwise, error
3151+
* messages can get very confusing.
3152+
*/
3153+
3154+
IOHandler->m_lastFlushSuccessful &&
3155+
/*
3156+
* If a read-only Series is opened without any backend access, then
3157+
* don't go there now. Just peacefully close.
3158+
*/
3159+
!(access::readOnly(IOHandler->m_frontendAccess) &&
3160+
!(*this)->m_writable.written))
31343161
{
31353162
impl.flush();
31363163
/*

src/binding/python/Series.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ this method.
442442
.def_property("base_path", &Series::basePath, &Series::setBasePath)
443443
.def_property(
444444
"meshes_path", &Series::meshesPath, &Series::setMeshesPath)
445+
.def_property_readonly("has_rank_table_read", &Series::hasRankTableRead)
445446
.def("get_rank_table", &Series::rankTable, py::arg("collective"))
446447
.def("set_rank_table", &Series::setRankTable, py::arg("my_rank_info"))
447448
.def_property(

src/binding/python/openpmd_api/pipe/__main__.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,10 @@ def __copy(self, src, dest, current_path="/data/"):
401401
print("\t {0}".format(r))
402402
# With linear read mode, we can only load the source rank table
403403
# inside `read_iterations()` since it's a dataset.
404-
self.inranks = src.get_rank_table(collective=True)
404+
if src.has_rank_table_read:
405+
self.inranks = src.get_rank_table(collective=True)
406+
else:
407+
self.inranks = {}
405408
out_iteration = write_iterations[in_iteration.iteration_index]
406409
sys.stdout.flush()
407410
self.__copy(

src/snapshots/ContainerImpls.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,22 @@ namespace openPMD
1515
{
1616
StatefulSnapshotsContainer::StatefulSnapshotsContainer(
1717
std::variant<std::function<StatefulIterator *()>, StatefulIterator *> begin)
18-
: members{std::move(begin)}
18+
: members{
19+
// Need to put the deferred function behind a shared_ptr to avoid a
20+
// gcc14 compiler bug
21+
// warning: '*(std::_Function_base*)((char*)this +
22+
// 8).std::_Function_base::_M_manager' may be used uninitialized
23+
std::visit(
24+
auxiliary::overloaded{
25+
[](std::function<StatefulIterator *()> &&f)
26+
-> Members::BufferedIterator_t {
27+
return std::make_shared<
28+
Members::Deferred_t::element_type>(std::move(f));
29+
},
30+
[](StatefulIterator *it) -> Members::BufferedIterator_t {
31+
return it;
32+
}},
33+
std::move(begin))}
1934
{}
2035

2136
StatefulSnapshotsContainer::StatefulSnapshotsContainer(
@@ -34,21 +49,19 @@ auto StatefulSnapshotsContainer::get() -> StatefulIterator *
3449
{
3550
return std::visit(
3651
auxiliary::overloaded{
37-
[this](
38-
std::function<StatefulIterator *()> &deferred_initialization) {
39-
auto it = deferred_initialization();
52+
[this](Members::Deferred_t &deferred_initialization) {
53+
auto it = (*deferred_initialization)();
4054
this->members.m_bufferedIterator = it;
4155
return it;
4256
},
43-
[](StatefulIterator *it) { return it; }},
57+
[](Members::Evaluated_t it) { return it; }},
4458
members.m_bufferedIterator);
4559
}
4660
auto StatefulSnapshotsContainer::get() const -> StatefulIterator const *
4761
{
4862
return std::visit(
4963
auxiliary::overloaded{
50-
[](std::function<StatefulIterator *()> const &)
51-
-> StatefulIterator const * {
64+
[](Members::Deferred_t const &) -> StatefulIterator const * {
5265
throw std::runtime_error(
5366
"[StatefulSnapshotscontainer] Initialization has been "
5467
"deferred, but container is accessed as const, so cannot "

0 commit comments

Comments
 (0)