Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Sort indices before gathering #174

Merged

Conversation

zhuofan1123
Copy link
Contributor

In continuous/chunked host mode, sorting indices before gathering can improve bandwidth by enhancing memory locality.

Copy link

copy-pr-bot bot commented May 27, 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's better to set raw_indices the same type as indices

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, whether we use gather_with_sorted_ids should also be related to embedding data type, not just to embedding dim.

@chuangz0 chuangz0 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels May 28, 2024
Copy link
Contributor

@linhu-nv linhu-nv left a comment

Choose a reason for hiding this comment

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

Seems good to me.

@zhuofan1123 zhuofan1123 force-pushed the gather_with_sorted_indices branch from 36a7456 to 9105ab6 Compare May 28, 2024 10:21
@BradReesWork
Copy link
Member

/okay to test

@zhuofan1123 zhuofan1123 force-pushed the gather_with_sorted_indices branch from 68bd1bd to 2a183f4 Compare May 29, 2024 03:29
@BradReesWork
Copy link
Member

/okay to test

@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 1921080 into rapidsai:branch-24.06 May 29, 2024
48 checks passed
@zhuofan1123 zhuofan1123 deleted the gather_with_sorted_indices branch May 30, 2024 04:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

4 participants