Skip to content

Properly serialize particle managers and generators. - #7099

Closed
bangerth wants to merge 2 commits into
geodynamics:mainfrom
bangerth:pr-a0896619d6fb68e93ab16113f641784e8f579c06
Closed

Properly serialize particle managers and generators.#7099
bangerth wants to merge 2 commits into
geodynamics:mainfrom
bangerth:pr-a0896619d6fb68e93ab16113f641784e8f579c06

Conversation

@bangerth

Copy link
Copy Markdown
Contributor

This is a difficult one, and it took me quite a lot of tokens to find and fix, and then brainpower to understand and revise. The key issue here is that the particle manager only saves the ParticleHandler class during serialization. But it doesn't save the following:

  • Its own state in the form of a random number generator
  • The particle generators
  • The particle property manager.
    This patch addresses this, but the underlying issue is actually more complicated.

At its root, the issue is that we call the serialization functionality on all processes, but we only store the information on process zero. I don't recall the exact thought processes that led to this design decision, but I think that fundamentally it was about this:

  1. We assume that the state of a plugin is the same across all processes, and so saving on one rank is enough.
  2. We of course still have to restore on all processes, but that can be the information saved on rank zero.
  3. We want to be able to resume on a different number of processes than what we saved.

All of this gets in trouble if you have a random number generator, for example for particle generation. In fact, the particle generation base class already provides a random number generator for all of its derived classes to use (see also #7097). The issue here is that on different ranks, we may be calling the random number generator a different number of times, and so the state of the RNG may be different on different ranks. If we really want to save the state of the simulator, we have to save the state from all ranks. That's what Copilot came up with: It collects the state from all ranks to rank zero, and saves that whole array. When we resume, each process picks its state out of the array and restores from that.

I'm not excited about this approach. I think memory is ok, since if we have many processes, we also have many solution vector elements to store and the addition of the state of an RNG per rank isn't going to make a difference. But it destroys our ability to restart with a different number of processes. This is in conflict with our goal that a resumed process results in exactly the same answer as the original process (which requires saving the state from all processes).

I can see two solutions here, neither of which is implemented in the current patch:

  • If the number of restart processes is different than the number of checkpointed processes, then simply reset all RNGs to the default seed. That's not unreasonable because in that case there has never been an expectation that the resumed computation is binary identical to the original one. If so, we can as well reset RNGs. But if we restart with the exact same number of processes, we do want to restart with the checkpointed numbers.
  • We could try and avoid checkpointing all RNGs to begin with, and only checkpoint from rank zero as we do with all data. This would requiring restoring the assumption that the same plugin has the same state across all ranks. A way to achieve that would be to make sure that at the beginning of every time step, each rank's RNG is exactly the same; for example, rank 0 could send its RNG state to all other ranks and they then set their own RNG state to that of rank zero.

Thoughts?

Before your first pull request:

For all pull requests:

For new features/models or changes of existing features:

  • I have tested my new feature locally to ensure it is correct.
  • I have created a testcase for the new feature/benchmark in the tests/ directory.
  • I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.

@bangerth

Copy link
Copy Markdown
Contributor Author

Part of #6744.

= Utilities::MPI::this_mpi_process(this->get_mpi_communicator());

AssertThrow(rank_states.size() == n_processes,
ExcMessage("The number of MPI processes used to resume the "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Help me understand: does that mean we no longer support resuming with different number of ranks?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With the current version of the patch that's correct. That's why I don't like it as it stands. We need one of the two solutions at the end of this post.

@tjhei

tjhei commented Jul 23, 2026

Copy link
Copy Markdown
Member

I apologize for not reading the whole pr description beforehand. I am fine with the current approach and would opt for simply not restoring the state if the number of ranks changes. This is not a problem because the simulation will be different regardless when running with a different number of ranks.

@tjhei

tjhei commented Jul 25, 2026

Copy link
Copy Markdown
Member

would you be ok restoring the ability to resume with different number of ranks (even if random state is not restored)?

@bangerth

Copy link
Copy Markdown
Contributor Author

Yes, I should have marked this as WIP. This was meant for discussion, not for merging.

@bangerth
bangerth marked this pull request as draft July 25, 2026 12:51
@bangerth

Copy link
Copy Markdown
Contributor Author

Let me close this. It's not the way I want to go.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants