Skip to content

Prohibit sloppy syncing#284

Merged
shakedregev merged 8 commits intodevelopfrom
slaven/prohibit_sloppy_syncing_dev
May 27, 2025
Merged

Prohibit sloppy syncing#284
shakedregev merged 8 commits intodevelopfrom
slaven/prohibit_sloppy_syncing_dev

Conversation

@pelesh
Copy link
Copy Markdown
Collaborator

@pelesh pelesh commented May 25, 2025

Description

Prohibit synchronizing memory space if it is already updated. Only memory space that is out of date can be synchronized with the updated space. For example, if host is up to date and device is out of date, then function syncData(DEVICE); will copy data from the host to the device. If the device is up-to-date (i.e. flag gpu_updated_ == true), Re::Solve will return an error. The user is required to keep track of which memory space is current.

Current behavior in Re::Solve is to do nothing if user tries to sync the memory space that is up to date. This provides a convenience for users -- if in doubt whether memory space has been synced, just sync again. This sloppy approach makes certain bugs difficult to track. For example, consider a case where cpu_updated_ == false and gpu_updated_ == true. If user changes vector elements on the host by accessing its raw data and forgets to to mark the host memory as updated, the device memory flag will still show that memory space is the current one. Then, if trying to sync the device with host, function syncData will quietly do nothing. By prohibiting sloppy syncing, this function will return an error alerting user something is wrong.

Closes #282

Proposed changes

  • Remove all instances of sloppy syncing inthe code.
  • Function syncData returns an error when trying to sync the memory space which is up-to-date.

Checklist

  • All tests pass. 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.

Further comments

@pelesh pelesh requested review from kswirydo and shakedregev May 25, 2025 22:55
@pelesh pelesh self-assigned this May 25, 2025
@pelesh pelesh added enhancement New feature or request question Further information is requested labels May 25, 2025
Copy link
Copy Markdown
Collaborator

@shakedregev shakedregev left a comment

Choose a reason for hiding this comment

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

This looks good. I suggest removing "Function call ignored", because this implies that the code otherwise executes normally. Which it doesn't because it errors.

Comment thread resolve/matrix/Coo.cpp Outdated
Comment thread resolve/matrix/Coo.cpp Outdated
Comment thread resolve/matrix/Csc.cpp Outdated
Comment thread resolve/matrix/Csc.cpp Outdated
Comment thread resolve/matrix/Csr.cpp Outdated
Comment thread resolve/matrix/Csr.cpp Outdated
@shakedregev shakedregev merged commit f81e94d into develop May 27, 2025
4 checks passed
@pelesh pelesh deleted the slaven/prohibit_sloppy_syncing_dev branch July 24, 2025 20:50
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.

Prevent user from synching to host if device has not been set to updated.

2 participants