Ner label re alignment
Author: elk-cloner
Pull request: yes

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Fixes #10263


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [x] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case. [link](https://github.com/huggingface/transformers/issues/10263#issuecomment-781648059)
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?
- pipelines: @LysandreJik, @Narsil, @joshdevins

## What capabilities have been added ?
label realignment: token predictions for subwords can be realigned with 4 different strategies
- default: reset all subword token predictions except for first token
- first: the prediction for the first token in the word is assigned to all subword tokens
- max: the highest confidence prediction among the subword tokens is assigned to all subword tokens
- average: the average pool of the predictions for all subwords is assigned to all subword tokens
- ignore subwords: enable ignoring subwords by merging tokens

## What are the expected changes from the current behavior?
- New flag subword_label_re_alignment enables realignment.
- Already existing flag ignore_subwords actually enables merging subwords.

## Example use cases with code sample enabled by the PR
```
ner = transformers.pipeline(
    'ner',
    model='elastic/distilbert-base-cased-finetuned-conll03-english',
    tokenizer='elastic/distilbert-base-cased-finetuned-conll03-english',
    ignore_labels=[],
    ignore_subwords=False,
    subword_label_re_alignment='average'
)
ner('Mark Musterman')
[
    {
        'word': 'Mark',
        'score': 0.999686598777771,
        'index': 1,
        'start': 0,
        'end': 4,
        'is_subword': False,
        'entity': 'B-PER'
    },
    {
        'word': 'Must',
        'score': 0.9995412826538086,
        'index': 2,
        'start': 5,
        'end': 9,
        'is_subword': False,
        'entity': 'I-PER'
    },
    {
        'word': '##erman',
        'score': 0.9996127486228943,
        'index': 3,
        'start': 9,
        'end': 14,
        'is_subword': True,
        'entity': 'I-PER'
    }
]
ner = transformers.pipeline(
    'ner',
    model='elastic/distilbert-base-cased-finetuned-conll03-english',
    tokenizer='elastic/distilbert-base-cased-finetuned-conll03-english',
    ignore_labels=[],
    ignore_subwords=True,
    subword_label_re_alignment='average'
)
ner('Mark Musterman')
[
    {
        'word': 'Mark',
        'score': 0.999686598777771,
        'index': 1,
        'start': 0,
        'end': 4,
        'is_subword': False,
        'entity': 'B-PER'
    },
    {
        'word': 'Musterman',
        'score': 0.9995412826538086,
        'index': 2,
        'start': 5,
        'end': 9,
        'is_subword': False,
        'entity': 'I-PER'
    }
]

```

## Previous use cases with code sample that see the behavior changes
```
ner = transformers.pipeline(
    'ner',
    model='elastic/distilbert-base-cased-finetuned-conll03-english',
    tokenizer='elastic/distilbert-base-cased-finetuned-conll03-english',
    ignore_labels=[],
    ignore_subwords=True
)
ner('Mark Musterman')
[
    {
        'word': 'Mark',
        'score': 0.999686598777771,
        'entity': 'B-PER',
        'index': 1,
        'start': 0,
        'end': 4
    },
    {
        'word': 'Must',
        'score': 0.9995412826538086,
        'entity': 'I-PER',
        'index': 2,
        'start': 5,
        'end': 9
    },
    {
        'word': '##erman',
        'score': 0.9996127486228943,
        'entity': 'I-PER',
        'index': 3,
        'start': 9,
        'end': 14
    }
]

```

Comments:
Thank you for addressing this! I left some minor comments/questions.

@LysandreJik I think this is ready for review now.

Hey @elk-cloner, @francescorubbo! That's an amazing work you've done here. The added tests are a wonderful addition, and will ensure the pipeline is as robust as it can be.

To make reviews easier, could you please fill in the PR description or add a comment mentioning the changes? For example:
- What capabilities have been added
- What are the expected changes from the current behavior

And optionally, if you have the time to:
- Example use cases with code sample enabled by the PR
- Previous use cases with code sample that see the behavior changes

If you don't have time to do any of that, that's perfectly fine - just let me know and I'll take care of it as soon as I have a bit of availability. 

Thanks again for the great work you've done here!

This looks good. I'm wondering if you can add some tests to verify the expected behaviour of two other scenarios from the bug report.

Specifically, the tests in the PR seem to ensure:
Accenture → A ##cc ##ent ##ure → B-ORG O O O → Accenture (ORG)

...but does not make assertions for mixed B/I/O labels in the same word:
Max Mustermann → Max Must ##erman ##n → B-PER I-PER I-PER O → Max Mustermann (PER)

...or inner entity labels surrounded by O labels:
Elasticsearch → El ##astic ##sea #rch → O O I-MISC O → Elasticsearch (MISC)


@joshdevins Thank you for suggesting to test those additional scenarios. Testing for those helped me identify some bugs in the previous implementation. I believe the new test should cover all three scenarios now.

@LysandreJik I'll add the requested notes here, as I don't seem to have permissions to edit the PR description. Maybe @elk-cloner can transfer some of the info there.

> What capabilities have been added

## label realignment

Token predictions for subwords can be realigned with 4 different strategies

- default: reset all subword token predictions except for first token
- first: the prediction for the first token in the word is assigned to all subword tokens
- max: the highest confidence prediction among the subword tokens is assigned to all subword tokens
- average: the average pool of the predictions for all subwords is assigned to all subword tokens
- ignore subwords: enable ignoring subwords by merging tokens

> What are the expected changes from the current behavior

## New flag `subword_label_re_alignment` enables realignment.

Already existing flag `ignore_subwords` actually enables merging subwords.

> Example use cases with code sample enabled by the PR

```
ner = transformers.pipeline('ner',
                            model='elastic/distilbert-base-cased-finetuned-conll03-english',
                            tokenizer='elastic/distilbert-base-cased-finetuned-conll03-english',
                            ignore_labels = [],
                            ignore_subwords=False,
                            subword_label_re_alignment='average'
                           )
ner('Mark Musterman')
[{'word': 'Mark',
  'score': 0.999686598777771,
  'index': 1,
  'start': 0,
  'end': 4,
  'is_subword': False,
  'entity': 'B-PER'},
 {'word': 'Must',
  'score': 0.9995412826538086,
  'index': 2,
  'start': 5,
  'end': 9,
  'is_subword': False,
  'entity': 'I-PER'},
 {'word': '##erman',
  'score': 0.9996127486228943,
  'index': 3,
  'start': 9,
  'end': 14,
  'is_subword': True,
  'entity': 'I-PER'}]
ner = transformers.pipeline('ner',
                            model='elastic/distilbert-base-cased-finetuned-conll03-english',
                            tokenizer='elastic/distilbert-base-cased-finetuned-conll03-english',
                            ignore_labels = [],
                            ignore_subwords=True,
                            subword_label_re_alignment='average'
                           )
ner('Mark Musterman')
[{'word': 'Mark',
  'score': 0.999686598777771,
  'index': 1,
  'start': 0,
  'end': 4,
  'is_subword': False,
  'entity': 'B-PER'},
 {'word': 'Musterman',
  'score': 0.9995412826538086,
  'index': 2,
  'start': 5,
  'end': 9,
  'is_subword': False,
  'entity': 'I-PER'}]
```
> Previous use cases with code sample that see the behavior changes

```
ner = transformers.pipeline('ner',
                            model='elastic/distilbert-base-cased-finetuned-conll03-english',
                            tokenizer='elastic/distilbert-base-cased-finetuned-conll03-english',
                            ignore_labels = [],
                            ignore_subwords=True
                           )
ner('Mark Musterman')
[{'word': 'Mark',
  'score': 0.999686598777771,
  'entity': 'B-PER',
  'index': 1,
  'start': 0,
  'end': 4},
 {'word': 'Must',
  'score': 0.9995412826538086,
  'entity': 'I-PER',
  'index': 2,
  'start': 5,
  'end': 9},
 {'word': '##erman',
  'score': 0.9996127486228943,
  'entity': 'I-PER',
  'index': 3,
  'start': 9,
  'end': 14}]
```

Thank you, @francescorubbo, I added them to PR.

I haven't looked at the code changes yet, but looking at the proposed functionality changes. 
Referring to [this comment](https://github.com/huggingface/transformers/issues/10263#issuecomment-782187601):

> As a general principle, I would argue that if `grouped_entities=True`, we should never be returning sub-words alone. 
> Either they're part of a word that has a label, or they're not. I honestly still don't understand what the flag `ignore_subwords` is supposed to control 🤷

It used to be that `grouped_entities=True` wouldn't treat subwords differently, `ignore_subwords` was added as a way to provide the current default behaviour, while still allowing `ignore_subwords=False` to be set for backwards compatibility. 
Indeed I had [similar thoughts](https://github.com/huggingface/transformers/pull/5970#issuecomment-693374424) about how the subwords should be treated, & if there was need for a custom strategy (average,max etc)

I like the below proposal as it can be seen as an expansion of the current logic:

> I would propose two flags:
> 
> * `grouped_entities` (boolean) -- note that this implies subword grouping/label realignment (see below)
>   
>   * `True` will group all words into larger entities, e.g. Max Mustermann -> B-PER I-PER -> "Max Musterman" (PER)
>   * `False` will leave words separated, , e.g. Max Mustermann -> B-PER I-PER -> "Max Musterman" (PER)
> * `subword_label_realignment` (boolean or strategy name)
>   
>   * `True` will use the default for the way the NER fine-tuning was performed, see default suggestions above
>   * `False` will leave sub-words alone -- note that this implies that `grouped_entities` should be ignores
>   * strategy name -- based on the above strategies

❗ Except that subword_label_realignment=False shouldn't ignore `grouped_entities`.  `grouped_entities` flag refers to B-I grouping not subword grouping.  We shouldn't enforce subword grouping with  `grouped_entities` flag ! We don't know what user cases there might be that use that combination.

👉  So my proposed generalized version would be like this:
 
`grouped_entities` (Current behaviour is left as is):
 * `True` will group all words into larger entities, e.g. Max Mustermann -> B-PER I-PER -> "Max Musterman" (PER)
 *  `False` will leave words separated, , e.g. Max Mustermann -> B-PER I-PER -> "Max Musterman" (B-PER I-PER)"

 `subword_label_realignment` (strategy name) (Replaces `ignore_subwords`)

* none: Don't treat subwords differently (equal to old ignore_subwords=False)
* first: the prediction for the first token in the word is assigned to the word (equal to old ignore_subwords=True, the current default behaviour)
* max: the highest confidence prediction among the wordpiece tokens is assigned to the word (New feature)
* average: the average pool of the predictions among the wordpiece tokens is assigned to the word  (New feature)

Here `subword_label_realignment`  becomes actually an expansion of the `ignore_subwords` flag.

Also I don't understand what the below mode is supposed to mean @francescorubbo @elk-cloner 
> default: reset all subword token predictions except for first token

Thank you for the feedback, @LysandreJik @sgugger @cceyda !
I've refactored things as follows:
- the new argument is named `aggregation_strategy` and only determines how score and label of the word are computed if the `ignore_subwords` argument is `True`
- the possible strategies are mapped to the `AggregationStrategy` enum
- expected results for the tests are moved into json fixtures

Note that I didn't push the refactor as far as @cceyda suggested because I wanted to preserve backward-compatibility, as also requested by @LysandreJik .

For some reason merging the latest master is causing the code quality check to fail on files unrelated to this PR...any thought on that?

There was a new release of the black library which touched a lot of files, so you will need to rebase your PR on master to have the quality tests pass again.

> There was a new release of the black library which touched a lot of files, so you will need to rebase your PR on master to have the quality tests pass again.

I did merge master (see 031f3ef39db9b7164bad783ca17086cdcf000389). Shouldn't it address that?

I think originally there was also mention of saving the `aggregation_strategy` to the model config?
since it makes the most sense to use the same strategy the model was trained on, ignoring subwords or else.

> I think originally there was also mention of saving the aggregation_strategy to the model config?
> since it makes the most sense to use the same strategy the model was trained on, ignoring subwords or else.

@cceyda Yes, this was my original proposal, but I think it might be too much for one PR. I would not close the original issue (https://github.com/huggingface/transformers/issues/10263) until the other items are addressed, but perhaps a new/smaller PR can address saving the strategy used at training/evaluation time to the model config file.

ugh...this ^ is why I hate rebasing on big project repos...
@sgugger from a cursory look the 215 (!) file diffs look legit, please let me know if this PR needs any more work before you can merge.

@LysandreJik @sgugger Is there more work needed for this PR? If the rebase is an issue, I can create a new PR with only the relevant changes, but we would loose the commit history.

We can't see the diff of the PR anymore after the rebase, so you should close this one and open a new one from the same branch please. (GitHub completely sucks at properly showing rebases, unless you force push after the rebase.)