Skip to content

Track updated status for each vector in multivector (third attempt) #314

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

Merged
merged 13 commits into from
Jun 16, 2025

Conversation

pelesh
Copy link
Collaborator

@pelesh pelesh commented Jun 14, 2025

Description

Vector class object is a multivector that allows access to individual vectors separately. It is plausible to expect that individual vectors could be updated individually and some of them may have device, while others may have host memory space updated. However, current implementation of the Vector class allows only for setting update flags for all vectors together, regardless of how they are accessed and updated. This is a bug, see #259.

@superwhiskers

Proposed changes

This PR provides following:

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

This PR supersede #278.

@pelesh pelesh added this to the Release 0.99.2 milestone Jun 14, 2025
@pelesh pelesh requested review from kswirydo and shakedregev June 14, 2025 19:15
@pelesh pelesh self-assigned this Jun 14, 2025
@pelesh pelesh added bug Something isn't working hip cuda labels Jun 14, 2025
Copy link
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.

Looks good. I made some minor semantic comments. Tests pass.

@pelesh pelesh merged commit 4a7c984 into develop Jun 16, 2025
4 checks passed
Copy link
Collaborator

@kswirydo kswirydo left a comment

Choose a reason for hiding this comment

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

See comments

@@ -33,33 +34,37 @@ namespace ReSolve { namespace vector {
int copyDataFrom(const real_type* data, memory::MemorySpace memspaceIn, memory::MemorySpace memspaceOut);
int copyDataFrom(Vector* v, memory::MemorySpace memspaceIn, memory::MemorySpace memspaceOut);
real_type* getData(memory::MemorySpace memspace);
real_type* getData(index_type i, memory::MemorySpace memspace); // get pointer to i-th vector in multivector
real_type* getData(index_type i, memory::MemorySpace memspace);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what was wrong with this comment :D

@@ -234,7 +234,7 @@ namespace ReSolve {
using namespace constants;

if (k < 200) {
hip::mass_inner_product_two_vectors(size, k, x->getData(memory::DEVICE) , x->getData(1, memory::DEVICE), V->getData(memory::DEVICE), res->getData(memory::DEVICE));
hip::mass_inner_product_two_vectors(size, k, x->getData(0, memory::DEVICE) , x->getData(1, memory::DEVICE), V->getData(memory::DEVICE), res->getData(memory::DEVICE));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code formatting is totally wrong here.

@pelesh pelesh deleted the slaven/vector_updated_fix 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
bug Something isn't working cuda hip
Projects
None yet
3 participants