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.

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

Add init fill-config-transformer CLI command #16

Merged
merged 44 commits into from
Aug 30, 2023

Conversation

shadeMe
Copy link
Collaborator

@shadeMe shadeMe commented Aug 7, 2023

Description

This command reads the Hugging Face model name and revision from the initialize.components.transformer.encoder_loader config section, fetches its config from the HF Model Hub and fills in the entry point parameters for the same.

Types of change

New feature

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

This command reads the Hugging Face model name and revision from the `initialize.components.transformer.encoder_loader` config section, fetches its config and fills in the entry point parameters for the same.
@shadeMe shadeMe added the enhancement New feature or request label Aug 7, 2023
Copy link
Contributor

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to merge if @adrianeboyd is also ok with it.

@adrianeboyd
Copy link
Contributor

I could run the CLI command for the prefilled config snippets, but there's still no way for users to get to an initial config:

nlp.add_pipe("curated_transformer")

or

spacy init config -p curated_transformer

both fail with:

Config validation error
curated_transformer.model -> vocab_size	field required
{'@architectures': 'spacy-curated-transformers.XlmrTransformer.v1', 'piece_encoder': {'@architectures': 'spacy-curated-transformers.XlmrSentencepieceEncoder.v1'}, 'with_spans': {'@architectures': 'spacy-curated-transformers.WithStridedSpans.v1'}}

@shadeMe
Copy link
Collaborator Author

shadeMe commented Aug 10, 2023

I could run the CLI command for the prefilled config snippets, but there's still no way for users to get to an initial config:

Fixed in #20.

@adrianeboyd
Copy link
Contributor

Can you tell the user based on the downloaded model which kind of architecture they probably need? (Like, I was even confused by the naming of something that looks like bert but wasn't.)

@shadeMe
Copy link
Collaborator Author

shadeMe commented Aug 14, 2023

I've expanded the error message to display either the expected HF model type for the given curated transformers architecture (in case the type of the HF model is not supported) or the correct curated transformers architecture (if the HF model type is supported).

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

(still reviewing - first few comments)

@svlandeg
Copy link
Member

I don't like the way that fill doesn't mean quite the same thing as in fill-config, but the alternatives are probably worse.

I was actually thinking the same earlier. What about update instead of fill? 🤔

@shadeMe
Copy link
Collaborator Author

shadeMe commented Aug 28, 2023

I was actually thinking the same earlier. What about update instead of fill? 🤔

I'd still keep fill as the grand majority of the fields being added are actually missing in the config, i.e., there are no keys for them (as opposed to updating the values of existing keys).

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Looks great!

@svlandeg svlandeg merged commit 159a4e9 into explosion:main Aug 30, 2023
7 checks passed
@shadeMe shadeMe deleted the feature/config-fill-cli-command branch August 30, 2023 17:34
adrianeboyd added a commit that referenced this pull request Nov 28, 2023
* Add `init fill-config-transformer` CLI command

This command reads the Hugging Face model name and revision from the `initialize.components.transformer.encoder_loader` config section, fetches its config and fills in the entry point parameters for the same.

* Feature-gate tests

* Lazy import `huggingface_hub`

* Rethrow exception when `CliRunner` fails

* Fix type

* Revert slow marker

* Print error when HF tokenizer loading fails

* Tick `transformers` version

* Install `sentencepiece` in CI

* Install `sentencepiece` as a `transformers` extra dependency

* Temporarily rethrow exception to debug CI

* Revert "Temporarily rethrow exception to debug CI"

This reverts commit 59b78d9.

* Fix website link in docstring

* Set default output path to `stdout`

* Update command arg helpstring

* Replace `IntEnum` with `Enum`

* Apply suggestions from code review

Co-authored-by: Adriane Boyd <[email protected]>

* Fix typo

* Set `model_max_length` to `sys.maxsize`

* Automatically fill in piece encoder loader

* Assert expected outputs in unit tests

* Add args to pass model name/revision via CLI

This overrides the name/revision in the config (if present).

* Apply suggestions from code review

Co-authored-by: Adriane Boyd <[email protected]>

* Use `main` as the default CLI revision arg

* Mention the model name/revision CLI args in error message

* Use `int32.max` as the sentinel value for `model_max_length`

* Add back `model_max_length` to unit tests

* Add clarification to the mismatching architectures error message

* Rename command to `fill-curated-transformer`

* Update tests

* Clarify the mismatching model type/architecture error message further

* `isort`

* Shorten docstring for display in CLI

* Sneaky readMe fix

* Apply suggestions from code review

Co-authored-by: Sofie Van Landeghem <[email protected]>

* Shorten docstring further

* Remove duplicate key

* Pretty-print errors when fetching model config from HF Hub

* Restructure error handling when validating model type/arch

* Add example model names for supported architectures

* Sort fetchd parameter list

---------

Co-authored-by: Adriane Boyd <[email protected]>
Co-authored-by: Sofie Van Landeghem <[email protected]>
adrianeboyd added a commit that referenced this pull request Nov 28, 2023
* Add `init fill-config-transformer` CLI command

This command reads the Hugging Face model name and revision from the `initialize.components.transformer.encoder_loader` config section, fetches its config and fills in the entry point parameters for the same.

* Feature-gate tests

* Lazy import `huggingface_hub`

* Rethrow exception when `CliRunner` fails

* Fix type

* Revert slow marker

* Print error when HF tokenizer loading fails

* Tick `transformers` version

* Install `sentencepiece` in CI

* Install `sentencepiece` as a `transformers` extra dependency

* Temporarily rethrow exception to debug CI

* Revert "Temporarily rethrow exception to debug CI"

This reverts commit 59b78d9.

* Fix website link in docstring

* Set default output path to `stdout`

* Update command arg helpstring

* Replace `IntEnum` with `Enum`

* Apply suggestions from code review

Co-authored-by: Adriane Boyd <[email protected]>

* Fix typo

* Set `model_max_length` to `sys.maxsize`

* Automatically fill in piece encoder loader

* Assert expected outputs in unit tests

* Add args to pass model name/revision via CLI

This overrides the name/revision in the config (if present).

* Apply suggestions from code review

Co-authored-by: Adriane Boyd <[email protected]>

* Use `main` as the default CLI revision arg

* Mention the model name/revision CLI args in error message

* Use `int32.max` as the sentinel value for `model_max_length`

* Add back `model_max_length` to unit tests

* Add clarification to the mismatching architectures error message

* Rename command to `fill-curated-transformer`

* Update tests

* Clarify the mismatching model type/architecture error message further

* `isort`

* Shorten docstring for display in CLI

* Sneaky readMe fix

* Apply suggestions from code review

Co-authored-by: Sofie Van Landeghem <[email protected]>

* Shorten docstring further

* Remove duplicate key

* Pretty-print errors when fetching model config from HF Hub

* Restructure error handling when validating model type/arch

* Add example model names for supported architectures

* Sort fetchd parameter list

---------

Co-authored-by: Adriane Boyd <[email protected]>
Co-authored-by: Sofie Van Landeghem <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants