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 support for replacing listeners #7

Merged
merged 11 commits into from
Jul 11, 2023

Conversation

shadeMe
Copy link
Collaborator

@shadeMe shadeMe commented Jul 4, 2023

Description

Multiple changes were required to facilitate this:

  • All transformer model entrypoints now have a wrapped_listener optional parameter. This parameter is only meant to be used by the machinery that performs the listener replacement.

  • Listeners are no longer subclasses of the TransformerListener class. Previously, the TransformerListener class subclassed Model and stored some state as instance attributes. To perform the replacement, the original listener instance in the downstream component needs to be (deep)copied. However, the implementation of Model.copy doesn't support classes that subclass Model - it merely performs deepcopies of the different Model instance attributes and initializes a new Model instance with them. What this results in is the loss of any state that was directly stored on the listener instance such as upstream_name, name, etc.

    To workaround this limitation, all listener state is now directly stored in Model.attrs. This ensures that no persistent state is lost between copies.

  • A new WrappedTransformerAndListener class has been introduced to be used as the replacement model for the downstream component's original listener. This wraps the upstream transformer pipe's model and the original listener. During training and prediction, it calls the wrapped transformer and directly passes the outputs to the wrapped listener. Gradients are additionally allocated during training, and during prediction, the wrapped listener is instructed to ignore any transformer annotations present on the Docs and use the ones directly stored in the listener.

This PR depends on the following:

Types of change

(Cursed) enhancement

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.

Multiple changes were required to facilitate this:
* All transformer model entrypoints now have a `wrapped_listener` optional parameter.
This parameter is only meant to be used by the machinery that performs the listener
replacement.
* Listeners are no longer subclasses of the `TransformerListener` class. Previously,
the `TransformerListener` class subclassed `Model` and stored some state as instance
attributes. To perform the replacement, the original listener instance in the downstream
component needs to be (deep)copied. However, the implementation of `Model.copy` doesn't
support classes that subclass `Model` - it merely performs deepcopies of the different
`Model` instance attributes and initializes a new `Model` instance with them. What this
results in is the loss of any state that was directly stored on the listener instance such
as `upstream_name`, `name`, etc.

To workaround this limitation, all listener state is now directly stored in `Model.attrs`.
This ensures that no persistent state is lost between copies.
* A new `WrappedTransformerAndListener` class has been introduced to be used as the replacement
model for the downstream component's original listener. This wraps the upstream transformer pipe's
model and the original listener. During training and prediction, it calls the wrapped transformer
and direcly passes the outputs to the wrapped listener. Gradients are additionally allocated during
training, and during prediction, the wrapped listener is instructed to ignore any transformer annotations
present on the `Doc`s and use the ones directly stored in the listener.
@shadeMe shadeMe added the enhancement New feature or request label Jul 4, 2023
@shadeMe shadeMe marked this pull request as ready for review July 7, 2023 09:10
Copy link
Collaborator

@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.

A first bunch of comments, I probably need to go over this another time.

spacy_curated_transformers/models/listeners.py Outdated Show resolved Hide resolved
spacy_curated_transformers/models/listeners.py Outdated Show resolved Hide resolved
spacy_curated_transformers/models/listeners.py Outdated Show resolved Hide resolved
spacy_curated_transformers/pipeline/transformer.py Outdated Show resolved Hide resolved
Inline listener construction code and remove classes
@danieldk danieldk merged commit 9fd8acf into explosion:main Jul 11, 2023
6 checks passed
@shadeMe shadeMe deleted the feature/replace-listeners-support branch July 11, 2023 16:52
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
2 participants