-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 support for llm
in spacy init config
#12820
base: v4
Are you sure you want to change the base?
Changes from 5 commits
97fa11f
564c2b0
4236942
f9dddbb
9cf8ac8
8da042c
0948dc1
a3bf379
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ the docs and the init config command. It encodes various best practices and | |
can help generate the best possible configuration, given a user's requirements. #} | ||
{%- set use_transformer = hardware != "cpu" and transformer_data -%} | ||
{%- set transformer = transformer_data[optimize] if use_transformer else {} -%} | ||
{%- set listener_components = ["tagger", "morphologizer", "parser", "ner", "textcat", "textcat_multilabel", "entity_linker", "span_finder", "spancat", "spancat_singlelabel", "trainable_lemmatizer"] -%} | ||
{%- set listener_components = ["tagger", "morphologizer", "parser", "ner", "textcat", "textcat_multilabel", "entity_linker", "span_finder", "spancat", "spancat_singlelabel", "trainable_lemmatizer", "llm"] -%} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. I vaguely remember that the LLM block was not output properly if "llm" was not in |
||
[paths] | ||
train = null | ||
dev = null | ||
|
@@ -328,6 +328,18 @@ grad_factor = 1.0 | |
{%- endif %} | ||
{%- endif %} | ||
|
||
{% if "llm" in components -%} | ||
[components.llm] | ||
factory = "llm" | ||
|
||
[components.llm.model] | ||
@llm_models = "{{ llm_spec['model'] }}" | ||
|
||
[components.llm.task] | ||
@llm_tasks = "{{ llm_spec['task'] }}" | ||
{% endif -%} | ||
|
||
|
||
{# NON-TRANSFORMER PIPELINE #} | ||
{% else -%} | ||
{% if "tok2vec" in full_pipeline -%} | ||
|
@@ -585,6 +597,17 @@ no_output_layer = false | |
{%- endif %} | ||
{% endif %} | ||
|
||
{% if "llm" in components -%} | ||
[components.llm] | ||
factory = "llm" | ||
|
||
[components.llm.model] | ||
@llm_models = "{{ llm_spec['model'] }}" | ||
|
||
[components.llm.task] | ||
@llm_tasks = "{{ llm_spec['task'] }}" | ||
{% endif -%} | ||
|
||
Comment on lines
+600
to
+610
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think (depending on exactly how this gets designed in the end), it should be possible to only have this block once because it does not depend on the listener setup. |
||
{% for pipe in components %} | ||
{% if pipe not in listener_components %} | ||
{# Other components defined by the user: we just assume they're factories #} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -628,7 +628,6 @@ def test_parse_cli_overrides(): | |
"pipeline", | ||
[ | ||
["tagger", "parser", "ner"], | ||
[], | ||
["ner", "textcat", "sentencizer"], | ||
["morphologizer", "spancat", "entity_linker"], | ||
["spancat_singlelabel", "textcat_multilabel"], | ||
|
@@ -651,6 +650,26 @@ def test_init_config(lang, pipeline, optimize, pretraining): | |
load_model_from_config(config, auto_fill=True) | ||
|
||
|
||
@pytest.mark.parametrize("pipeline", [["llm"]]) | ||
@pytest.mark.parametrize("llm_model", ["noop"]) | ||
@pytest.mark.parametrize("llm_task", ["ner", "sentiment"]) | ||
def test_init_config_llm(pipeline, llm_model, llm_task): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this test should probably be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we should consider installing |
||
config = init_config( | ||
lang="en", | ||
pipeline=pipeline, | ||
llm_model=llm_model, | ||
llm_task=llm_task, | ||
optimize="accuracy", | ||
pretraining=False, | ||
gpu=False, | ||
) | ||
assert isinstance(config, Config) | ||
assert len(config["components"]) == 1 | ||
assert "llm" in config["components"] | ||
|
||
load_model_from_config(config, auto_fill=True) | ||
|
||
|
||
def test_model_recommendations(): | ||
for lang, data in RECOMMENDATIONS.items(): | ||
assert RecommendationSchema(**data) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are 2 versions of a model/task, is this code guaranteed to give you the latest version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't - I just checked - it produces
spacy.NER.v1
instead ofspacy.NER.v2
when given"ner"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it just grabs the first occurence as of now. I left that as TBD since it wasn't clear how we want to go forward.