Skip to content

Moves DomainView to datatype namespace#1679

Open
Rohit-Kakodkar wants to merge 2 commits intodevelfrom
refactor-domain-view
Open

Moves DomainView to datatype namespace#1679
Rohit-Kakodkar wants to merge 2 commits intodevelfrom
refactor-domain-view

Conversation

@Rohit-Kakodkar
Copy link
Collaborator

Description

Please describe the changes/features in this pull request.

This PR removes the include folder and moves everything to new core structure

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

This PR removes the include folder and moves everything to new core structure
Copy link
Collaborator

@lsawade lsawade left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Copilot AI left a comment

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 restructures the codebase by moving DomainView and its related utilities (create_mirror_view, deep_copy) from the specfem::kokkos namespace to the specfem::datatype namespace. This reorganization consolidates datatype-related code under a unified namespace and removes the dependency on directly including domain_view.hpp by making it part of the specfem/datatype.hpp umbrella header.

Changes:

  • Moved DomainView, create_mirror_view, and deep_copy from specfem::kokkos namespace to specfem::datatype namespace
  • Updated all references throughout the codebase to use the new namespace
  • Replaced direct includes of "domain_view.hpp" with #include "specfem/datatype.hpp"
  • Modified chunk_config.hpp to use Kokkos::Experimental::simd directly instead of specfem::datatype::simd wrapper

Reviewed changes

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

Show a summary per file
File Description
core/specfem/datatype/domain_view.hpp Changed namespace from specfem::kokkos to specfem::datatype
core/specfem/datatype.hpp Added include for domain_view.hpp to make it available via umbrella header
core/specfem/parallel_configuration/chunk_config.hpp Removed specfem/datatype.hpp include, added Kokkos_SIMD.hpp include, and changed to use Kokkos SIMD directly
core/specfem/medium_container/impl/domain_container.hpp Replaced domain_view.hpp include with specfem/datatype.hpp
core/specfem/macros/data_container.hpp Updated all namespace references and comments from specfem::kokkos to specfem::datatype
core/specfem/io/impl/medium_writer.tpp Replaced domain_view.hpp include with specfem/datatype.hpp
core/specfem/data_access/container.hpp Replaced domain_view.hpp include with specfem/datatype.hpp and updated type aliases
core/specfem/assembly/jacobian_matrix/dim2/jacobian_matrix.hpp Replaced domain_view.hpp include with specfem/datatype.hpp
core/specfem/assembly/jacobian_matrix/dim2/jacobian_matrix.cpp Updated all function calls to use specfem::datatype namespace

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

#include "domain_view.hpp"
#include "specfem/datatype.hpp"
#include "specfem/element.hpp"
#include "specfem/element.hpp"
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

Duplicate include statement. The line #include "specfem/element.hpp" appears twice (lines 4 and 5). Remove one of these duplicate includes.

Suggested change
#include "specfem/element.hpp"

Copilot uses AI. Check for mistakes.
constexpr int chunk_size = impl::hip_chunk_size;
#elif defined(KOKKOS_ENABLE_OPENMP)
constexpr int simd_size = specfem::datatype::simd<type_real, true>::size();
constexpr int simd_size = Kokkos::Experimental::simd<type_real>::size();
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

This change directly uses Kokkos::Experimental::simd<type_real> instead of specfem::datatype::simd<type_real, true>. This bypasses the SPECFEM_ENABLE_SIMD compile-time flag check that exists in the wrapper class. When SPECFEM_ENABLE_SIMD is not defined, the wrapper falls back to Kokkos::Experimental::basic_simd<T, Kokkos::Experimental::simd_abi::scalar>. Directly using Kokkos::Experimental::simd<type_real> may cause different behavior or compilation issues when SPECFEM_ENABLE_SIMD is disabled. Consider using specfem::datatype::simd<type_real, true>::size() to maintain consistency with the rest of the codebase.

Copilot uses AI. Check for mistakes.
constexpr int chunk_size = impl::openmp_chunk_size;
#else
constexpr int simd_size = specfem::datatype::simd<type_real, true>::size();
constexpr int simd_size = Kokkos::Experimental::simd<type_real>::size();
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

This change directly uses Kokkos::Experimental::simd<type_real> instead of specfem::datatype::simd<type_real, true>. This bypasses the SPECFEM_ENABLE_SIMD compile-time flag check that exists in the wrapper class. When SPECFEM_ENABLE_SIMD is not defined, the wrapper falls back to Kokkos::Experimental::basic_simd<T, Kokkos::Experimental::simd_abi::scalar>. Directly using Kokkos::Experimental::simd<type_real> may cause different behavior or compilation issues when SPECFEM_ENABLE_SIMD is disabled. Consider using specfem::datatype::simd<type_real, true>::size() to maintain consistency with the rest of the codebase.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants