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.

Sign up for GitHub

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

Jump to bottom

feat: TRTLLM API handle tokenizers without pad_id (e.g., tiktoken) #399

Merged
merged 6 commits into from
Nov 20, 2024
Merged

feat: TRTLLM API handle tokenizers without pad_id (e.g., tiktoken) #399

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
79630dd
feat: TRTLLM API handle tokenizers without pad_id (e.g., tiktoken)
terrykong Nov 12, 2024
72ad72b
ci: adds cherry-pick to enable GPT TRTLLM version with bias=False
terrykong Nov 12, 2024
3dd6eb5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 12, 2024
83f1c0f
go with pad_id=-42 all the time
terrykong Nov 13, 2024
bd9fa0b
revert the dockerfile change
terrykong Nov 15, 2024
80f7960
Merge branch 'main' into tk/trtllm-next
terrykong Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions nemo_aligner/utils/trt_llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ def append_and_repad_list(list_of_items, item_to_append, pad_id):


class GPTGenerateTRTLLM:
# If a tokenizer does not have a pad_id, we use a large negative number and replace
# with self.eos_id after generation.
# Use a reserved negative number since there is variation between tokenizers if
# they (1) have a pad_id (2) don't have a pad_id or (3) have None as the pad_id.
# This pad_id is replaced with eos_id after generation.
DEFAULT_PAD_ID = -42

def __init__(
Expand All @@ -72,12 +73,6 @@ def __init__(
"You are trying to use NeMo-Aligner's TensorRT-LLM acceleration for LLM generation. Please build the dockerfile to enable this feature: https://github.com/NVIDIA/NeMo-Aligner/blob/main/Dockerfile"
)

# If this assert turns out to be a blocker with some tokenizers, potential workarounds could be to:
# - add a config option to allow specifying which token we pass as `end_id` to TRT-LLM (should
# be a token that the model is guaranteed to never generate)
assert (
tokenizer.pad_id != tokenizer.eos_id
), f"We require tokenizers to have a different {tokenizer.pad_id=} than {tokenizer.eos_id=} when using TRT-LLM. This is to make sure all code goes into the same path and include the eos_id when the response lengths are computed"
assert max_input_len > 0
assert max_generation_length > 0
assert (
Expand All @@ -104,7 +99,7 @@ def __init__(
rng_generator.manual_seed(seed)
self.rng_generator = rng_generator

self.pad_id = tokenizer.pad_id if tokenizer.pad_id is not None else GPTGenerateTRTLLM.DEFAULT_PAD_ID
self.pad_id = GPTGenerateTRTLLM.DEFAULT_PAD_ID
self.eos_id = tokenizer.eos_id
end_strings = list(end_strings)

Expand Down
Loading