Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use explicit host-device syncing #200

Merged
merged 4 commits into from
Oct 12, 2024
Merged

Conversation

pelesh
Copy link
Collaborator

@pelesh pelesh commented Oct 10, 2024

Instead of having matrix getters (e.g. Csr::getValues()) sync host-device data under the hood, perhaps we should require explicit sync calls. So instead of

  // allocate and initialize matrix A on host
  double* a = A.getValues(memory::DEVICE); // getValues copies values from host to device

one would do

  // allocate and initialize matrix A on host
  A.syncData(memory::DEVICE); // explicitly copy matrix data  from host to device
  double* a = A.getData(memory::DEVICE);

Here are a few arguments why this might be a good idea:

  • The code is more readable, the intent to perform operation in specific memory space is made explicit.
  • Current implementation silently ignores syncing if the source memory space is not up-to-date, leaving user under impression that the syncing was successful.
  • Re::Solve's solvers are really not using implicit syncing. It is only tests and examples that used this features.
  • With getters not modifying matrix class by syncing host and device memory, we can implement const getters with identical behavior. That could help address Consider using const in ReSolve code more aggressively #89 more effectively.

@pelesh pelesh added enhancement New feature or request question Further information is requested labels Oct 10, 2024
@pelesh pelesh self-assigned this Oct 10, 2024
@pelesh pelesh merged commit 46155fc into copy2sync-dev Oct 12, 2024
4 checks passed
pelesh added a commit that referenced this pull request Oct 12, 2024
* Matrix getters don't sync.

* Use explicit host-device syncing in tests.
pelesh added a commit that referenced this pull request Oct 12, 2024
…cing (#199)

* Wholesale name change copyData -> syncData

* Set checks in Vector::syncData function

* Only one argument is sufficient in Vector::syncData

* Document matrix::*::syncData methods

* Use explicit host-device syncing (#200)

* Matrix getters don't sync.

* Use explicit host-device syncing in tests.

* Update examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants