Skip to content

CPU-only build of GMRES solver no longer tries to allocate on GPU#287

Closed
shakedregev wants to merge 1 commit intodevelopfrom
shaked/cpu_only_gmres_fix
Closed

CPU-only build of GMRES solver no longer tries to allocate on GPU#287
shakedregev wants to merge 1 commit intodevelopfrom
shaked/cpu_only_gmres_fix

Conversation

@shakedregev
Copy link
Copy Markdown
Collaborator

@shakedregev shakedregev commented May 28, 2025

Description

Error messages are returned when running GMRES tests suggesting the solver is trying to access GPU memory. These are benign messages as computation is carried out to completion on a CPU device and all GPU calls are ignored, but the code should not be trying to access the the device memory in the CPU mode in the first place.

_Closes #286 _

@pelesh

Proposed changes

My changes enforce strict checks on owning gpu data. Previously this was set by default to true, which is wrong in the case of a CPU only build.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.

  • 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

If this is a relatively large or complex change, kick off the discussion by explaining
why you chose the solution you did and what alternatives you considered, etc.

@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented May 30, 2025

Closing in favor of #292.

@pelesh pelesh closed this May 30, 2025
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.

2 participants