Skip to content

Comments

[Bug] Resume with multiple particle_types#115

Open
cgeudeker wants to merge 4 commits intomasterfrom
bugfix/resumeParticleType
Open

[Bug] Resume with multiple particle_types#115
cgeudeker wants to merge 4 commits intomasterfrom
bugfix/resumeParticleType

Conversation

@cgeudeker
Copy link
Collaborator

Describe the PR
When resuming a simulation with multiple particle_types, a full set of particles would be generated for each particle type. Additionally, it seems that each instance of the same particle type would be loaded in full, though subsequent generation would likely replace the previous. As such the following modifications have been made:

  • In ParticlePODTypeName, each distinct particle type has been assigned a specific POD type name, though these all lead to the eventual use of the "particles" POD type for file reading and writing.
  • In initialise_particle_types, a modification has been made to only add distinct particle types.
  • New .h5 files for each particle_type will be generated as a result, ensuring no multiple generations of the same particle upon resume.

Related Issues/PRs
N/A

Additional context
N/A

…sures that resume does not load particle types multiple times. Also only adds unique particle types to particle_types_. Not clanged. Not tested.
@cgeudeker cgeudeker added the bug Something isn't working label Dec 14, 2025
@codecov
Copy link

codecov bot commented Dec 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.76%. Comparing base (b690d4a) to head (5893b3f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #115   +/-   ##
=======================================
  Coverage   95.76%   95.76%           
=======================================
  Files         244      244           
  Lines       51467    51470    +3     
=======================================
+ Hits        49284    49287    +3     
  Misses       2183     2183           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bodhinandach bodhinandach self-requested a review December 14, 2025 18:30
Copy link
Member

@bodhinandach bodhinandach left a comment

Choose a reason for hiding this comment

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

@cgeudeker Thanks for finding and fixing this. Perhaps it is good to make some tests, if it is not too difficult?

@cgeudeker
Copy link
Collaborator Author

@cgeudeker Thanks for finding and fixing this. Perhaps it is good to make some tests, if it is not too difficult?

Yes will work on a couple.

@bodhinandach bodhinandach requested a review from Curiim December 17, 2025 10:11
Comment on lines -1646 to -1658
//! Read HDF5 particles with type name
template <unsigned Tdim>
bool mpm::Mesh<Tdim>::read_particles_hdf5(const std::string& filename,
const std::string& type_name,
const std::string& particle_type) {
bool status = false;
if (type_name == "particles" || type_name == "fluid_particles")
status = this->read_particles_hdf5(filename, particle_type);
else if (type_name == "twophase_particles")
status = this->read_particles_hdf5_twophase(filename, particle_type);
return status;
}

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't we still need these lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Becomes the read_hdf5() function in mpm_base.

Comment on lines -529 to -540
for (const auto ptype : particle_types_) {
std::string attribute = mpm::ParticlePODTypeName.at(ptype);
std::string extension = ".h5";

auto particles_file =
io_->output_file(attribute, extension, uuid_, step_, this->nsteps_)
.string();

// Load particle information from file
mesh_->read_particles_hdf5(particles_file, attribute, ptype);
}

Copy link
Member

Choose a reason for hiding this comment

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

Any specific reasons why this is moved to read_hdf5() function? Just to tidy it up?

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 its mainly a tidy up, better follows the write approach as well. Just makes it a little more readable in my opinion, can switch it back if we want.

Comment on lines -442 to -449
//! Read HDF5 particles with type name
//! \param[in] filename Name of HDF5 file to write particles data
//! \param[in] typename Name of particle type name
//! \param[in] particle_type Particle type to be generated
//! \retval status Status of reading HDF5 output
bool read_particles_hdf5(const std::string& filename,
const std::string& type_name,
const std::string& particle_type);
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Becomes the read_hdf5() function in mpm_base.

@cgeudeker
Copy link
Collaborator Author

cgeudeker commented Dec 17, 2025

Updates from attempts with multi-particle-type simulations with pml simulation:

  • 🟢 Write functions working as expected in serial without resume.
  • 🟢 Read and write functions working as expected in serial after resume.
  • 🟡 Write functions maybe working with mpi without resume.
  • 🟡 Read and write functions maybe working with mpi after resume.

Received a segmentation issue in the first time simulating with MPI and then received the previously observed pml-specific material information error in the first resume attempt. I have not been able to reproduce either and have only had it happen in the first instance of using either. After first instances, both have worked in each call so far. Screenshots of both cases below.
Without resume:
image
With resume:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants