Handling tag with no prefix for aggregation_strategy in TokenClassificationPipeline

[jbpolle]

🚀 Feature request

Previously the parameter grouped_entities would handle entity with no prefix (like "PER" instead of "B-PER") and would correctly group similar entities next to each others. With the new parameter aggregation_strategy, this is not the case anymore.

Motivation

In some simple models, the prefix add some complexity that is not always required. Because of this we are forced to add a prefix to make aggregation works even if not required by the model.

Your contribution

[LysandreJik]

cc @Narsil

[Narsil]

Hi @jbpolle what do you mean correctly ? We should not have changed behavior there, but indeed it's not part of the testing right now, so there might be some issues.

Could you provide a small script on an older transformers version that displays the intended behavior ?

[jbpolle]

Hello Nicolas, Here is what it looks like now in the "hosted inference API » panel: This is from my model here: https://huggingface.co/Jean-Baptiste/camembert-ner?text=Je+m%27appelle+jean-baptiste+et+je+vis+%C3%A0+montr%C3%A9al In previous version, It would display « jean-baptiste PER » and « Montreal LOC ». However I renamed my entities in the config.json file to I-PER, I-ORG,…which I believe should fix this issue. Before that the entities were just PER, LOC,… I hope this help, Thank you, Jean-Baptiste

…

Le 30 août 2021 à 09:15, Nicolas Patry ***@***.***> a écrit : Hi @jbpolle <https://github.com/jbpolle> what do you mean correctly ? We should not have changed behavior there, but indeed it's not part of the testing right now, so there might be some issues. Could you provide a small script on an older transformers version that displays the intended behavior ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13325 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMIMGPPBHLPBSACPHW5PZWDT7OAAXANCNFSM5DAT4PAA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[jbpolle]

Adding missing screenshot in previous message:

[Narsil]

I went back to 4.3.3 and I can see that the splitting was exactly the same. (no grouping when tags didn't include B-, I- ).

The fact that the cache wasn't probably cleaned on the widget is still an issue, clearing it.

[jbpolle]

I was working on 4.3.2 and here is how this was working:

But now in 4.9:

And even when playing with new aggregation_strategy parameters, I can't get previous results.
Anyway it's fixed in my case by adding the prefix so don't hesitate to close the ticket.

Thank you,

[Narsil]

Ok, I must have tested it wrong before. I can confirm. This is indeed because the default for tags wasn't really explicited, but did behave as I-

Code was:

entity["entity"].split("-")[0] != "B"

Which would resolve to "PER" != "B" whereas now the default tag was explicitely set as B-:
https://github.com/huggingface/transformers/blob/master/src/transformers/pipelines/token_classification.py#L413

The fix would be easy but I am unsure about reverting this now that was merged 6th June.
Tagging a core maintainer for advice for how to handle this. @LysandreJik

We would need to run some numbers on the hub too, to get an idea of amount of affected repos.

[LysandreJik]

I would fix it to behave the same as it was in v4.3.2 as this is the expected behavior when using grouped_entities

[Narsil]

PR opened. #13493