get_vtx_patches without kokkos by Fuad-HH · Pull Request #231 · SCOREC/omega_h

Fuad-HH · 2026-08-06T21:07:21Z

This function was enabled only when kokkos was linked. Now with the alternative implementation, it can work without kokkos.
There are two assumptions here:

Device code is only supported through kokkos -> without kokkos, it is always CPU only.
Using std::sort which needs Omega_h to be compiled with c++17 or higher.

Fixes issue #230

- all other code in the file works without kokkos excect this. - moved the kokkos linking guards only wrapping this. - passes all tests locally (including python) - fixes issue SCOREC#230

Fuad-HH · 2026-08-11T16:47:25Z

@jacobmerson @cwsmith @Sichao25 , Could you please review this? I cannot find the review request button for this PR.

cwsmith · 2026-08-11T16:55:24Z

/runtests

github-actions · 2026-08-11T18:52:02Z

Test Results:

Kokkos CUDA: success
No Kokkos: success
Kokkos Serial: success
Kokkos OpenMP: success
Kokkos CUDA Warnings as Errors: success
Kokkos CUDA Python: success

(details)

cwsmith

Thanks @Fuad-HH . Below are a few comments towards tweaking the code to make it clearer that it is intended to only run in serial.

With this change I assume test_patches should no longer be within the if(Omega_h_USE_Kokkos) conditional.

omega_h/src/CMakeLists.txt

Lines 412 to 421 in 96463bb

    
             if (Omega_h_USE_Kokkos) 
        
               osh_add_exe(bbox_reduce_test) 
        
               osh_add_exe(initKokkosAndLib) 
        
               osh_add_exe(test_patches) 
        
               if(Omega_h_USE_MPI) 
        
                 test_func(run_test_patches_par 4 ./test_patches) 
        
               else() 
        
                 test_func(run_test_patches 1 ./test_patches) 
        
               endif() 
        
             endif()

cwsmith · 2026-08-12T18:12:32Z

+[[nodiscard]] Graph adj_segment_sort(Graph& g) {
+  auto offsets = g.a2ab;
+  auto elms_r = Read(g.ab2b); //read only
+  Write<LO> elms(elms_r.size(), "elms");


even though this is outside the kokkos block, I think this should still use a HostWrite to make the intention clear

cwsmith · 2026-08-12T18:15:06Z

+  auto copyFn = OMEGA_H_LAMBDA(LO i) {
+    elms[i] = elms_r[i];
+  };
+  parallel_for(elms.size(), copyFn);
+  auto sortFn = OMEGA_H_LAMBDA(LO i) {
+    std::sort(elms.begin() + offsets[i], elms.begin() + offsets[i+1]);
+  };
+  parallel_for(g.nnodes(), sortFn);


likewise, since this is outside the kokkos block, I think it would be easier to read/understand if these were serial for loops instead of parallel_for(...)

I was thinking that in the future, omega_h parallel for may use the ranges or something else for host parallelism without kokkos and we don't want to manage it here.

Hmmm. Is std::sort expected to be thread safe is that context? I guess I'd rather do the conservative thing that we know won't break if someone does eventually implement a non-kokkos parallel backend. Note, the plan for the foreseeable future is to only support on-device parallelism via kokkos.

Okay, I will replace this. Thanks.

removed parallel_for and Read/Write moved test_patch out of kokkos condition

cwsmith · 2026-08-12T19:21:17Z

/runtests

github-actions · 2026-08-12T20:01:20Z

Test Results:

Kokkos CUDA: success
No Kokkos: failure
Kokkos Serial: skipped
Kokkos OpenMP: skipped
Kokkos CUDA Warnings as Errors: skipped
Kokkos CUDA Python: skipped

(details)

Fuad-HH · 2026-08-12T20:44:39Z

The test_patches is failing with MPI on 2 ranks where it is expected to fail but here it is getting a segfault. It is failing at this line in a Write::size() call:

omega_h/src/Omega_h_shared_alloc.hpp

Line 141 in 96463bb

return alloc->size;

Do you have any suggestion how to can I fix this?

Fuad-HH · 2026-08-12T20:52:06Z

I am not sure if this is the intended behavior of SharedAlloc. But all the tests pass with this change.

cwsmith · 2026-08-13T13:43:07Z

/runtests

github-actions · 2026-08-13T15:23:38Z

Test Results:

Kokkos CUDA: success
No Kokkos: success
Kokkos Serial: success
Kokkos OpenMP: success
Kokkos CUDA Warnings as Errors: success
Kokkos CUDA Python: success

(details)

cwsmith · 2026-08-14T14:56:30Z

@Fuad-HH The fix for SharedAlloc is valid. Thank you. This took some digging to understand and hopefully the following summary is correct. In short, SharedAlloc was designed to support serial execution -or- host parallelism via openmp without kokkos. When kokkos is enabled kokkos view's are used by read/write arrays instead of SharedAlloc. When a host-based parallel for loop captured a write/read array by value the following logic relied on the Alloc pointer having the size of the allocation directly encoded/packed into it instead of being an actual pointer to heap memory. See

omega_h/src/Omega_h_shared_alloc.hpp

Lines 79 to 81 in 96463bb

    
           if (entering_parallel) { 
        
             alloc = reinterpret_cast<Alloc*>( 
        
                 (std::uintptr_t(alloc->size) << 3) | IN_PARALLEL);

As you gathered, the problem comes when alloc is set to nullptr but the call to size() didn't distinguish between the IN_PARALLEL mask not being set (i.e., alloc is all zeros) versus nullptr:

omega_h/src/Omega_h_shared_alloc.hpp

Lines 134 to 148 in 96463bb

    
             OMEGA_H_INLINE std::size_t size() const noexcept { 
        
           #ifndef __CUDA_ARCH__ 
        
               if (!(reinterpret_cast<std::uintptr_t>(alloc) & IN_PARALLEL)) { 
        
           #if defined(__GNUC__) && (__GNUC__ >= 7) && (!defined(__clang__)) 
        
           #pragma GCC diagnostic push 
        
           #pragma GCC diagnostic ignored "-Wnull-dereference" 
        
           #endif 
        
                 return alloc->size; 
        
           #if defined(__GNUC__) && (__GNUC__ >= 7) && (!defined(__clang__)) 
        
           #pragma GCC diagnostic pop 
        
           #endif 
        
               } 
        
           #endif 
        
               return reinterpret_cast<std::uintptr_t>(alloc) >> 3; 
        
             }

Fuad-HH · 2026-08-14T15:05:38Z

Thanks @cwsmith. I also had the similar idea looking at the stack trace.

Fuad-HH mentioned this pull request Aug 6, 2026

Python API implicitly requires Kokkos #230

Open

Fuad-HH marked this pull request as ready for review August 11, 2026 14:05

alternative get_vtx_patches without kokkos

91b1cd2

- all other code in the file works without kokkos excect this. - moved the kokkos linking guards only wrapping this. - passes all tests locally (including python) - fixes issue SCOREC#230

Fuad-HH force-pushed the patches_wo_kokkos branch from 8cab4a9 to 91b1cd2 Compare August 11, 2026 14:23

cwsmith reviewed Aug 12, 2026

View reviewed changes

test patches without kokkos

18fb8c4

removed parallel_for and Read/Write moved test_patch out of kokkos condition

Fuad-HH force-pushed the patches_wo_kokkos branch from b532295 to 18fb8c4 Compare August 12, 2026 19:19

SharedAlloc::size() returns zero if nullptr

50fb275

cwsmith merged commit d5489aa into SCOREC:master Aug 14, 2026
17 checks passed

Fuad-HH deleted the patches_wo_kokkos branch August 14, 2026 15:05

	if (Omega_h_USE_Kokkos)
	osh_add_exe(bbox_reduce_test)
	osh_add_exe(initKokkosAndLib)
	osh_add_exe(test_patches)
	if(Omega_h_USE_MPI)
	test_func(run_test_patches_par 4 ./test_patches)
	else()
	test_func(run_test_patches 1 ./test_patches)
	endif()
	endif()

Uh oh!

Conversation

Fuad-HH commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fuad-HH commented Aug 11, 2026

Uh oh!

cwsmith commented Aug 11, 2026

Uh oh!

github-actions Bot commented Aug 11, 2026

Uh oh!

cwsmith left a comment

Choose a reason for hiding this comment

Uh oh!

cwsmith Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cwsmith Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Fuad-HH Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cwsmith Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Fuad-HH Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cwsmith commented Aug 12, 2026

Uh oh!

github-actions Bot commented Aug 12, 2026

Uh oh!

Fuad-HH commented Aug 12, 2026

Uh oh!

Fuad-HH commented Aug 12, 2026

Uh oh!

cwsmith commented Aug 13, 2026

Uh oh!

github-actions Bot commented Aug 13, 2026

Uh oh!

cwsmith commented Aug 14, 2026

Uh oh!

Uh oh!

Fuad-HH commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fuad-HH commented Aug 6, 2026 •

edited

Loading