-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Impossible to pass end to end test when the same entity is extracted by different classifiers #9771
Comments
@TyDunn So far, no customer has brought this up yet. The workaround is to not use the But since we encourage people to write test stories, I feel this should be fixed in 3.0.0 and not be treated as a nice to have. |
@hsm207 The reason is that it is a nice to have is because we want to avoid scope creep with this big release. The scope has been set for months now, and we risk delaying it if we add things at this point. If we have time, we'll get to it. Otherwise, let's keep it high in the CSE issues board and the team will take care of it shortly after the 3.0 release :) |
I've been able to reproduce this issue and it's worse than I thought 🙂 It boils down to us comparing expected and predicted entities in a naive and harsh way. It even insists on the entities being extracted in the same order as listed in the training story, which can get you into real trouble: Imagine you have - story: simple
steps:
- intent: inform
user: |
I'm a [researcher](job_name) from [Germany](country) Now, if you have The other thing is that the code that compares expected and predicted entities ignores roles and groups (which feels wrong). This being said, I'll focus on fixing the bug reported above and create followup issues for the other things. |
thanks @samsucik for the update! |
@JEM-Mosig fyi I'm now implementing a fix and I'm not de-duplicating the extracted entities globally, only for the purposes of the checks which compare expected and predicted entities. I think there's value in leaving duplicates in the extracted entities for now because it might uncover underlying issues with the way one uses multiple entity extractors for the same entity. And also because fixing only the checking code is simpler. |
@hsm207 I've created a fix in #9875. It addresses the issue with duplicated or differently ordered entity predictions. Note that it does not touch the actual attributes of a @JEM-Mosig review of the code should be quick but I'd like you to also challenge the overall approach I took 😉 |
Rasa Open Source version
2.8.7
Rasa SDK version
No response
Rasa X version
No response
Python version
3.8
What operating system are you using?
Linux
What happened?
rasa test
will fail when the same entity is extracted by different classifiers.Steps to reproduce:
i.e. list of countries
rasa test --fail-on-prediction-errors
Command / Request
No response
Relevant log output
Definition of done
@JEM-Mosig is reviewer
The text was updated successfully, but these errors were encountered: