NCI accessors: chunk_edge (normal, intersection_factor) and point (transfer_function) #1339

int-ptr-ptr · 2025-11-01T02:50:38Z

Description

Please describe the changes/features in this pull request.

updates the point impl_load for nonconforming interfaces
adds new chunk_edge containers for compute_coupling
- since different flux schemes need different data, there is no one-size-fits-all interface_data container. We can decide how to handle delegation along with the accessor compatibility update we will have to do in the future for coupled interfaces.

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

Rohit-Kakodkar

Small changes.

Rohit-Kakodkar · 2025-11-03T13:52:58Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+  KOKKOS_INLINE_FUNCTION const TransferViewType &get_transfer_function() const {
+    return transfer_function_self;
+  }


We dont need the get function since transfer_function_self is itself public.

get_transfer_function() is used to abstract the difference between transfer_function_self and transfer_function_coupled so that the same code can be used.

As an example, here's the transfer algorithm using it, where interface_data has already been checked to inherit only one of transfer_function<is_self=true> and transfer_function<is_self=false>. get_transfer_function() is used instead of something like

(stores_self_transfer_function<decltype(interface_data)>::type)? interface_data.transfer_function_self : interface_data.transfer_function_coupled

Would storing a const TransferViewType& field be better than the function?

I am not getting this. What cant you directly reference it in the line you linked const auto& transfer_functions = interface_data.transfer_function

I am not getting this. What cant you directly reference it in the line you linked const auto& transfer_functions = interface_data.transfer_function

transfer_function_self is different from transfer_function_coupled. You would need a compile-time ternary, which is what the get_transfer_function() method is.

SPECFEMPP/core/specfem/point/impl/nonconforming_transfer_function.hpp

Lines 57 to 62 in 68e7089

/** @brief Transfer function (edge -> mortar). Only the relevant side is

* enabled.

*/

TransferViewType transfer_function_self;

TransferViewType &transfer_function = transfer_function_self;

:

SPECFEMPP/include/algorithms/transfer.hpp

Lines 108 to 111 in 68e7089

intersection_point_view(icomp) +=

coupled_field(iedge, ipoint_edge, icomp) *

interface_data.transfer_function(iedge, ipoint_edge,

ipoint_intersection);

Is this what you had in mind?

Rohit-Kakodkar · 2025-11-03T13:55:03Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+  template <typename MemberType>
+  KOKKOS_INLINE_FUNCTION nonconforming_transfer_function(const MemberType &team)
+      : transfer_function_self(team.team_scratch(0)) {}
+
+  /**
+   * @brief Get the amount memory in bytes required for shared memory
+   *
+   * @return int Amount of shared memory in bytes
+   */
+  constexpr static int shmem_size() { return TransferViewType::shmem_size(); }


Point views would not use a team allocator. Point views are instantiated in register space and not scrach space. For example, shmem_size() does not exist for datatype::VectorPointViewType

Rohit-Kakodkar · 2025-11-03T13:55:24Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+   * enabled.
+   */
+  TransferViewType transfer_function_coupled;
+  KOKKOS_INLINE_FUNCTION const TransferViewType &get_transfer_function() const {


See comment above.

Rohit-Kakodkar · 2025-11-03T13:55:34Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+  template <typename MemberType>
+  KOKKOS_INLINE_FUNCTION nonconforming_transfer_function(const MemberType &team)
+      : transfer_function_coupled(team.team_scratch(0)) {}
+
+  /**
+   * @brief Get the amount memory in bytes required for shared memory
+   *
+   * @return int Amount of shared memory in bytes
+   */
+  constexpr static int shmem_size() { return TransferViewType::shmem_size(); }


See comment above.

Rohit-Kakodkar · 2025-11-03T13:56:45Z

core/specfem/assembly/nonconforming_interfaces/dim2/impl/nonconforming_interface.hpp

                specfem::data_access::is_point<PointType>::value, int> = 0>
  KOKKOS_FORCEINLINE_FUNCTION void impl_load(const IndexType &index,
                                             PointType &point) const {
-    if constexpr (on_device) {


Was this wrong before?

It was outdated and never had to be compiled, since no point<connection_tag=nonconforming> accessor existed.

Can you merge branch issue-1350 into this. That branch should provide a fix for this.

lsawade

LGTM

Rohit-Kakodkar

See comments.

Rohit-Kakodkar · 2025-11-05T13:05:30Z

core/specfem/chunk_edge/impl/nonconforming_transfer_function.hpp

-  KOKKOS_INLINE_FUNCTION const TransferViewType &get_transfer_function() const {
-    return transfer_function_self;
-  }
+  TransferViewType &transfer_function = transfer_function_self;


I still dont understand why you need a reference type? Can you not name the above variable just transfer_function?

There will be a flux scheme where compute_coupling needs both transfer_function_self and transfer_function_coupled, and it makes more sense to do it this way than to do something like
intersection_data.specfem::chunk_edge::nonconforming_transfer_function<true,...>::transfer_function.

Right now, the compatibility checker looks for the field names, so if this is something to change, I would rather wait until we rework that.

Would updating this to const auto &transfer_function = interface_container.transfer_function_self in the transfer kernel do the trick?

Rohit-Kakodkar · 2025-11-05T13:06:20Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+  /**
+   * @brief Constructor that initializes data views in Scratch
+   * Memory.
+   *
+   * @tparam MemberType Kokos team member type.
+   * @param team Kokkos team member.
+   */
+  template <typename MemberType>
+  KOKKOS_INLINE_FUNCTION nonconforming_transfer_function(const MemberType &team)
+      : transfer_function_self(team.team_scratch(0)) {}


Point views should not have team operator.

Rohit-Kakodkar · 2025-11-05T13:06:32Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+  /**
+   * @brief Constructor that initializes data views in Scratch
+   * Memory.
+   *
+   * @tparam MemberType Kokos team member type.
+   * @param team Kokkos team member.
+   */
+  template <typename MemberType>
+  KOKKOS_INLINE_FUNCTION nonconforming_transfer_function(const MemberType &team)
+      : transfer_function_coupled(team.team_scratch(0)) {}


Same as above

Rohit-Kakodkar · 2025-11-05T13:46:31Z

core/specfem/point/impl/nonconforming_transfer_function.hpp

+          specfem::dimension::type DimensionTag, bool UseSIMD>
+struct nonconforming_transfer_function;
+
+template <int NQuadIntersection, bool UseSIMD>


I dont think we'd be able to SIMD vectorize compute coupling yet just because of how data is layed out.

- [x] Update index function in edge/intersection iterator - [x] Update `for_all` execution function

Rohit-Kakodkar · 2025-11-07T19:01:56Z

core/specfem/assembly/nonconforming_interfaces/dim2/impl/nonconforming_interface.hpp

                specfem::data_access::is_point<PointType>::value, int> = 0>
  KOKKOS_FORCEINLINE_FUNCTION void impl_load(const IndexType &index,
                                             PointType &point) const {
-    if constexpr (on_device) {


Can you merge branch issue-1350 into this. That branch should provide a fix for this.

revert ncmarked_conforming_grid_curved CMakeLists.txt sources to relative (to match ncmarked_conforming_grid_flat CmakeLists)

additional accessors necessary for dg2d kernel

a27de06

int-ptr-ptr requested review from Rohit-Kakodkar, icui and lsawade November 1, 2025 02:50

Rohit-Kakodkar requested changes Nov 3, 2025

View reviewed changes

lsawade approved these changes Nov 3, 2025

View reviewed changes

removed shmem_size from point accessors

69a6fc9

This was linked to issues Nov 3, 2025

Add point specializations #1257

Open

Add data_access functions (load_on_device etc.) #1256

Open

icui approved these changes Nov 3, 2025

View reviewed changes

replaced get_transfer_function method with reference

68e7089

int-ptr-ptr requested a review from Rohit-Kakodkar November 3, 2025 22:21

int-ptr-ptr and others added 4 commits November 3, 2025 18:41

Merge branch 'devel' into nci-accessor

c6fac7c

gpu not compiling again -- points to ncmarked_conforming_grid_flat

8ad6e5a

Merge branch 'devel' into nci-accessor

023a51b

modified cmakelists

19f2ef5

Rohit-Kakodkar reviewed Nov 5, 2025

View reviewed changes

Rohit-Kakodkar and others added 4 commits November 5, 2025 11:55

Fix bug in coupled interfaces

f05a2e7

- [x] Update index function in edge/intersection iterator - [x] Update `for_all` execution function

removed scratch constructor from point

6ac86d0

removed simd

d3fd0ff

removed simd from test -- compiles now

031c499

int-ptr-ptr requested a review from Rohit-Kakodkar November 5, 2025 17:52

Rohit-Kakodkar and others added 2 commits November 5, 2025 13:30

compiler fix

7cbbdee

polarized transfer algorithm, removed ambiguated transfer_function ref

a3ca78c

int-ptr-ptr mentioned this pull request Nov 6, 2025

Fix bug in coupled interfaces #1351

Merged

10 tasks

Merge branch 'devel' into issue-1350

3b074d7

Rohit-Kakodkar approved these changes Nov 7, 2025

View reviewed changes

int-ptr-ptr added 2 commits November 7, 2025 14:32

Merge remote-tracking branch 'public_repo/issue-1350' into nci-accessor

2a9b25c

ncmarked_conforming_grid_curved: rel src

c6df95e

revert ncmarked_conforming_grid_curved CMakeLists.txt sources to relative (to match ncmarked_conforming_grid_flat CmakeLists)

	/** @brief Transfer function (edge -> mortar). Only the relevant side is
	* enabled.
	*/
	TransferViewType transfer_function_self;
	TransferViewType &transfer_function = transfer_function_self;

	intersection_point_view(icomp) +=
	coupled_field(iedge, ipoint_edge, icomp) *
	interface_data.transfer_function(iedge, ipoint_edge,
	ipoint_intersection);

NCI accessors: chunk_edge (normal, intersection_factor) and point (transfer_function) #1339

Are you sure you want to change the base?

NCI accessors: chunk_edge (normal, intersection_factor) and point (transfer_function) #1339

Uh oh!

Conversation

int-ptr-ptr commented Nov 1, 2025

Description

Issue Number

Checklist

Uh oh!

Rohit-Kakodkar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

int-ptr-ptr Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lsawade left a comment

Choose a reason for hiding this comment

Uh oh!

Rohit-Kakodkar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rohit-Kakodkar Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

int-ptr-ptr Nov 3, 2025 •

edited

Loading

Rohit-Kakodkar Nov 6, 2025 •

edited

Loading