Skip to content

Conversation

@mfoerste4
Copy link
Contributor

@mfoerste4 mfoerste4 commented Feb 28, 2025

This is an optimization raised by #718 .

The quantile selection process currently relies on sampling. The memory requirements for the originally used
raft::random::sample_without_replacement
are roughly 48*input_len bytes (4*len*8byte + ~2*len*8byte for sort).
This has now been replaced by raft::matrix::sample_rows which essentially has the same requirement but based on the sample size instead of the original input.

@mfoerste4 mfoerste4 requested a review from a team as a code owner February 28, 2025 12:43
@github-actions github-actions bot added the cpp label Feb 28, 2025
@tfeher tfeher added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Feb 28, 2025
@tfeher tfeher changed the title [OPT] reduce memory consumption of scalar quantizer Reduce memory consumption of scalar quantizer Feb 28, 2025
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Malte, it looks good, just one comment below.

@mfoerste4 mfoerste4 requested a review from tfeher March 4, 2025 19:15
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Malte for the update, LGTM.

@tfeher
Copy link
Contributor

tfeher commented Mar 17, 2025

/merge

@rapids-bot rapids-bot bot merged commit 7f634be into rapidsai:branch-25.04 Mar 17, 2025
64 checks passed
divyegala pushed a commit to divyegala/cuvs that referenced this pull request Mar 21, 2025
This is an optimization raised by rapidsai#718 . 

The quantile selection process currently relies on sampling. The memory requirements for the originally used 
`raft::random::sample_without_replacement`
are roughly `48*input_len bytes` (`4*len*8byte + ~2*len*8byte for sort`). 
This has now been replaced by `raft::matrix::sample_rows` which essentially has the same requirement but based on the sample size instead of the original input.

Authors:
  - Malte Förster (https://github.com/mfoerste4)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)

URL: rapidsai#736
jiangyinzuo pushed a commit to jiangyinzuo/cuvs that referenced this pull request Mar 27, 2025
This is an optimization raised by rapidsai#718 . 

The quantile selection process currently relies on sampling. The memory requirements for the originally used 
`raft::random::sample_without_replacement`
are roughly `48*input_len bytes` (`4*len*8byte + ~2*len*8byte for sort`). 
This has now been replaced by `raft::matrix::sample_rows` which essentially has the same requirement but based on the sample size instead of the original input.

Authors:
  - Malte Förster (https://github.com/mfoerste4)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)

URL: rapidsai#736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cpp improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

Development

Successfully merging this pull request may close these issues.

2 participants