Skip to content

Issue 1700 - Updates the solver to perform MPI communication and the assembly to hold buffers#1895

Open
lsawade wants to merge 11 commits into
develfrom
issue-1700
Open

Issue 1700 - Updates the solver to perform MPI communication and the assembly to hold buffers#1895
lsawade wants to merge 11 commits into
develfrom
issue-1700

Conversation

@lsawade

@lsawade lsawade commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Description

First working variant of the solver update to include MPI communication for the acceleration.

uses #1704 related PR to write the seismograms correctly.

Issue Number

Closes #1700

Checklist

Please make sure to check developer documentation on specfem docs.

  • I ran the code through pre-commit to check style
  • THE DOCUMENTATION BUILDS WITHOUT WARNINGS/ERRORS
  • I have added labels to the PR (see right hand side of the PR page)
  • My code passes all the integration tests
  • I have added sufficient unittests to test my changes
  • I have added/updated documentation for the changes I am proposing
  • I have updated CMakeLists to ensure my code builds
  • My code builds across all platforms

@lsawade lsawade added the enhancement New feature or request label Jun 3, 2026
@lsawade lsawade requested review from Rohit-Kakodkar, Copilot and icui and removed request for icui June 3, 2026 21:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces MPI communication/computation overlap in the solver by classifying elements as inner/outer and adding persistent acceleration-field exchange buffers, while also updating receiver/source ownership terminology (islicepartition_index) and enabling optional “gather-to-rank-0 then write” seismogram output.

Changes:

  • Add mpi_tag classification (inner/outer) and incorporate it into tag-dispatch validity and element-type indexing.
  • Implement a four-phase stiffness update that overlaps acceleration halo exchange with inner-element computation via new mpi_accel_buffers.
  • Update seismogram writers and receiver/sources metadata to support per-rank filtering and optional rank-0-only output; adjust/extend unit tests accordingly.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/unit-tests/serial.cmake Link unit tests against specfem::program to support Context initialization.
tests/unit-tests/io/seismogram/seismogram_writer_tests.cpp Update seismogram tests for new writer API and add stronger file comparisons.
tests/unit-tests/assembly/receivers/impl/receiver_iterator_tests.cpp Update StationInfo construction to include partition_index.
tests/unit-tests/assembly/receivers/impl/dim2/seismogram_iterator_tests.cpp Adapt tests to unified iterator + rotation-matrix behavior.
tests/unit-tests/algorithms/mpi/dim3/locate_point_test.cpp Rename ownership wording to partition_index in assertions.
tests/unit-tests/algorithms/mpi/dim2/locate_point_test.cpp Rename ownership wording to partition_index in assertions.
core/specfem/tags.hpp Add mpi_tag support to the generic Tags machinery.
core/specfem/tag_dispatch/is_valid.hpp Add non-physical tag stripping (wavefield, mpi) for combo validity checks.
core/specfem/tag_dispatch/element_combinations.hpp Add mpi_set and update validity dispatch to strip trailing non-physical tags.
core/specfem/source/source.hpp Rename source MPI owner from islice_ to partition_index_.
core/specfem/runtime_configuration/writer/seismogram.hpp Add optional write-from-main runtime configuration knob.
core/specfem/runtime_configuration/writer/seismogram.cpp Parse write-from-main and pass through to seismogram writer.
core/specfem/receivers/dim3/receiver.hpp Rename receiver MPI owner from islice_ to partition_index_.
core/specfem/receivers/dim3/receiver.cpp Update printed receiver metadata to use partition_index_.
core/specfem/receivers/dim2/receiver.hpp Rename receiver MPI owner from islice_ to partition_index_.
core/specfem/receivers/dim2/receiver.cpp Update printed receiver metadata to use partition_index_.
core/specfem/macros/tag_dispatch.hpp Add MPI_SET(...) macro to build MPI tag sets.
core/specfem/logger/logger.hpp Set debug-build default log level to TRACE.
core/specfem/io/seismogram/writer.hpp Filter stations per-rank or gather+write-from-main for seismograms.
core/specfem/io/seismogram/impl/seismogram_writer.hpp Change write API to accept an explicit station range.
core/specfem/io/seismogram/impl/sac.hpp Update SAC writer to use filtered stations and iterator metadata helpers.
core/specfem/io/seismogram/impl/channel_generator.hpp Track sample interval via nstep_between_samples.
core/specfem/io/seismogram/impl/ascii.hpp Update ASCII writer for filtered stations and add debug logging.
core/specfem/element/to_string.hpp Declare to_string(mpi_tag).
core/specfem/element/to_string.cpp Implement to_string(mpi_tag).
core/specfem/element/tags.hpp Introduce element::mpi_tag enum (inner/outer).
core/specfem/compute/update_wavefields.tpp Route stiffness update through new overlap-aware implementation.
core/specfem/compute/impl/compute_stiffness_interaction.hpp Implement overlap orchestration + mpi-tag element selection.
core/specfem/assembly/sources/impl/source_medium.hpp Use partition_index ownership when filtering sources per-rank.
core/specfem/assembly/sources/impl/locate_sources.tpp Rename islice_selectedpartition_index_selected and update ownership assignment.
core/specfem/assembly/sources.hpp Rename stored ownership vector accessor to get_partition_index().
core/specfem/assembly/sources.cpp Populate source_partition_index_ from sources’ partition ownership.
core/specfem/assembly/receivers/impl/seismogram_iterator.hpp Unify dim2/dim3 seismogram iterators; add gather-to-main via MPI.
core/specfem/assembly/receivers/impl/receiver_iterator.hpp Store stations as StationInfo objects and add station filtering.
core/specfem/assembly/receivers/dim3/receivers.hpp Remove redundant stations() wrapper (use base iterator API).
core/specfem/assembly/receivers/dim3/receivers.cpp Track per-station partition_index and set receiver partition ownership.
core/specfem/assembly/receivers/dim2/receivers.hpp Remove redundant stations() wrapper (use base iterator API).
core/specfem/assembly/receivers/dim2/receivers.cpp Build 2D rotation matrices and track per-station partition_index.
core/specfem/assembly/receivers/dim2/impl/seismogram_iterator.hpp Remove old dim2-specific iterator implementation (replaced by unified).
core/specfem/assembly/mpi/dim3/mpi.hpp Modernize host mirror typedef usage.
core/specfem/assembly/mpi_accel_buffers.hpp Add persistent acceleration exchange buffers (dim3) + dim2 no-op specialization.
core/specfem/assembly/fields/fields.hpp Add mutable simulation-field accessor.
core/specfem/assembly/element_types/impl.hpp Add mpi-tag classification, mpi-aware element index stores, and new ctor using adjacency graph.
core/specfem/assembly/element_types.hpp Add mpi_set to element tag sets and tidy typedef formatting.
core/specfem/assembly/assembly/impl/compute_wavefield_helper.hpp Use centralized dimension<>::dim rather than ad-hoc ndim logic.
core/specfem/assembly/assembly/dim3/assembly.hpp Add accel_buffers field to assembly (dim3).
core/specfem/assembly/assembly/dim3/assembly.cpp Construct mpi-aware element types and initialize accel buffers.
core/specfem/assembly/assembly/dim2/assembly.hpp Add accel_buffers no-op field to assembly (dim2).
core/specfem/assembly/assembly/dim2/assembly.cpp Construct mpi-aware element types using adjacency graph.
core/specfem/algorithms/locate_point.hpp Rename returned ownership vector to partition_index_selected.
Comments suppressed due to low confidence (2)

core/specfem/assembly/receivers/impl/seismogram_iterator.hpp:107

  • seismogram_components is never initialized. For receivers that are not owned by the rank, device entries may remain uninitialized; when sync_seismograms() followed by gather_to_main() is used, those garbage values will be included in the reduction and corrupt the rank-0 traces. Initialize the seismogram storage to 0 on construction to ensure non-owned receivers contribute zeros.
    core/specfem/assembly/receivers/impl/seismogram_iterator.hpp:160
  • In MPI_Reduce, recvbuf must be a valid pointer on all ranks (even though it is ignored on non-root ranks). Passing nullptr can crash on some MPI implementations. Use a real buffer pointer for recvbuf (it can alias sendbuf on non-root ranks since it is ignored there).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 108 to 114
return backward;
} else if constexpr (ReturnFieldType ==
specfem::simulation::field_type::buffer) {
return buffer;
} else {
static_assert("field type not supported");
}
Comment on lines +134 to +140
return backward;
} else if constexpr (ReturnFieldType ==
specfem::simulation::field_type::buffer) {
return buffer;
} else {
static_assert("field type not supported");
}
Comment thread core/specfem/io/seismogram/impl/ascii.hpp

@Rohit-Kakodkar Rohit-Kakodkar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thinking about this, maybe solver should hold the mpi_buffers - since the solver algorithm defines how the buffers are created and used. We can talk about this.

Comment thread core/specfem/assembly/element_types/impl.hpp Outdated
Comment thread core/specfem/assembly/mpi_accel_buffers.hpp
Comment thread core/specfem/compute/impl/compute_stiffness_interaction.hpp Outdated
Base automatically changed from issue-1701 to devel June 15, 2026 13:06
@lsawade

lsawade commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

MPI will be fixed with upstream PRs

@lsawade lsawade requested a review from Rohit-Kakodkar June 15, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants