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

Bf/combine transformer embeddings #2558

Merged

Conversation

helpmefindaname
Copy link
Collaborator

@helpmefindaname helpmefindaname commented Dec 16, 2021

creates a transformer embedding that combines both TransformerWordEmbedding and TransformerDocumentEmbedding

it should be able to:

  • still load all embeddings saved with previous versions
  • should not need to load the tokenizer -> Cannot load NER model from local file #2445 should be fixed
  • allow using document embeddings and token embeddings at once (might be usefull for Multitask Learning in flair #2508 ?)
  • allow using long sequences / document context for DocumentClassification too (if the pooling is max or mean)
  • make code better maintainable, as there is no duplicated code

current state:

  • first implementation
  • run test
  • manually test training with all kinds of features
  • test loading old embeddings / test backwardscompability

@helpmefindaname helpmefindaname force-pushed the bf/combine_transformer_embeddings branch 3 times, most recently from 6de0b1e to 3803b00 Compare December 28, 2021 09:33
@helpmefindaname helpmefindaname marked this pull request as ready for review December 29, 2021 11:51
Copy link
Collaborator

@alanakbik alanakbik left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! Still testing, but found an error that appears with the following code:

embeddings = TransformerWordEmbeddings(model='xlm-roberta-base',
                                       layers="-1",
                                       subtoken_pooling="first",
                                       fine_tune=True,
                                       use_context=False,
                                       )
text = "."
sentence = Sentence(text)
embeddings.embed(sentence)

Suggestion to solve this (I think) added in-line.

flair/data.py Show resolved Hide resolved
flair/embeddings/base.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@alanakbik alanakbik left a comment

Choose a reason for hiding this comment

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

Thanks a lot for refactoring this!

@alanakbik alanakbik merged commit 7d5746f into flairNLP:master Dec 30, 2021
@alanakbik
Copy link
Collaborator

@helpmefindaname I found another error. It seems the fix for the previous error now broke sentences that are too long (over 512 subtokens).

Reproducible with this script:

from flair.data import Sentence
from flair.embeddings import TransformerWordEmbeddings

# example transformer embeddings
embeddings = TransformerWordEmbeddings(model='distilbert-base-uncased')

# create sentence with more than 512 subtokens
long_sentence = Sentence('a ' * 513)

# embed
embeddings.embed(long_sentence)

Throws the same assertion error as previously, i.e.:

  File ".../flair/flair/embeddings/base.py", line 769, in _add_embeddings_internal
    self._add_embeddings_to_sentences(expanded_sentences)
  File ".../flair/flair/embeddings/base.py", line 728, in _add_embeddings_to_sentences
    self._extract_token_embeddings(sentence_hidden_states, sentences, all_token_subtoken_lengths)
  File ".../flair/flair/embeddings/base.py", line 656, in _extract_token_embeddings
    assert subword_start_idx < subword_end_idx <= sentence_hidden_state.size()[1]
AssertionError

Any ideas how to fix this?

@isaac47
Copy link

isaac47 commented Sep 20, 2022

Hi,
this merged request is done? How can use it?

@helpmefindaname helpmefindaname deleted the bf/combine_transformer_embeddings branch November 28, 2022 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants