Simplified fields by joyal-mathew · Pull Request #180 · SCOREC/pcms

joyal-mathew · 2025-06-04T20:58:19Z

This PR introduces the new simplified field interface.

We use Kokkos MDSpan for everything now.

This refactor was made in preparation for updates that will use Rank2 arrays as well for consistency.

jacobmerson · 2025-08-21T22:35:58Z

I'm working through some issues with this pull request. Progress can be found here: https://github.com/jacobmerson/pcms/tree/simplified-field-cleanup

jacobmerson · 2025-08-23T05:58:48Z

+
+  auto coords = mesh_.coords();
+
+  size_t offset = 0;


We should somehow be getting this information from MeshFIelds.

SCOREC/meshFields#70

jacobmerson · 2025-08-23T06:18:38Z

+  auto channel =
+    rdv.CreateAdiosChannel("field2_chan1", params, redev::TransportType::BP4);
+
+  auto layout = pcms::OmegaHFieldLayout(mesh, layout_arr, 1);


I think the way we handle the field definition and layout is close, but not quite right. I expect we want a factory function that will create the field layout where the user would request for example a certain order field, or an integration point based field.

Some basic pseudo code with current thoughts, but we will probably want to iterate on this.

auto layout = CreateLagrangeLayout(mesh, order, num_components, coordinate_system); auto layout2 = CreateLagrangeIntegrationPointLayout(mesh, order, num_components, coordinate_system); auto layout3 = CreatePointCloudLayout(coordinates); auto data = View(layout.GetNumOwnedDOFHolders()); // factory function can determine the appropriate field type based on the layout auto field = CreateField(data, layout); auto field2 = CreateField(layout2); // helper to set field with a function. field2.SetDofHolders([&](coords){foo(coords);} auto data3 = View(layout3.GetNumOwnedDOFHolders()); auto field3 = CreateField(layout3); field3.SetDofHolders(data3);

Note that the fields adapter no longer needs a name. The reason that OmegaHFieldAdapter had a name is because it used that to access the backend tag storage.

- Handle dof holder data using Rank1View - Make CreateField a method of FieldLayout

jacobmerson · 2025-09-26T18:23:54Z

@joyal-mathew Can you resolve the conflicts?

joyal-mathew · 2025-09-26T18:34:22Z

Yes, I'll resolve these conflicts today.

jacobmerson · 2025-09-26T18:36:29Z

Thanks! @Sichao25 added modules on the SCOREC systems for clang-tidy and cmake-format

joyal-mathew · 2025-09-28T03:21:32Z

It seems like there are some false positives from clang-tidy. It complains about these two cases being the same (error: switch has 2 consecutive identical branches):

pcms/src/pcms/adapter/omega_h/omega_h_field2.cpp:131

      switch (mesh_.dim()) {
        case 1:
          mesh_field_ = std::make_unique<MeshFieldBackendImpl<1, 1>>(mesh_);
          break;
        case 2:
          mesh_field_ = std::make_unique<MeshFieldBackendImpl<2, 1>>(mesh_);
          break;
      }

I think it may be getting confused by the template.

Sichao25

Added a few minor questions and suggestions for consideration.

Sichao25 · 2025-10-02T22:10:27Z

+  Omega_h::Mesh& mesh_;
+  std::unique_ptr<MeshFieldBackend> mesh_field_;
+  GridPointSearch search_;
+  Kokkos::View<Real*> dof_holder_data_;


I noticed that some Views are explicitly specified with HostMemorySpace while others are not. Looks like some features are intended to run on CPU only. Would it be helpful to ensure consistent memory space annotations?

Sichao25 · 2025-10-02T22:12:08Z

+    owned_("", coords.extent(0)),
+    gids_("", coords.extent(0))
+{
+  compoents_ = 1;


Is "compoents" intentional, or a typo?

Sichao25 · 2025-10-02T22:14:19Z

+    GlobalIDView<HostMemorySpace> received_gids(
+      received_msg.data_handle() + offset + 5, length);
+
+    PCMS_ALWAYS_ASSERT(offset + 5 + length - 1 < received_msg.size());


The value 5 appears multiple times throughout the code. Would it be helpful to extract this into a named constant?

Sichao25 · 2025-10-02T22:20:51Z

+    CoordinateView<HostMemorySpace> coordinate_view) const override;
+
+  void Evaluate(LocalizationHint location,
+                FieldDataView<double, HostMemorySpace> results) const override;


Would Real be preferred over double?

Sichao25 · 2025-10-02T22:22:26Z

+  ts::ClassificationPartition cp;
+  std::map<redev::ClassPtn::ModelEnt, int> ent_to_rank;
+
+  for (int e = 0; e <= 0; ++e) {


Does this mean that this block will only trigger once. If so,could this be simplified?

Copilot

Pull Request Overview

This PR introduces a new simplified field interface for PCMS, providing a more streamlined API for field operations and data management. The changes include new field abstractions, layout classes, and communication utilities to improve the handling of field data across different computational meshes.

Introduces new field interface classes (FieldT, FieldLayout, FieldDataView) with simplified APIs
Adds communication support for fields with layout-aware serialization/deserialization
Implements concrete field types for Omega_h meshes and point clouds with evaluation capabilities

Reviewed Changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/test_spline_interpolator.cpp	Updates Kokkos extents syntax for consistency
test/test_field_*.cpp	New test files for field evaluation, interpolation, copying, and communication
test/CMakeLists.txt	Adds test configurations for new field functionality
src/pcms/field.h	Core field interface definitions and data view classes
src/pcms/field_layout.h	Abstract field layout interface for DOF management
src/pcms/transfer_field2.h	Field transfer operations (copy and interpolate)
src/pcms/create_field.cpp	Factory functions for creating Lagrange field layouts
src/pcms/coordinate_system.h	Coordinate system abstractions and transformations
src/pcms/adapter/omega_h/	Omega_h-specific field implementations
src/pcms/adapter/point_cloud/	Point cloud field implementations
src/pcms/field_communicator.h	Communication utilities for distributed field operations

Comments suppressed due to low confidence (2)

src/pcms/interpolator/adj_search.hpp:1

Removed unused variable nvertices_source which improves code clarity by eliminating dead code.

#ifndef ADJ_SEARCH_HPP

src/pcms/interpolator/adj_search.hpp:1

Removed unused variable nvertices_source which improves code clarity by eliminating dead code.

#ifndef ADJ_SEARCH_HPP

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-03T15:31:53Z

+
+private:
+  int dim_;
+  int compoents_;


Corrected spelling of 'compoents_' to 'components_'.

Suggested change

int compoents_;

int components_;

jacobmerson · 2025-10-09T16:37:43Z

@joyal-mathew it looks like github actions is using the actions from this pull request. If you merge develop and push the updated CI from #206 should hopefully resolve the build issues.

jacobmerson · 2025-10-10T03:56:30Z

It looks like the integration of MeshFields in the cmake workflow is still not quite right. One good thing is that it looks like the workflow can be updated in this PR and doesn't need to be merged through from main.

joyal-mathew · 2025-10-10T03:58:11Z

Yea, I'll get that fixed on this branch.

jacobmerson · 2025-10-15T04:46:31Z

0001-update-meshfields-workflow.patch

@joyal-mathew Can you give this patch a try?

jacobmerson · 2025-10-15T05:29:50Z

Looks like some minor clang-tidy issues, but the build/tests are back to passing.

jacobmerson and others added 17 commits March 9, 2025 22:36

remove combiners that are no longer used after gather/scatter removal

31af6e6

remove non standar library span

45c4a92

We use Kokkos MDSpan for everything now.

remove unused coordinate array types

af310c0

ScalarArrayView -> Rank1View

01ded73

This refactor was made in preparation for updates that will use Rank2 arrays as well for consistency.

move field adapters into folder

f5379b5

remove CoordinateElement type from OmegaH fields

511f179

add field layout and fiel2 that uses layout

1babbee

initial api transition steps

9319692

rough interpolate_field code

4a01211

refined omega_h evaluation adapter

fb988ec

testing field evaluation and copy

543165f

test interpolate field & "NodalData" -> "DOFHolderData"

4487613

field communicator implementation and test

23f5d57

added profiling statements

6ae0902

made communicator layout specific instead of field specific

1fdbbcb

factor out FieldLayoutCommunicator from FieldCommunicator2

fbf6f38

merge scorec/pcms/develop

4aee586

jacobmerson mentioned this pull request Jun 8, 2025

Create concept for mesh that fields can share #122

Closed

joyal-mathew added 7 commits June 20, 2025 15:56

add meshFields dependency

d335090

initial field generalization

ac78d44

pcms meshFields backend implementation

ae1c1ea

initial generalized field communication

68970c8

field communication update

96a0733

Communication bug fixes

46a0a74

initial point cloud adapter implementation

9e9b2bf

joyal-mathew marked this pull request as ready for review August 16, 2025 03:42

jacobmerson self-requested a review August 19, 2025 18:27

jacobmerson added 2 commits August 21, 2025 18:52

Merge branch 'develop' into merge-simplified-fields

c019435

move header files to headerfile list

3b7fa59

cleanup compiler warnings

97b862c

jacobmerson reviewed Aug 26, 2025

View reviewed changes

joyal-mathew added 2 commits September 20, 2025 16:40

initial interface redesign

436de23

update/simplify new api

f53566f

- Handle dof holder data using Rank1View - Make CreateField a method of FieldLayout

joyal-mathew added 3 commits September 26, 2025 21:41

Merge branch 'main' into simplified-fields

99a796a

formatting update

d521b8c

updated .git-blame-ignore-revs

e6d731c

Sichao25 reviewed Oct 2, 2025

View reviewed changes

jacobmerson requested a review from Copilot October 3, 2025 15:30

Copilot AI reviewed Oct 3, 2025

View reviewed changes

cleaned up code

40bb4f3

jacobmerson mentioned this pull request Oct 6, 2025

add MeshFields dependency to CI/CD #205

Closed

joyal-mathew added 2 commits October 9, 2025 22:41

Merge branch 'develop' into simplified-fields

500105f

formatting

3b02339

fix workflow build script

bcb407d

applied meshfields workflow patch

e9ec776

joyal-mathew added 2 commits October 19, 2025 20:50

fixed clang-tidy issues

3796c4e

actually fixed all clang-tidy issues

bf6a502

jacobmerson approved these changes Oct 27, 2025

View reviewed changes

jacobmerson merged commit 0711fbf into SCOREC:develop Oct 27, 2025
4 checks passed

Sichao25 mentioned this pull request Dec 9, 2025

Improvements of simplified field implementation #233

Closed

Conversation

joyal-mathew commented Jun 4, 2025

Uh oh!

jacobmerson commented Aug 21, 2025

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!

jacobmerson commented Sep 26, 2025

Uh oh!

joyal-mathew commented Sep 26, 2025

Uh oh!

jacobmerson commented Sep 26, 2025

Uh oh!

joyal-mathew commented Sep 28, 2025

Uh oh!

Sichao25 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!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

jacobmerson commented Oct 9, 2025

Uh oh!

jacobmerson commented Oct 10, 2025

Uh oh!

joyal-mathew commented Oct 10, 2025

Uh oh!

jacobmerson commented Oct 15, 2025

Uh oh!

jacobmerson commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants