refactor: move tokenizer_data_utils with the rest of utils, add further unit testing. #348
Conversation
Signed-off-by: Will Johnson <[email protected]>
… with special tokens, resize multiple of. fmt Signed-off-by: Will Johnson <[email protected]>
willmj
requested review from
anhuong,
Ssukriti and
alex-jw-brooks
as code owners
|
Thanks for making a pull request! 😃 |
|
Can you resolve the conflicts? I think it might need some code changes |
Signed-off-by: Will Johnson <[email protected]>
Signed-off-by: Will Johnson <[email protected]>
|
|
||
| output_tokenizer_len = len(tokenizer.get_vocab()) | ||
|
|
||
| assert output_tokenizer_len == input_tokenizer_len + 2 |
There was a problem hiding this comment.
Because the new return of of tokenizer_and_embedding_resize, I think we can test it even more thorough here with the returned value which is {"num_new_tokens": num_new_tokens, "new_embedding_size": embedding_size}.
We can assign an output above:
resize_results = tokenizer_and_embedding_resize(
special_tokens_dict=special_tokens,
tokenizer=tokenizer,
model=model,
multiple_of=1,
)
and then here, we can do in addition of checking that it's 2:
assert output_tokenizer_len - input_tokenizer_len == resize_results["num_new_tokens"]
|
|
||
| output_tokenizer_len = len(tokenizer.get_vocab()) | ||
|
|
||
| assert input_tokenizer_len == output_tokenizer_len |
There was a problem hiding this comment.
in addition, we can do another assert here resize_result[num_new_tokens] == 0 as well
| special_tokens_dict={}, tokenizer=tokenizer, model=model, multiple_of=8 | ||
| ) | ||
|
|
||
| assert model.get_input_embeddings().embedding_dim % 8 == 0 |
There was a problem hiding this comment.
do we need to make sure that the input embeddings has multiple of 8?
There was a problem hiding this comment.
Yes, we do. Pls ignore my silly question
| ) | ||
|
|
||
| assert model.get_input_embeddings().embedding_dim % 8 == 0 | ||
| assert model.get_output_embeddings().out_features % 8 == 0 |
There was a problem hiding this comment.
this is interesting. Is out_features the same as embedding size?
We can test here as well that resize_result["new_embedding_size"] %8 ==0 tho
Signed-off-by: Will Johnson <[email protected]>
Signed-off-by: Will Johnson <[email protected]>
Signed-off-by: Will Johnson <[email protected]>
Description of the change
Move
tokenizer_data_utils.pyfrom/datato/utilswith the rest of the utils.Update imports so function calls change from
tokenizer_data_utils.tokenizer_and_embedding_resizetotokenizer_and_embedding_resize.Add 3 unit tests:
How to verify the PR
tox -e pyWas the PR tested