Issue 1711 - Update the MPI adjancies / MPI reader

[Rohit-Kakodkar]

Description

Please describe the changes/features in this pull request.

Additional constraint on MPI surfaces
This change should remove rotational sliding across MPI surfaces.

Issue Number

If there is an issue created for these changes, link it here

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

[Copilot]

Pull request overview

Updates the MESHFEM3D MPI adjacency metadata and the corresponding C++ adjacency-graph representation/reader to support additional constraints on MPI interfaces (intended to remove rotational sliding across MPI surfaces).

Changes:

• Extend MESHFEM3D’s mpi_adjacency table to include anchor-corner indices and write them into the database.
• Add MPI-connection storage (mpi_connections_) and rename dim3 adjacency-graph access from graph() to local_connections().
• Extend the dim3 Fortran adjacency-graph reader to also read MPI adjacency data.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
fortran/meshfem3d/meshfem3D/save_databases.F90	Writes extended MPI adjacency records (adds anchor corner fields).
fortran/meshfem3d/meshfem3D/assemble_MPI.F90	Computes and stores anchor corner indices during MPI interface matching.
core/specfem/mesh/dim3/adjacency_graph/adjacency_graph.hpp	Adds MPI edge properties storage and renames local graph accessor.
core/specfem/io/mesh/impl/fortran/dim3/read_adjacency_graph.cpp	Updates reader to use local_connections() and attempts to read MPI adjacencies.

---

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

[Copilot]

The MPI adjacency record format being read here does not match what MESHFEM3D writes in save_databases.F90. The writer outputs 7 integers per row (my_ispec, neighbor_rank, neighbor_ispec, my_conn_id, neighbor_conn_id, anchor_local, anchor_remote), but this reader expects 9 integers (elem1, elem2, connection_type, orientation, ...). This will misread/corrupt the stream. Update the reader to read the exact 7-field format and apply 1-based -> 0-based conversions to the element indices (and anchor indices if C++ expects 0-based).

Suggested change

	int elem1, elem2;
	int connection_type_int, orientation_int;
	int neighbor_rank, neighbor_local_idx, local_idx, local_anchor_idx,
	neighbor_anchor_idx;
	specfem::io::fortran_read_line(stream, &elem1, &elem2, &connection_type_int,
	&orientation_int, &neighbor_rank,
	&neighbor_local_idx, &local_idx,
	&local_anchor_idx, &neighbor_anchor_idx);
	// Convert to zero-based indexing
	elem1 -= 1;
	elem2 -= 1;
	MPIEdgeProperties mpi_edge_props(
	static_cast<specfem::element_connections::type>(connection_type_int),
	static_cast<specfem::mesh_entity::dim3::type>(orientation_int),
	neighbor_rank, neighbor_local_idx, local_idx, local_anchor_idx,
	neighbor_anchor_idx);
	int my_ispec, neighbor_rank, neighbor_ispec;
	int my_conn_id, neighbor_conn_id;
	int anchor_local, anchor_remote;
	// MESHFEM3D writes 7 integers per MPI adjacency:
	// my_ispec, neighbor_rank, neighbor_ispec,
	// my_conn_id, neighbor_conn_id,
	// anchor_local, anchor_remote
	specfem::io::fortran_read_line(stream, &my_ispec, &neighbor_rank,
	&neighbor_ispec, &my_conn_id,
	&neighbor_conn_id, &anchor_local,
	&anchor_remote);
	// Convert from 1-based (Fortran) to 0-based (C++) indexing
	my_ispec -= 1;
	neighbor_ispec -= 1;
	anchor_local -= 1;
	anchor_remote -= 1;
	MPIEdgeProperties mpi_edge_props(my_ispec, neighbor_rank, neighbor_ispec,
	my_conn_id, neighbor_conn_id,
	anchor_local, anchor_remote);

[Copilot]

MPIEdgeProperties is a nested type of specfem::mesh::adjacency_graph<dim3>, but it is used here without qualification or a using alias, which will not compile. Add an alias similar to EdgeProperties (e.g., using MPIEdgeProperties = ...::MPIEdgeProperties;) or fully-qualify the type name.

[Copilot]

The MPI adjacency count is written twice (two consecutive write(IIN_database) num_mpi_adjacencies records). The C++ reader only reads a single count, so the extra record will shift the stream and corrupt all subsequent reads. Remove the redundant write so exactly one count record precedes the adjacency rows.

Suggested change

write(IIN_database) num_mpi_adjacencies

[Copilot]

compute_anchor_points_for_match returns on the first matching local corner (return inside the nested loops), which depends on the original corner ordering. This contradicts the routine’s documentation (“lexicographically smallest … independent of corner ordering”) and risks selecting different physical anchor corners on each partition, which would defeat the goal of removing rotation ambiguity. Instead, evaluate all matching corner pairs and pick a canonical anchor (e.g., by sorting corners lexicographically and choosing the minimum coordinate corner, then mapping to the corresponding remote corner).

Suggested change

	double precision :: tol
	logical :: matched_remote(4)
	logical :: found
	matched_remote(:) = .false.
	! Find the lexicographically smallest unmatched local corner
	! and its matching remote corner
	do ic = 1, ncorners
	found = .false.
	do ic_remote = 1, ncorners
	if (matched_remote(ic_remote)) cycle
	! Check if this pair of corners match within tolerance
	tol = COORD_TOL * (1.0d0 + maxval(dabs(coords_local(ic,:))))
	if (dabs(coords_local(ic,1) - coords_remote(ic_remote,1)) <= tol .and. &
	dabs(coords_local(ic,2) - coords_remote(ic_remote,2)) <= tol .and. &
	dabs(coords_local(ic,3) - coords_remote(ic_remote,3)) <= tol) then
	anchor_local = ic
	anchor_remote = ic_remote
	matched_remote(ic_remote) = .true.
	found = .true.
	return
	endif
	end do
	end do
	integer :: best_local, best_remote
	double precision :: tol
	logical :: found
	logical :: is_better
	found = .false.
	! Evaluate all matching corner pairs and select a canonical anchor
	! based on lexicographically smallest coordinates, independent of
	! input corner ordering.
	do ic = 1, ncorners
	do ic_remote = 1, ncorners
	! Check if this pair of corners match within tolerance
	tol = COORD_TOL * (1.0d0 + maxval(dabs(coords_local(ic,:))))
	if (dabs(coords_local(ic,1) - coords_remote(ic_remote,1)) <= tol .and. &
	dabs(coords_local(ic,2) - coords_remote(ic_remote,2)) <= tol .and. &
	dabs(coords_local(ic,3) - coords_remote(ic_remote,3)) <= tol) then
	if (.not. found) then
	! First matching pair found
	best_local = ic
	best_remote = ic_remote
	found = .true.
	else
	! Decide if this pair is lexicographically better than
	! the current best. First compare local coordinates,
	! then break ties using remote coordinates.
	is_better = .false.
	! Compare local coordinates lexicographically
	if (coords_local(ic,1) < coords_local(best_local,1)) then
	is_better = .true.
	else if (coords_local(ic,1) == coords_local(best_local,1)) then
	if (coords_local(ic,2) < coords_local(best_local,2)) then
	is_better = .true.
	else if (coords_local(ic,2) == coords_local(best_local,2)) then
	if (coords_local(ic,3) < coords_local(best_local,3)) then
	is_better = .true.
	else if (coords_local(ic,3) == coords_local(best_local,3)) then
	! Local coordinates are identical; use remote
	! coordinates as deterministic tie-breaker.
	if (coords_remote(ic_remote,1) < coords_remote(best_remote,1)) then
	is_better = .true.
	else if (coords_remote(ic_remote,1) == coords_remote(best_remote,1)) then
	if (coords_remote(ic_remote,2) < coords_remote(best_remote,2)) then
	is_better = .true.
	else if (coords_remote(ic_remote,2) == coords_remote(best_remote,2)) then
	if (coords_remote(ic_remote,3) < coords_remote(best_remote,3)) then
	is_better = .true.
	else if (coords_remote(ic_remote,3) == coords_remote(best_remote,3)) then
	! Final tie-breaker on remote index
	if (ic_remote < best_remote) then
	is_better = .true.
	end if
	end if
	end if
	end if
	end if
	end if
	end if
	if (is_better) then
	best_local = ic
	best_remote = ic_remote
	end if
	end if
	endif
	end do
	end do
	if (found) then
	anchor_local = best_local
	anchor_remote = best_remote
	return
	end if

[Copilot]

mpi_connections_ is declared as a std::vector, but this header does not include <vector>. This will fail to compile for translation units that include this header without already including <vector>. Add the missing include in this file.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.

---

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

[Copilot]

Renaming graph() to local_connections() removes the existing API, but there are still call sites expecting adjacency_graph<dim3>::graph() (e.g., core/specfem/mesh/dim3/mesh.cpp:5). This will break the build unless you either (a) keep graph() as a forwarding alias (possibly deprecated) or (b) update all call sites in this PR.

[Copilot]

Anchor corner IDs are written as 1-based Fortran values (19–26) but MPIEdgeProperties documents them as 0-based (18–25). The current code passes local_anchor_idx / neighbor_anchor_idx through without converting them. Subtract 1 (and validate range) when reading to keep the C++ representation consistent.

[Copilot]

MPIEdgeProperties is a nested type of specfem::mesh::adjacency_graph<dim3>, but it’s used here without a qualification or using alias (unlike EdgeProperties). Add a using MPIEdgeProperties = ...::MPIEdgeProperties; or fully qualify the name so this compiles.

[Copilot]

This Doxygen block has several formatting artifacts (\n * inside @param/@return, and the trailing * at the end of line 31) that may trigger documentation warnings or render incorrectly. Clean up the markup so the docs build remains warning-free.

[Copilot]

The face/edge mapping tables in get_element_corner_id don’t match the connection ID conventions in adjacency_graph.f90. For example, adjacency_graph.f90 defines edge IDs 7..18 as bottom_left, bottom_right, top_right, top_left, front_bottom, front_top, front_left, front_right, back_bottom, back_top, back_left, back_right; but here edge_8_map is labeled/mapped as top_left, and several other edges/faces appear inconsistent. This will produce incorrect element-absolute anchor corner IDs. Please re-derive these tables directly from the same corner ordering used in prepare_interface_coordinates_buffers / assemble_ibool_corners and the ID definitions in adjacency_graph.f90.

Suggested change

	integer, parameter :: edge_7_map(2) = [19, 21] ! bottom_left
	integer, parameter :: edge_8_map(2) = [23, 25] ! top_left
	integer, parameter :: edge_9_map(2) = [19, 20] ! front_bottom
	integer, parameter :: edge_10_map(2) = [20, 22] ! bottom_right
	integer, parameter :: edge_11_map(2) = [24, 26] ! top_right
	integer, parameter :: edge_12_map(2) = [21, 22] ! back_bottom
	integer, parameter :: edge_13_map(2) = [19, 23] ! front_left
	integer, parameter :: edge_14_map(2) = [20, 24] ! front_right
	integer, parameter :: edge_15_map(2) = [25, 21] ! back_left
	integer, parameter :: edge_16_map(2) = [26, 22] ! back_right
	integer, parameter :: edge_17_map(2) = [23, 24] ! front_top
	integer, parameter :: edge_18_map(2) = [25, 26] ! back_top
	! Order must match adjacency_graph.f90:
	! 7: bottom_left, 8: bottom_right, 9: top_right, 10: top_left,
	! 11: front_bottom, 12: front_top, 13: front_left, 14: front_right,
	! 15: back_bottom, 16: back_top, 17: back_left, 18: back_right
	integer, parameter :: edge_7_map(2) = [19, 21] ! bottom_left
	integer, parameter :: edge_8_map(2) = [20, 22] ! bottom_right
	integer, parameter :: edge_9_map(2) = [24, 26] ! top_right
	integer, parameter :: edge_10_map(2) = [23, 25] ! top_left
	integer, parameter :: edge_11_map(2) = [19, 20] ! front_bottom
	integer, parameter :: edge_12_map(2) = [23, 24] ! front_top
	integer, parameter :: edge_13_map(2) = [19, 23] ! front_left
	integer, parameter :: edge_14_map(2) = [20, 24] ! front_right
	integer, parameter :: edge_15_map(2) = [21, 22] ! back_bottom
	integer, parameter :: edge_16_map(2) = [25, 26] ! back_top
	integer, parameter :: edge_17_map(2) = [25, 21] ! back_left
	integer, parameter :: edge_18_map(2) = [26, 22] ! back_right

[Copilot]

The header comment for mpi_adjacency columns 6/7 says the anchors are corner indices in the range 1-4, but compute_anchor_points_for_match returns element-absolute corner IDs 19-26 via get_element_corner_id. Update the comment (and any downstream expectations) so the stored values and documented range match.

Suggested change

	!! col 6 = anchor_local_corner — corner index (1-4) on local element
	!! col 7 = anchor_remote_corner — corner index (1-4) on remote element
	!! col 6 = anchor_local_corner — element-absolute corner ID (19-26)
	!! on the local element
	!! col 7 = anchor_remote_corner — element-absolute corner ID (19-26)
	!! on the remote element

[Copilot]

The Fortran writer (save_databases.F90) outputs MPI adjacency rows with 7 integers: (my_ispec, neighbor_rank, neighbor_ispec, my_conn_id, neighbor_conn_id, anchor_local, anchor_remote) (and currently writes the count twice). This reader expects 9 integers with different semantics (elem1, elem2, connection_type, orientation, neighbor_rank, ...). Update the reader to match the actual binary format (field count/order) and map values to the correct C++ types.

Suggested change

	specfem::io::fortran_read_line(stream, &mpi_adjacencies);
	for (int i = 0; i < mpi_adjacencies; ++i) {
	int elem1, elem2;
	int connection_type_int, orientation_int;
	int neighbor_rank, neighbor_local_idx, local_idx, local_anchor_idx,
	neighbor_anchor_idx;
	specfem::io::fortran_read_line(stream, &elem1, &elem2, &connection_type_int,
	&orientation_int, &neighbor_rank,
	&neighbor_local_idx, &local_idx,
	&local_anchor_idx, &neighbor_anchor_idx);
	// Convert to zero-based indexing
	elem1 -= 1;
	elem2 -= 1;
	MPIEdgeProperties mpi_edge_props(
	static_cast<specfem::element_connections::type>(connection_type_int),
	static_cast<specfem::mesh_entity::dim3::type>(orientation_int),
	neighbor_rank, neighbor_local_idx, local_idx, local_anchor_idx,
	neighbor_anchor_idx);
	// Fortran writer currently outputs the MPI adjacency count twice.
	// First read: primary count.
	specfem::io::fortran_read_line(stream, &mpi_adjacencies);
	// Second read: redundant count (ignored, but consumed to keep the stream in sync).
	{
	int mpi_adjacencies_redundant;
	specfem::io::fortran_read_line(stream, &mpi_adjacencies_redundant);
	// We intentionally do not alter behavior based on a mismatch; this read
	// is solely to align with the current Fortran binary layout.
	(void)mpi_adjacencies_redundant;
	}
	for (int i = 0; i < mpi_adjacencies; ++i) {
	int my_ispec, neighbor_rank, neighbor_ispec;
	int my_conn_id, neighbor_conn_id;
	int anchor_local, anchor_remote;
	// Fortran format:
	// (my_ispec, neighbor_rank, neighbor_ispec,
	// my_conn_id, neighbor_conn_id, anchor_local, anchor_remote)
	specfem::io::fortran_read_line(stream, &my_ispec, &neighbor_rank,
	&neighbor_ispec, &my_conn_id,
	&neighbor_conn_id, &anchor_local,
	&anchor_remote);
	// Convert Fortran 1-based indices to 0-based C++ indices where applicable.
	my_ispec -= 1;
	neighbor_ispec -= 1;
	// Anchor indices are element-absolute corner IDs; convert to 0-based.
	anchor_local -= 1;
	anchor_remote -= 1;
	MPIEdgeProperties mpi_edge_props(my_ispec, neighbor_rank, neighbor_ispec,
	my_conn_id, neighbor_conn_id,
	anchor_local, anchor_remote);

[Copilot]

The documented MPI adjacency binary format (lines 37–40) doesn’t match what MESHFEM3D writes: save_databases.F90 writes 7 integers per MPI adjacency row (and currently repeats the count). Please update this comment to reflect the actual on-disk layout so readers don’t implement the wrong parser.

Suggested change

	* 3. Count of MPI adjacencies
	* 4. For each MPI adjacency: local_elem, neighbor_elem, connection_type,
	* orientation, neighbor_rank, neighbor_local_idx, local_idx,
	* local_anchor_corner_id (1-based Fortran, 19-26),
	* neighbor_anchor_corner_id (1-based Fortran, 19-26)
	* 3. Count of MPI adjacencies (N_mpi)
	* 4. For each MPI adjacency row as written by MESHFEM3D's save_databases.F90
	* (7 integers per row): local_elem, neighbor_elem, connection_type,
	* orientation, neighbor_rank, local_anchor_corner_id (1-based Fortran,
	* 19-26), neighbor_anchor_corner_id (1-based Fortran, 19-26). The Fortran
	* writer currently also repeats N_mpi on each row; this redundant value is
	* ignored by the C++ reader.

[Copilot]

compute_anchor_points_for_match claims it returns the lexicographically smallest corner pair, but the implementation returns the first matching (ic, ic_remote) encountered in the unsorted input order. This makes anchor selection dependent on the buffer corner ordering and can be non-deterministic across different interface orderings. Implement the stated lexicographic selection (e.g., sort corners while tracking original indices, then pick the smallest coordinate and map to element-absolute IDs).

[Copilot]

mpi_connections_ uses std::vector, but this header does not include <vector>. Add the missing include to avoid relying on transitive includes.