Skip to content

backend: isolate core backend generation/input updates#65

Open
Parry-Parry wants to merge 24 commits into
mainfrom
backend-core-followup
Open

backend: isolate core backend generation/input updates#65
Parry-Parry wants to merge 24 commits into
mainfrom
backend-core-followup

Conversation

@Parry-Parry

Copy link
Copy Markdown
Collaborator

This PR isolates backend runtime changes from prompt refactoring work. In particular, it allows batch_size to be passed through HF backends and adds message support

What changed
Updated backend core generation plumbing:
Added HuggingFace backend handling for modern input paths
Added vLLM backend handling for chat/message-style inputs:

Comment thread pyterrier_rag/backend/_base.py
Comment thread pyterrier_rag/backend/_base.py Outdated
Comment thread pyterrier_rag/backend/_hf.py Outdated
Comment thread pyterrier_rag/backend/_hf.py Outdated
refactor: tokenizer does not need a setter
refactor: batch_size is only set for subclasses of Backend that need it
refactor: tokenizer does not need a setter
refactor: batch_size is only set for subclasses of Backend that need it
Comment thread pyterrier_rag/backend/_base.py
Comment thread pyterrier_rag/backend/_base.py Outdated
Comment thread pyterrier_rag/backend/_base.py Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I thought we decided not to add batch_size to the base Backend?

(Also, it looks like these changes were pushed to main as well as this branch?)

Comment thread pyterrier_rag/backend/_hf.py Outdated
Comment thread .gitignore Outdated
Comment thread requirements.txt Outdated
Comment thread pyterrier_rag/backend/_hf.py
self.tokenizer.pad_token = self.tokenizer.eos_token
self._model.generation_config.pad_token_id = self.tokenizer.pad_token_id
self._tokenizer = AutoTokenizer.from_pretrained(model_id)
self._model = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

whats the logic going on here - just one line comment please

if not isinstance(inps, list):
raise TypeError("Expected list as input to generate(), found " + str(type(inps)))
if not isinstance(inps[0], str):
raise ValueError(f'{self!r} only supports str inputs to generate')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

so if it allows lists of non strings, can we check that whether it supports is valid?

sliced_sequences.append(sequences[i, prompt_length:])
prompt_len_with_padding = inputs["input_ids"].shape[1]
for seq in sequences:
sliced_sequences.append(seq[prompt_len_with_padding:])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

did you test this change works as expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants