Skip to content

Make Vector::copyDataTo able to copy from device to host and vice versa#416

Merged
pelesh merged 13 commits intodevelopfrom
shaked/copyData
Mar 3, 2026
Merged

Make Vector::copyDataTo able to copy from device to host and vice versa#416
pelesh merged 13 commits intodevelopfrom
shaked/copyData

Conversation

@shakedregev
Copy link
Collaborator

@shakedregev shakedregev commented Feb 27, 2026

Description

Current implementation of Vector::copyDataTo methods allows for copying from host-to-host and device-to-device only. If one needs to copy from host to device one needs an additional copy operation. Need to modify Vector::copyDataTo so it can copy data from any memory space to any memory space, similar as Vector::copyDataFrom methods.

Proposed changes

My changes allow the source and memory space to be different. I also addressed naming issues and syncing issues.

Checklist

  • All tests pass (make test and make test_install per testing instructions). Code tested on
    • CPU backend
    • CUDA backend
    • HIP backend
  • I have manually run the non-experimental examples and verified that residuals are close to machine precision. (In your build directory run:
    ./examples/<your_example>.exe -h to get instructions how to run examples). Code tested on:
    • CPU backend
    • CUDA backend
    • HIP backend
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows Re::Solve style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

I noticed some issues that I left to not explode the scope of the PR. We should follow up on these.

  1. copyDataFrom has similar naming and potentially syncing issues
  2. GramSchmidt is littered with deviceSynchronize() seemingly arbitrarily
  3. Vector tests have some commented tests.

@shakedregev shakedregev requested a review from pelesh February 27, 2026 18:42
@shakedregev shakedregev self-assigned this Feb 27, 2026
@alexander-novo
Copy link
Collaborator

with the recent change to getData being const is it possible to mark these functions as const?

Copy link
Collaborator

@pelesh pelesh left a comment

Choose a reason for hiding this comment

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

I would suggest renaming copyDataFrom --> copyFromExternal in this PR. Otherwise we leave vector methods names in "asymmetric" state.

Other than that, this looks good!

@pelesh
Copy link
Collaborator

pelesh commented Mar 3, 2026

with the recent change to getData being const is it possible to mark these functions as const?

It is, but would require cleaning up the code where the sloppy data movement was used.

@shakedregev shakedregev requested a review from pelesh March 3, 2026 17:44
@shakedregev
Copy link
Collaborator Author

Thanks for the feedback. Fixed. Please re-review.

shakedregev and others added 2 commits March 3, 2026 12:46
@pelesh pelesh added this to the Release 0.99.3 milestone Mar 3, 2026
@pelesh pelesh added the enhancement New feature or request label Mar 3, 2026
@pelesh pelesh merged commit a67aeef into develop Mar 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants