Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constellation checkpointing #129

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cfg/default_cfg.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ sim:
minutes: 0. # as floating points
hours: 0. # as floating points
days: 0. # as floating points
referenceTime: 2022-01-01 # calendar day associated with simulation start at iteration 0 (yyyy/mm/dd)
referenceTime: 2022-01-01 # calendar day associated with simulation start at iteration 0 (yyyy-mm-dd)
maxAltitude: 10000 # Maximum satellite altitude above earth core. This number times two is the simulation box length. [km]
minAltitude: 150 # Everything below this altitude above ground will be considered burning up [km]
deltaT: 10.0 # [s]
Expand Down
22 changes: 15 additions & 7 deletions src/ladds/Simulation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ Simulation::initIntegrator(AutoPas_t &autopas, ConfigReader &config) {
}

std::tuple<size_t, std::shared_ptr<HDF5Writer>, std::shared_ptr<ConjuctionWriterInterface>> Simulation::initWriter(
ConfigReader &config) {
ConfigReader &config, std::set<size_t> &alreadyExistingIds) {
if (config.defines("io/hdf5")) {
std::shared_ptr<HDF5Writer> hdf5Writer;
const auto hdf5WriteFrequency = config.get<unsigned int>("io/hdf5/writeFrequency", 0u);
Expand All @@ -145,7 +145,7 @@ std::tuple<size_t, std::shared_ptr<HDF5Writer>, std::shared_ptr<ConjuctionWriter
// TODO: Remove DATADIR functionality
const auto checkpointPath = std::string(DATADIR) + checkpointFileName;
// compression level already set when file already exists
hdf5Writer = std::make_shared<HDF5Writer>(checkpointPath, false, 0);
hdf5Writer = std::make_shared<HDF5Writer>(checkpointPath, false, 0, alreadyExistingIds);
}
// ... or write to new HDF5 file ....
if (const auto hdf5FileName = config.get<std::string>("io/hdf5/fileName", ""); not hdf5FileName.empty()) {
Expand All @@ -155,7 +155,7 @@ std::tuple<size_t, std::shared_ptr<HDF5Writer>, std::shared_ptr<ConjuctionWriter
"HDF5Writer already defined!\nProbably both a checkpoint and a HDF5 output file are defined. Please choose "
"only one.");
}
hdf5Writer = std::make_shared<HDF5Writer>(hdf5FileName, true, hdf5CompressionLvl);
hdf5Writer = std::make_shared<HDF5Writer>(hdf5FileName, true, hdf5CompressionLvl, alreadyExistingIds);
}
// check that at least something set a filename
if (not hdf5Writer) {
Expand All @@ -172,15 +172,16 @@ std::tuple<size_t, std::shared_ptr<HDF5Writer>, std::shared_ptr<ConjuctionWriter
void Simulation::updateConstellation(AutoPas_t &autopas,
std::vector<Constellation> &constellations,
std::vector<Particle> &delayedInsertionTotal,
double constellationCutoff) {
double constellationCutoff,
size_t simulationTime) {
// first insert delayed particles from previous insertion and collect the repeatedly delayed
delayedInsertionTotal = checkedInsert(autopas, delayedInsertionTotal, constellationCutoff);
// container collecting delayed particles from one constellation at a time in order to append them to
// totalDelayedInsertion
std::vector<Particle> delayedInsertion;
for (auto &constellation : constellations) {
// new satellites are gradually added to the simulation according to their starting time and operation duration
auto newSatellites = constellation.tick();
auto newSatellites = constellation.tick(simulationTime);
delayedInsertion = checkedInsert(autopas, newSatellites, constellationCutoff);
delayedInsertionTotal.insert(delayedInsertionTotal.end(), delayedInsertion.begin(), delayedInsertion.end());
}
Expand Down Expand Up @@ -222,7 +223,14 @@ size_t Simulation::simulationLoop(AutoPas_t &autopas,

const auto vtkWriteFrequency = config.get<size_t>("io/vtk/writeFrequency", 0ul);

const auto [hdf5WriteFrequency, hdf5Writer, conjuctionWriter] = initWriter(config);
// collects particle ids of potential checkpoint. They need to be passed to the HDF5Writer to avoid redundant output.
std::set<size_t> alreadyExistingIds{};
if (config.defines("io/hdf5/checkpoint/file")) {
for (auto &particle : autopas) {
alreadyExistingIds.insert(particle.getID());
}
}
const auto [hdf5WriteFrequency, hdf5Writer, conjuctionWriter] = initWriter(config, alreadyExistingIds);

// set constellation particle IDs and fetch maxExistingParticleId
const size_t maxExistingParticleId = setConstellationIDs(autopas, constellations);
Expand Down Expand Up @@ -252,7 +260,7 @@ size_t Simulation::simulationLoop(AutoPas_t &autopas,
timers.constellationInsertion.start();
// new satellites from constellations inserted over time
if (iteration % constellationInsertionFrequency == 0) {
updateConstellation(autopas, constellations, delayedInsertion, constellationCutoff);
updateConstellation(autopas, constellations, delayedInsertion, constellationCutoff, iteration);
}
timers.constellationInsertion.stop();

Expand Down
7 changes: 5 additions & 2 deletions src/ladds/Simulation.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ class Simulation {
/**
* Depending on config initialize readers.
* @param config
* @param alreadyExistingIds a list of IDs of already existing particles. Empty if no checkpoint is loaded.
* @return tuple<hdf5WriteFrequency, hdf5Writer, conjuctionWriter>
*/
[[nodiscard]] std::tuple<size_t, std::shared_ptr<HDF5Writer>, std::shared_ptr<ConjuctionWriterInterface>> initWriter(
ConfigReader &config);
ConfigReader &config, std::set<size_t> &alreadyExistingIds);

/**
* Tick constellation state machines and if applicable insert new satellites as well as delayed ones from previous
Expand All @@ -77,11 +78,13 @@ class Simulation {
* parameter!
* @param constellationCutoff range parameter for checked insertion: if the insertion would be within a distance
* of constellationCutoff to any other object the insertion is delayed instead
* @param simulationTime the current iteration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't you rename the parameter to iteration then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to more stress that this is in the time unit of the simulation. But I think you are right iteration also makes it clear and is less confusing :D

*/
void updateConstellation(AutoPas_t &autopas,
std::vector<Constellation> &constellations,
std::vector<Particle> &delayedInsertion,
double constellationCutoff);
double constellationCutoff,
size_t simulationTime);

/**
* Check for collisions / conjunctions and write statistics about them.
Expand Down
23 changes: 15 additions & 8 deletions src/ladds/io/hdf5/HDF5Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@

#include "HDF5Definitions.h"

HDF5Writer::HDF5Writer(const std::string &filename, bool replace, unsigned int compressionLevel)
HDF5Writer::HDF5Writer(const std::string &filename,
bool replace,
unsigned int compressionLevel,
const std::set<size_t> &alreadyExistingIds)
#ifdef LADDS_HDF5
: _file(filename, replace ? h5pp::FilePermission::REPLACE : h5pp::FilePermission::READWRITE),
collisionInfoH5Type(H5Tcreate(H5T_COMPOUND, sizeof(HDF5Definitions::CollisionInfo))),
Expand All @@ -20,6 +23,13 @@ HDF5Writer::HDF5Writer(const std::string &filename, bool replace, unsigned int c
if (replace) {
_file.setCompressionLevel(compressionLevel);
}

// add ids of previous checkpoint to list of existing particle ids in order to avoid adding duplicate
// constantProperties
for (auto &id : alreadyExistingIds) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (auto &id : alreadyExistingIds) {
for (const auto &id : alreadyExistingIds) {

addedConstantPropertiesIds.insert(static_cast<HDF5Definitions::IntType>(id));
}

// CollisionInfo
H5Tinsert(collisionInfoH5Type,
"idA",
Expand Down Expand Up @@ -75,7 +85,6 @@ void HDF5Writer::writeParticles(size_t iteration, const AutoPas_t &autopas) {
std::vector<HDF5Definitions::Vec3<HDF5Definitions::FloatType>> vecPos;
std::vector<HDF5Definitions::Vec3<HDF5Definitions::FloatType>> vecVel;
std::vector<HDF5Definitions::IntType> vecId;
HDF5Definitions::IntType maxParticleId{0};
std::vector<HDF5Definitions::ParticleConstantProperties> newConstantProperties;

vecPos.reserve(autopas.getNumberOfParticles());
Expand All @@ -96,11 +105,10 @@ void HDF5Writer::writeParticles(size_t iteration, const AutoPas_t &autopas) {
static_cast<HDF5Definitions::FloatType>(vel[1]),
static_cast<HDF5Definitions::FloatType>(vel[2])});
vecId.emplace_back(id);
// track the highest particle id that was written to the file
// All particles that have a higher id than the highest id from the last time something was written are new
// and their static properties need to be recorded.
maxParticleId = std::max(maxParticleId, id);
if (maxWrittenParticleID == 0 or id > maxWrittenParticleID) {

// only add properties not yet added
if (addedConstantPropertiesIds.find(id) == addedConstantPropertiesIds.end()) {
addedConstantPropertiesIds.insert(id);
Comment on lines +110 to +111
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this potentially expensive? We do this if for every particle (O(N)) and the find is in O(log(M)) where M==addedConstantPropertiesIds.size() which is >=N.
For now I would be ok with merging this but please add a comment here that this might need improvement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is :( I can add a comment

newConstantProperties.emplace_back(
HDF5Definitions::ParticleConstantProperties{id,
particle.getIdentifier().c_str(),
Expand All @@ -127,7 +135,6 @@ void HDF5Writer::writeParticles(size_t iteration, const AutoPas_t &autopas) {
_file.appendTableRecords(newConstantProperties, particleConstantPropertiesFullPath);
}

maxWrittenParticleID = maxParticleId;
#endif
}

Expand Down
11 changes: 7 additions & 4 deletions src/ladds/io/hdf5/HDF5Writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ class HDF5Writer final : public ConjuctionWriterInterface {
* @param replace if true replace an existing file, else append.
* @param compressionLevel
*/
HDF5Writer(const std::string &filename, bool replace, unsigned int compressionLevel);
HDF5Writer(const std::string &filename,
bool replace,
unsigned int compressionLevel,
const std::set<size_t> &alreadyExistingIds);

~HDF5Writer() override = default;

Expand All @@ -49,10 +52,10 @@ class HDF5Writer final : public ConjuctionWriterInterface {

private:
/**
* Highest partilce ID that was written in any previous iteration.
* For anything below this ID constant particle properties are already written.
* Contains particle IDs in order to add a particles constantProperties only once.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Contains particle IDs in order to add a particles constantProperties only once.
* Set of IDs that are already written to the file.

Is this correct? Maybe rename it to writtenIDs ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically not true during the execution of writeParticles(), but it doesn't really matter right? Should I apply this suggestion?

*/
HDF5Definitions::IntType maxWrittenParticleID{0};
std::set<HDF5Definitions::IntType> addedConstantPropertiesIds{};

#ifdef LADDS_HDF5
/**
* Actual file that will be created. All of the data this writer gets ends up in this one file.
Expand Down
8 changes: 6 additions & 2 deletions src/ladds/particle/Constellation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ Constellation::Constellation(ConfigReader &constellationConfig, ConfigReader &co
schedule[i].push_back(timestamps[i] + j * timeStepSize);
}
}

// if checkpoint is loaded, throw away already inserted satellites
if (config.defines("io/hdf5/checkpoint/file")) {
tick(config.get<size_t>("io/hdf5/checkpoint/iteration", -1, true));
}
Comment on lines +73 to +76
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some more explanatory comments here? I don't fully get this. So if a checkpoint is used you call tick() but if no iteration of the checkpoint is defined you pass -1 which is actually 2^64 -1 because it is a size_t. In tick() we will hit the inactive case because we just initialized status. There you then cast this back to a signed long, hence the if in Line 111 may or may not be true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed the startingIteration as it is in the simulation class, but instead of passing it, I copied what was assigned to startingIteration. I see now though that this is false, because the fallback value of -1 used in Simulation would be added +1 and would then be 0 (right?). I mean this snippet:

const auto startingIteration = config.defines("io/hdf5", true) ? config.get<size_t>("io/hdf5/checkpoint/iteration", -1, true) + 1 : 0;

Is there actually a case where the -1 (+1) becomes relevant? Because if a checkpoint is used, but no iteration is given, the highest iteration found in the checkpoint is always loaded right?

Copy link
Collaborator Author

@albertNos albertNos May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I casted simulationTime to long, because startTime can be negative (if insertion started before referenceTime or before the startingIteration when a checkpoint is loaded) and the negative number was then evaluated to be bigger than simulationTime since both numbers were apparently treated as size_t numbers. So the if in Line 111, when loading a checkpoint, is true if the loaded iteration is later than the scheduled start of the constellation (insertion started during simulation that produced the loaded checkpoint)

}

void Constellation::setStartTime(const std::string &startTimeStr, const std::string &refTimeStr) {
Expand All @@ -95,7 +100,7 @@ void Constellation::setDuration(const std::string &durationStr) {
}
}

std::vector<Particle> Constellation::tick() {
std::vector<Particle> Constellation::tick(size_t simulationTime) {
std::vector<Particle> particles{};
switch (status) {
case Status::deployed:
Expand Down Expand Up @@ -135,7 +140,6 @@ std::vector<Particle> Constellation::tick() {
timeActive += interval;
break;
}
simulationTime += interval;
return particles;
}

Expand Down
2 changes: 1 addition & 1 deletion src/ladds/particle/Constellation.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Constellation {
* and linearly over time.
* @return std::vector<Particle> : satellites to be added to the simulation.
*/
std::vector<Particle> tick();
std::vector<Particle> tick(size_t simulationTime);
Comment on lines 38 to +41
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc for new parameter missing


/**
* Offsets all local constellation IDs by the parameter baseId to create global IDs.
Expand Down
8 changes: 4 additions & 4 deletions tests/testladds/io/hdf5/HDF5WriterReaderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ TEST_F(HDF5WriterReaderTest, WriteReadTestParticleData) {
// 2. write data
constexpr auto filename = "WriteReadTestParticleData.h5";
constexpr size_t iterationNr = 42;
HDF5Writer hdf5Writer(filename, true, 4);
HDF5Writer hdf5Writer(filename, true, 4, {});
hdf5Writer.writeParticles(iterationNr, autopas);

// 3. read data and check that read data is equal to generated data
Expand Down Expand Up @@ -103,7 +103,7 @@ TEST_F(HDF5WriterReaderTest, WriteReadTestCollisionData) {
// 2. write data
constexpr auto filename = "WriteReadTestCollisionData.h5";
constexpr size_t iterationNr = 42;
HDF5Writer hdf5Writer(filename, true, 4);
HDF5Writer hdf5Writer(filename, true, 4, {});
hdf5Writer.writeConjunctions(iterationNr, conjunctions);

// 3. read data
Expand Down Expand Up @@ -162,7 +162,7 @@ TEST_F(HDF5WriterReaderTest, AppendCheckpointTest) {
constexpr auto filename = "AppendCheckpointTest.h5";
constexpr size_t iterationStepA{42};
{
HDF5Writer hdf5WriterReplace(filename, true, 4);
HDF5Writer hdf5WriterReplace(filename, true, 4, {});
hdf5WriterReplace.writeParticles(iterationStepA, autopas);
hdf5WriterReplace.writeConjunctions(iterationStepA, conjunctionsStepA);
}
Expand All @@ -181,7 +181,7 @@ TEST_F(HDF5WriterReaderTest, AppendCheckpointTest) {
Particle::calculateBcInv(0., 1., 1., 2.2)});
autopas.addParticle(particles.back());
{
HDF5Writer hdf5WriterAppend(filename, false, 4);
HDF5Writer hdf5WriterAppend(filename, false, 4, {});
hdf5WriterAppend.writeParticles(iterationStepB, autopas);
hdf5WriterAppend.writeConjunctions(iterationStepB, conjunctionsStepB);
}
Expand Down