Swappable parsing #242

rmitsch · 2023-08-03T16:40:41Z

Description

Make parsing a swappable, configurable attribute to allow modify parsing behaviour without retiring the entire task class.

Corresponding documentation PR

todo

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 all tests in tests and usage_examples/tests, and all new and existing tests passed. This includes
- all external tests (i. e. pytest ran with --external)
- all tests requiring a GPU (i. e. pytest ran with --gpu)
My changes don't require a change to the documentation, or if they do, I've added all required information.

spacy_llm/tasks/lemma.py

spacy_llm/ty.py

rmitsch · 2023-08-04T14:14:27Z

It might make sense to move each task into their own module and split the actual task implementation from the factory methods and utils (such as the parsing methods). Over time this will become unwieldy otherwise.

rmitsch · 2023-08-04T14:27:53Z

Making parsing swappable is a bit more complicated for tasks inheriting from SpanTask, as there other class methods are invoked. There isn't a strong reason for that though, this setup works fine with moving all parsing-related functions out of the class and registering them (which is what this PR aims for, ultimately).

svlandeg

This looks like a promising approach to me to allow more flexibility both for users, as well as for our own builtin functionality.

As discussed internally, we'd also need to make the _create_prompt_example functionality swappable.

svlandeg · 2023-08-09T09:18:04Z

It might make sense to move each task into their own module and split the actual task implementation from the factory methods and utils (such as the parsing methods). Over time this will become unwieldy otherwise.

We can definitely do that, yep.

svlandeg

I like this direction! Leaving some first comments.

spacy_llm/tasks/lemma/registry.py

spacy_llm/tasks/lemma/task.py

* Refactor tasks. * Update spacy_llm/tasks/builtin_task.py Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com> * Update spacy_llm/tasks/builtin_task.py Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com> * Update spacy_llm/tasks/builtin_task.py Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com> * Fix imports. * Make scorer configurable. --------- Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

# Conflicts: # spacy_llm/tasks/rel.py

…lm into feat/swappable-parsing

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

spacy_llm/ty.py

rmitsch · 2023-08-16T08:41:06Z

Ready for a final review round. We should decide whether we want to keep the file structure for tasks as-is - we could also merge parser.py, example.py, scorer.py into a single file?

rmitsch · 2023-08-16T15:03:11Z

scorer currently runs on **kwargs as well - we should update that to a generic protocol in a follow-up PR. Added an internal ticket.

svlandeg

This is looking good! Left a few final comments & fixes, nothing major.
Feel free to merge afterwards so we can move on to the CoT to put this in practice 🎉

spacy_llm/tasks/builtin_task.py

spacy_llm/tasks/summarization/parser.py

spacy_llm/tasks/textcat/parser.py

spacy_llm/ty.py

spacy_llm/tasks/builtin_task.py

svlandeg · 2023-08-17T10:18:32Z

And maybe to be sure, run the GPU & external tests at the end before merging.

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

svlandeg

🎉

rmitsch added 2 commits August 3, 2023 18:36

Demo swappable parser.

d7143d7

Update response parser usage.

02e6d9d

rmitsch added feat/new New feature feat/task Feature: tasks labels Aug 3, 2023

rmitsch self-assigned this Aug 3, 2023

rmitsch changed the base branch from main to develop August 3, 2023 16:41

Adjust naming.

01996e5

rmitsch commented Aug 3, 2023

View reviewed changes

spacy_llm/tasks/lemma.py Outdated Show resolved Hide resolved

kabirkhan reviewed Aug 3, 2023

View reviewed changes

spacy_llm/ty.py Outdated Show resolved Hide resolved

rmitsch added 2 commits August 4, 2023 09:14

Add parse_responses in Lemma factory function.

4020c88

Move default parse function out of task class.

59404f5

Move parse default setting.

0b47777

svlandeg reviewed Aug 9, 2023

View reviewed changes

rmitsch added 4 commits August 9, 2023 13:03

Refactor lemma task to use new file structure.

27bf584

Adjusted lemma task to make fewshot example type configurable.

ea6657c

Remove superfluous fewshot example type definition.

0a33bd8

Fix wrong Type import.

35ba265

svlandeg reviewed Aug 9, 2023

View reviewed changes

rmitsch added 9 commits August 9, 2023 15:01

Try to fix Pydantic issue.

7e061d9

Try to fix Pydantic issue.

4f8836b

Fix docstring. Remove redundant factory function for fewshot examples.

047a496

Split util.py into examples.py and parser.py.

bd416ba

Ignore Pydantic deprecation warning.

ee0f2e7

Revert ignoring Pydantic deprecation warning.

40760cc

Ignore Pydantic deprecation warning.

a206dd4

Ignore Pydantic deprecation warning.

4fc7372

Ignore Pydantic deprecation warning.

a16291e

rmitsch and others added 6 commits August 14, 2023 20:27

Update spacy_llm/tasks/ner/registry.py

5f89aee

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

Merge branch 'develop' into feat/swappable-parsing

f283d1b

# Conflicts: # spacy_llm/tasks/rel.py

Merge branch 'feat/swappable-parsing' of github.com:explosion/spacy-l…

2ef223d

…lm into feat/swappable-parsing

Update spacy_llm/tasks/ner/registry.py

fb06de3

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

Update spacy_llm/tasks/ner/registry.py

c8ee33f

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

kabirkhan reviewed Aug 15, 2023

View reviewed changes

spacy_llm/ty.py Outdated Show resolved Hide resolved

Kabir Khan and others added 3 commits August 16, 2023 09:18

make response parser a generic protocol and pass the task (#264)

5731f66

Pass task instead of varying arguments.

1034883

Fix docstrings.

a3c6b72

rmitsch added 3 commits August 16, 2023 11:50

Use fewshot_examples instead of examples.

b443670

Revert from fewshot_examples to examples in task factory functions.

3428497

Rename fewshot_examples to prompt_examples in tasks.

db1f789

rmitsch added 3 commits August 17, 2023 10:50

Fused scorer.py and examples.py into util.py. Renamed scorer protocols.

aee0122

Fix circular import.

d158e65

Remove 'Protocol' suffix.

891f13b

svlandeg reviewed Aug 17, 2023

View reviewed changes

rmitsch and others added 3 commits August 17, 2023 12:31

Apply suggestions from code review

b6dd886

Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>

Fix wrong type names in docstrings.

b687d54

Update docstrings. Rename fewshot_example_type to prompt_example_type.

b804c5b

rmitsch added Test external Run external tests Test GPU Run GPU tests labels Aug 17, 2023

Empty CUDA cache in GPU tests.

cee3e01

svlandeg approved these changes Aug 17, 2023

View reviewed changes

rmitsch merged commit 2572b0f into develop Aug 18, 2023
11 checks passed

svlandeg mentioned this pull request Aug 21, 2023

Add entity linking task #241

Merged

3 tasks

svlandeg deleted the feat/swappable-parsing branch August 24, 2023 12:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swappable parsing #242

Swappable parsing #242

rmitsch commented Aug 3, 2023 •

edited

Loading

rmitsch commented Aug 4, 2023

rmitsch commented Aug 4, 2023

svlandeg left a comment

svlandeg commented Aug 9, 2023

svlandeg left a comment

rmitsch commented Aug 16, 2023

rmitsch commented Aug 16, 2023

svlandeg left a comment

svlandeg commented Aug 17, 2023

svlandeg left a comment

Swappable parsing #242

Swappable parsing #242

Conversation

rmitsch commented Aug 3, 2023 • edited Loading

Description

Corresponding documentation PR

Types of change

Checklist

rmitsch commented Aug 4, 2023

rmitsch commented Aug 4, 2023

svlandeg left a comment

Choose a reason for hiding this comment

svlandeg commented Aug 9, 2023

svlandeg left a comment

Choose a reason for hiding this comment

rmitsch commented Aug 16, 2023

rmitsch commented Aug 16, 2023

svlandeg left a comment

Choose a reason for hiding this comment

svlandeg commented Aug 17, 2023

svlandeg left a comment

Choose a reason for hiding this comment

rmitsch commented Aug 3, 2023 •

edited

Loading