Skip to content
This repository has been archived by the owner on Sep 24, 2024. It is now read-only.

Added model_max_length as a tok parameter #113

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

aittalam
Copy link
Member

@aittalam aittalam commented Aug 5, 2024

Since 4.40.0, transformers does not get model_max_tokens from the model's family dict but explicitly looks for this parameter in tokenizer_config.json. Not all models provide this, so one possibility is to explicitly allow users to provide this parameter in the config. This PR implements exactly this.

Tested with facebook/bart-large-cnn + pytest unit/integration tests.

Copy link
Contributor

@binaryaaron binaryaaron left a comment

Choose a reason for hiding this comment

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

approving given the internal constraints; double check a few things before landing though. I'd suggest adding an extra test to the unit tests to check for it.

src/lm_buddy/jobs/model_clients.py Show resolved Hide resolved
@aittalam
Copy link
Member Author

aittalam commented Aug 6, 2024

approving given the internal constraints; double check a few things before landing though. I'd suggest adding an extra test to the unit tests to check for it.

Thanks! I have two more PRs in queue:

  • bump of all libs (this will allow us, anmong other things, to run more recent models)
  • adding text generation pipeline + prompts

Will make unit tests for both and add one for this change too (explicitly passing a longer text as input and verifying it does not break with the new setup)

@aittalam aittalam merged commit 79c8e98 into main Aug 6, 2024
3 of 4 checks passed
@aittalam aittalam deleted the davide/summarizer_truncation branch August 6, 2024 13:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants