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

Revert "Generic linalg::map (#1329)" #1336

Merged

Conversation

cjnolet
Copy link
Member

@cjnolet cjnolet commented Mar 13, 2023

The change to the public API was siginficant enough here that it's going to require updates on the cuml side which invoke map_k with very different arguments (e.g. mdspan, different order). For now, it's best we revert this commmit to unblock cuml and then we can proceed by keeping the old APIs and deprecating them until we change cuml.

@cjnolet cjnolet added bug Something isn't working non-breaking Non-breaking change labels Mar 13, 2023
@cjnolet cjnolet marked this pull request as ready for review March 13, 2023 23:29
@cjnolet cjnolet requested a review from a team as a code owner March 13, 2023 23:30
@github-actions github-actions bot added the cpp label Mar 13, 2023
@cjnolet
Copy link
Member Author

cjnolet commented Mar 13, 2023

cc @achirkin Github doesn't seem to provide a way to re-open a merged PR. Can you open a new PR and add the original map_k API back into it? That way we can deprecate it in the PR and merge the changes over without breaking cuml at all.

@cjnolet
Copy link
Member Author

cjnolet commented Mar 14, 2023

/merge

@rapids-bot rapids-bot bot merged commit 7e328ff into rapidsai:branch-23.04 Mar 14, 2023
lowener pushed a commit to lowener/raft that referenced this pull request Mar 15, 2023
The change to the public API was siginficant enough here that it's going to require updates on the cuml side which invoke `map_k` with very different arguments (e.g. mdspan, different order). For now, it's best we revert this commmit to unblock cuml and then we can proceed by keeping the old APIs and deprecating them until we change cuml.

Authors:
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Ben Frederickson (https://github.com/benfred)

URL: rapidsai#1336
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 Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants