Skip to content

Fix NumPy copy functions when using GPU backend - #229

Merged
cwsmith merged 4 commits into
SCOREC:masterfrom
Fuad-HH:fix_numpy_copy
Aug 11, 2026
Merged

Fix NumPy copy functions when using GPU backend#229
cwsmith merged 4 commits into
SCOREC:masterfrom
Fuad-HH:fix_numpy_copy

Conversation

@Fuad-HH

@Fuad-HH Fuad-HH commented Aug 6, 2026

Copy link
Copy Markdown
  • Add a test
  • Resolve strided numpy array

followed the other functions' way of copying
@Fuad-HH

Fuad-HH commented Aug 6, 2026

Copy link
Copy Markdown
Author

@Sichao25 , @cwsmith , Could you please add these changes into your spack pull request spack/spack-packages#5792 after adding this in a new version? Otherwise I will need to create a different pull request with this which is time consuming since the spack people take time to merge.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Please feel free to let me know if there's a better way of testing this.

@Fuad-HH

Fuad-HH commented Aug 6, 2026

Copy link
Copy Markdown
Author

Why tests are not run on the CUDA workflows? May I run the tests there?

@cwsmith

cwsmith commented Aug 6, 2026

Copy link
Copy Markdown

/runtests

@cwsmith

cwsmith commented Aug 6, 2026

Copy link
Copy Markdown

@Fuad-HH It looks like the github hosted cuda builds completed without issue (note, these don't run tests as github doesn't provide gpus). I just queued the self-hosted builds+tests.

@cwsmith
cwsmith requested a review from Sichao25 August 6, 2026 13:28
@cwsmith

cwsmith commented Aug 6, 2026

Copy link
Copy Markdown

Could you please add these changes into your spack pull request spack/spack-packages#5792 after adding this in a new version?

That is fine with me. @Sichao25 would you mind creating the new 'patch' version and including this?

@Fuad-HH @Sichao25 Are there other python interface changes coming soon we should include in the spack PR (it seems like it can/will be merged soon as we have some help).

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

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)

@jacobmerson jacobmerson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I only added the comment to one place, but all of these require two copies of the data. I'm wondering if we can somehow do this with a direct deep_copy style call from the numpy buffer. That may require wrapping in a Kokkos<unmanaged>

Comment thread src/PyOmega_h_numpy_transform.hpp
@Sichao25

Sichao25 commented Aug 6, 2026

Copy link
Copy Markdown

No problem. I'll create the patch version once the PR is merged.

@Sichao25 Sichao25 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No further questions here. This reminds me I should remove or update the related/duplicate functions in pcms as well.

@jacobmerson

Copy link
Copy Markdown
Collaborator

@Sichao25, @Fuad-HH just made an issue for that here: SCOREC/pcms#351

If we want to reuse the Omega_h functionality in PCMS (which is beneficial), we need to make sure to include the PyOmega_h_numpy_transform.hpp in the set of headers that are installed.

@Fuad-HH
Fuad-HH requested a review from jacobmerson August 6, 2026 20:14
@Fuad-HH

Fuad-HH commented Aug 6, 2026

Copy link
Copy Markdown
Author

@cwsmith, Could you please run the tests again?

@cwsmith

cwsmith commented Aug 7, 2026

Copy link
Copy Markdown

/runtests

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

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)

@jacobmerson

Copy link
Copy Markdown
Collaborator

@cwsmith this looks good to me. I had asked Fuad to add tests for strided numpy arrays which he did.

@cwsmith
cwsmith merged commit 96463bb into SCOREC:master Aug 11, 2026
@Fuad-HH
Fuad-HH deleted the fix_numpy_copy branch August 11, 2026 13:56
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.

4 participants