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
8705 response selector #9010
8705 response selector #9010
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done! 💯
I have a few minor comments. Before we can merge this, please also
- add a change log entry
- run NLU model regression tests with all configurations on all datasets
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
/modeltest # Run "Rules + TED" config on all Core datasets.
dataset_branch: "variable_random_seed"
include:
- dataset: ["all"]
config: ["all-nlu"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting there... :)
Some conflicts with main
need to be resolved.
|
||
if training and label_attribute is not None: | ||
if training: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if training: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_create_model_data is also used during predict
`FeatureArray`s in case there is no subkey), or | ||
an empty list in case the requested data cannot be found | ||
""" | ||
default = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean most get
methods have None
as a default and we'd have to write default = []
which would be unusual? And if we wrote default = None
then we'd have to modify all calls.
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Co-authored-by: Johannes E. M. Mosig <j.mosig@rasa.com>
Hey @ka-bu! 👋 To run model regression tests, comment with the Tips 💡: The model regression test will be run on Tips 💡: Every time when you want to change a configuration you should edit the comment with the previous configuration. You can copy this in your comment and customize:
|
/modeltest include:
- dataset: ["all"]
config: ["all-nlu"] |
The model regression tests have started. It might take a while, please be patient. Used configuration can be found in the comment. |
Commit: 0502ae9, The full report is available as an artifact. Dataset:
Dataset:
Dataset:
Dataset:
Dataset:
Dataset:
Dataset:
Dataset:
|
Should all the configurations with |
Closing because we're merging this into 2.8.x via #9205 |
Note:
Continuing this on #9205. (closing this issue because we're merging this into 2.8.x via #9205)
Proposed changes:
use_text_as_label
and no transformer due to inconsistent data pre-processing #8705_use_default_label_features
)Status (please check what you already did):
black
(please check Readme for instructions)