Skip to content
This repository has been archived by the owner on May 19, 2023. It is now read-only.

[REVIEW] Tokenizer with rmm integration #155

Merged
merged 71 commits into from
Jun 12, 2020
Merged

[REVIEW] Tokenizer with rmm integration #155

merged 71 commits into from
Jun 12, 2020

Conversation

brhodes10
Copy link
Contributor

@brhodes10 brhodes10 commented May 15, 2020 •
edited
Loading

  • Updated tokenizer to use rmm
  • Added tokenizer benchmark and unit tests

Bianca Rhodes added 30 commits April 28, 2020 21:35
Updated cmake file. Updated basicTokenizer to use rmm
73a14ad
Revert basicTokenizer changes
74a36bd
Updated device token ids to rmm device_vector
da89fea
code cleanup
304dbdc
Update cmake file
6bebdf5
Update device_word_indices to use rmm device vector
9b86290
Converted device_tokens_per_word to rmm device_vector
4f54001
Updated tensor_tokenIDS to rmm device_vector
fd26d1c
Update attention_mask to rmm device_vector
d3d3174
Update metadata to rmm device_vector
a395674
Update device_row2log to rmm device_vector
c2c01f6
Updated device_row2row_within_log to rmm device_vector
87a72f5
Updated copy_vec_to_device function param to rmm device vector
cab62e3
Update device_sentence_offsets to rmm device_vector
2de9740
Add device synchronize
d771ab1
Revert "Update device_sentence_offsets to rmm device_vector"
d7d81e3 
This reverts commit 2de9740.
Revert "Updated copy_vec_to_device function param to rmm device vector"
c9c42a4 
This reverts commit cab62e3.
Update device_sentence_offsets to rmm device_vector
8e67c8a
Update device_sentences and device_sentence_offsets to rmm device vector
610c8a4
Updated device_chars_per_thread to rmm device vector
38b7721
Update device_code_points to rmm device_vector
f5b7d02
Update device_num_selected to rmm device vector
b0f8e5b
Updated device_cp_metadata to rmm device_vector
b51f0b7
Update device_aux_data to rmm device_vector
e4e1a4d
Update device_hash_table to rmm device_vector
175e3b6
Update device_bin_coefficients to rmm device_vector
43e4155
Update device_bin_offsets to rmm device_vector
38ebaa0
Update device_num_selected to rmm device_vector
a628753
Update cub_temp_storage to rmm device_vector
e4f3a3c
Updated cub_temp_storage to rmm device_vector
64460a0
brhodes10 and others added 5 commits May 27, 2020 16:18
@brhodes10 @harrism
Update cpp/src/basicTokenizer.cu
2f99323 
update to use thrust exclusive scan

Co-authored-by: Mark Harris <mharris@nvidia.com>
@brhodes10 @harrism
Update cpp/src/basicTokenizer.cu
457805f 
update to use thrust exclusive scan

Co-authored-by: Mark Harris <mharris@nvidia.com>
Removed thrust exclusive scan. Revert "Update cpp/src/basicTokenizer.…
18fb379 
…cu
"

This reverts commit 2f99323.
Removed transfer_cp_data_to_device function
02792f1
Update device_row2log and device_row2row_within_log
b669d81
@brhodes10
Copy link
Contributor Author

I just reviewed some of BasicTokenizer for now. Suggest making those changes and corresponding changes to the other tokenizers, and then I can re-review. This should simplify the changes a little.

Hi @harrism I have made changes based on your comments. I saw areas where I could apply your suggestions to the fullTokenizer as well. Let me know if you have any more comments. Thank you

@brhodes10 brhodes10 closed this May 28, 2020
@brhodes10 brhodes10 reopened this May 28, 2020
@brhodes10 brhodes10 changed the base branch from branch-0.14 to branch-0.15 June 1, 2020 14:13
harrism
harrism reviewed Jun 10, 2020
View reviewed changes
cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@harrism
Copy link
Member

harrism commented Jun 10, 2020

I think something is wrong here? 124000 additions and deletions? Those files can't be that long, can they?

image

@BartleyR
Copy link
Member

I think something is wrong here? 124000 additions and deletions? Those files can't be that long, can they?

I believe there are a few vectors in those files that we use as input to the basic tokenizer. They are large, and that's what is likely causing the large diffs.

@brhodes10
Copy link
Contributor Author

rerun tests

2 similar comments
@efajardo-nv
Copy link
Contributor

rerun tests

@brhodes10
Copy link
Contributor Author

rerun tests

@brhodes10 @harrism
Update cpp/CMakeLists.txt
83dd0aa 
Co-authored-by: Mark Harris <mharris@nvidia.com>
@efajardo-nv
Copy link
Contributor

rerun tests

@brhodes10 brhodes10 merged commit d72b813 into rapidsai:branch-0.15 Jun 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kkraus14 kkraus14 kkraus14 left review comments

@harrism harrism harrism approved these changes

@raydouglass raydouglass raydouglass approved these changes

@BartleyR BartleyR BartleyR approved these changes

@jrhemstad jrhemstad Awaiting requested review from jrhemstad

Assignees
No one assigned
Labels
3 - Ready for Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@brhodes10 @GPUtester @BartleyR @efajardo-nv @harrism @raydouglass @kkraus14