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

Fix Grace-specific issues in CAGRA #527

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

achirkin
Copy link
Contributor

@achirkin achirkin commented Dec 11, 2024

Fix Grace-specific test failures:

  1. Add stream syncs at the places where host-allocated memory may be destructed while still being accessed by GPU to avoid relying on stream-ordered semantics of memory allocations.
  2. A bug in tests: CAGRA index produced by cagra::build is not guaranteed to be owning. The tests assumed otherwise; when the host dataset is passed, and it's accessible on the device (it's the case with Grace), the created index ended up non-owning. The lifetime of the host dataset in the tests was smaller than of the index, which led to invalid host accesses from the device.
  3. A bug in dataset_deserialize.hpp: deserialize_strided function constructed a non-owning strided dataset, because the host data was accessible by the GPU. The current fix is to add a move-semantics overload of make_strided_dataset that always owns the passed data (either via moving the mdarray or by copying the data).

@achirkin achirkin added bug Something isn't working non-breaking Introduces a non-breaking change DO NOT MERGE labels Dec 11, 2024
@achirkin achirkin self-assigned this Dec 11, 2024
@achirkin achirkin requested a review from a team as a code owner December 11, 2024 10:07
Copy link

copy-pr-bot bot commented Dec 11, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cjnolet
Copy link
Member

cjnolet commented Dec 11, 2024

/merge

@rapids-bot rapids-bot bot merged commit ef16a9e into rapidsai:branch-25.02 Dec 11, 2024
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cpp non-breaking Introduces a non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants