Skip to content

Support for Intel Ponte Vecchio nodes#93

Merged
haykh merged 10 commits into1.2.0rcfrom
dev/pvc
Jul 9, 2025
Merged

Support for Intel Ponte Vecchio nodes#93
haykh merged 10 commits into1.2.0rcfrom
dev/pvc

Conversation

@LudwigBoess
Copy link
Collaborator

This PR tracks the development to get entity to work on Intel PVC nodes (used e.g. in Stampede3 and Aurora).

For reference I use this configuration (with intel/24.0 and impi/21.13.1):
cmake -B build -D pgen=srpic/weibel -D precision=single -D mpi=ON -D output=OFF -D Kokkos_ENABLE_SYCL=ON -D Kokkos_ARCH_SPR=ON -D Kokkos_ARCH_INTEL_PVC=ON -DCMAKE_C_COMPILER=mpiicc -DCMAKE_CXX_COMPILER=mpiicpx

It compiled on commit 6cf0dc8e5eb44528d049b31111ff7ff81bb52bc3 after the fixes in this PR, but does not compile in the current 1.2.0rc version.

The error is:

/work2/10559/lboess/stampede3/work/libs/kokkos/include/Kokkos_ExecPolicy.hpp:112:9: error: no matching constructor for initialization of 'typename traits::execution_space' (aka 'Kokkos::Serial')
  112 |         m_space(p.m_space),
      |         ^       ~~~~~~~~~
/scratch/10559/lboess/scratch/PIC/entity_debug/entity/src/global/arch/kokkos_aliases.cpp:8:10: note: in instantiation of function template specialization 'Kokkos::RangePolicy<Kokkos::Serial>::RangePolicy<Kokkos::SYCL>' requested here
    8 |   return Kokkos::RangePolicy<Kokkos::DefaultExecutionSpace>(p1, p2);
      |          ^
/work2/10559/lboess/stampede3/work/libs/kokkos/include/Serial/Kokkos_Serial.hpp:94:7: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'const typename traits::execution_space' (aka 'const Kokkos::SYCL') to 'const Serial' for 1st argument
   94 | class Serial {
      |       ^~~~~~
/work2/10559/lboess/stampede3/work/libs/kokkos/include/Serial/Kokkos_Serial.hpp:94:7: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const typename traits::execution_space' (aka 'const Kokkos::SYCL') to 'Serial' for 1st argument
   94 | class Serial {
      |       ^~~~~~
/work2/10559/lboess/stampede3/work/libs/kokkos/include/Serial/Kokkos_Serial.hpp:118:12: note: candidate constructor not viable: no known conversion from 'const typename traits::execution_space' (aka 'const Kokkos::SYCL') to 'NewInstance' for 1st argument
  118 |   explicit Serial(NewInstance);
      |            ^      ~~~~~~~~~~~
/work2/10559/lboess/stampede3/work/libs/kokkos/include/Serial/Kokkos_Serial.hpp:124:3: note: candidate constructor template not viable: no known conversion from 'const typename traits::execution_space' (aka 'const Kokkos::SYCL') to 'NewInstance' for 1st argument
  124 |   Serial(NewInstance)
      |   ^      ~~~~~~~~~~~
/work2/10559/lboess/stampede3/work/libs/kokkos/include/Serial/Kokkos_Serial.hpp:116:3: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
  116 |   Serial();

@LudwigBoess LudwigBoess requested review from haykh and jmahlmann April 4, 2025 19:36
@LudwigBoess LudwigBoess self-assigned this Apr 4, 2025
@LudwigBoess
Copy link
Collaborator Author

It seem the issue was with kokkos, it compiles by now!

@haykh haykh added the compilation Building/compilation errors label Apr 19, 2025
@LudwigBoess LudwigBoess changed the base branch from 1.2.0rc to dev/pgens May 1, 2025 19:55
@haykh haykh changed the base branch from dev/pgens to 1.2.0rc May 12, 2025 18:29
@LudwigBoess
Copy link
Collaborator Author

I revisited this and it looks like with intel25 and kokkos/4.6.01 even virtual functions are supported.
So the only required change to work with SYCL is the change from printf to Kokkos::printf.
I think we should abandon the virtual functions and overrides anyway to allow for backwards compatibility. But besides that I think we can merge it.

@haykh
Copy link
Collaborator

haykh commented Jun 15, 2025

@LudwigBoess agree with getting rid of overrides in KOKKOS_INLINE functions. I don't think we needs these really, the point was to throw an error if the supplied metric/distribution doesn't overload a specific function.

Funny enough, I don't think below 4.4 or 4.3 of Kokkos printf were a thing. So technically, with Kokkos::printf we're still bumping the minimum requirement up (which is why I decided not to use it at the beginning). But I have no objections either way.

Since I don't have access to PVC nodes, could you try and run the default tests? Just to verify that these configurations work ok:

  • -D TESTS=ON
  • -D TESTS=ON -D mpi=ON
  • -D TESTS=ON -D precision=double

output is ON by default.

@haykh haykh removed the request for review from jmahlmann June 15, 2025 22:35
@haykh haykh merged commit 2e8ad04 into 1.2.0rc Jul 9, 2025
2 checks passed
@haykh haykh deleted the dev/pvc branch July 9, 2025 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compilation Building/compilation errors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants