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

Add patch reverting CCCL PR 211. #511

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Dec 19, 2023

Description

Something is going wrong in thrust::copy, so I am adding a patch to revert NVIDIA/cccl#211. This will fix the failing tests for rapidsai/cuspatial#1315.

This is where it's failing, with thrust::copy:
https://github.com/rapidsai/cuspatial/blob/8e8fa0689339c0288746b01a8af66fb599f5c770/cpp/tests/range/multipolygon_range_test.cu#L800

CI log: https://github.com/rapidsai/cuspatial/actions/runs/7229405991/job/19700477218?pr=1315#step:7:3280

[ RUN      ] MultipolygonRangeOneTest/1.OneMultipolygonRange
unknown file: Failure
C++ exception with description "__copy:: D->D: failed: cudaErrorInvalidValue: invalid argument" thrown in the test body.

[  FAILED  ] MultipolygonRangeOneTest/1.OneMultipolygonRange, where TypeParam = double (0 ms)

I traced it down to this CCCL PR - note that thrust::transform(..., thrust::identity{}) works, but the new thrust::copy code path in NVIDIA/cccl#211 that calls cudaMemcpyAsync doesn't work for some reason. I was not able to identify the exact reason why that PR causes the failure. Patching this out will unblock RAPIDS and let us migrate forward with CCCL 2.2.0.

This helps with #502.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@bdice bdice self-assigned this Dec 19, 2023
@bdice bdice added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Dec 19, 2023
@bdice bdice changed the title Add patch for reverting CCCL PR 211. Add patch reverting CCCL PR 211. Dec 19, 2023
@bdice
Copy link
Contributor Author

bdice commented Dec 19, 2023

/merge

@rapids-bot rapids-bot bot merged commit ef3388f into rapidsai:branch-24.02 Dec 19, 2023
16 checks passed
@bdice bdice mentioned this pull request Dec 19, 2023
26 tasks
@bdice
Copy link
Contributor Author

bdice commented Jul 7, 2024

rapidsai/cuspatial#1402 will fix a bug that was being masked by this CCCL patch. I will open a PR to remove this patch.

rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this pull request Jul 9, 2024
This PR fixes a bug in the multipolygon geometry iterator. The geometry iterator was returning part iterators by mistake, which masked an issue that we were patching out in rapids-cmake's CCCL. See rapidsai/rapids-cmake#511.

With this fix, we can remove that patch from rapids-cmake's CCCL: rapidsai/rapids-cmake#640

Authors:
  - Bradley Dice (https://github.com/bdice)
  - Paul Taylor (https://github.com/trxcllnt)

Approvers:
  - Michael Schellenberger Costa (https://github.com/miscco)
  - Mark Harris (https://github.com/harrism)
  - Michael Wang (https://github.com/isVoid)
  - Paul Taylor (https://github.com/trxcllnt)

URL: #1402
rapids-bot bot pushed a commit to rapidsai/cudf that referenced this pull request Jul 9, 2024
While upgrading CCCL, we ran into a test failure in cuSpatial. We added a patch to revert some changes from CCCL but the root cause was a bug in cuSpatial. I have fixed that bug here: rapidsai/cuspatial#1402

Once that PR is merged, we can remove this CCCL patch.

See also:
- rapids-cmake patch removal: rapidsai/rapids-cmake#640
- Original rapids-cmake patch: rapidsai/rapids-cmake#511
- CCCL epic to remove RAPIDS patches: NVIDIA/cccl#1939

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)

URL: #16207
rapids-bot bot pushed a commit that referenced this pull request Jul 9, 2024
While upgrading CCCL, we ran into a test failure in cuSpatial. We added a patch to revert some changes from CCCL but the root cause was a bug in cuSpatial. I have fixed that bug here: rapidsai/cuspatial#1402

Once that PR is merged, we can remove this CCCL patch.

See also:
- #511
- NVIDIA/cccl#1939

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)

URL: #640
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants