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

Response Selector in Cross Validation #4976

Merged
merged 18 commits into from
Dec 18, 2019
Merged

Conversation

dakshvar22
Copy link
Contributor

@dakshvar22 dakshvar22 commented Dec 13, 2019

Proposed changes:

  • Added evaluation of response selection as part of cross validation pipeline.
  • Mostly mimics how evaluation for intent classification is done.

Status (please check what you already did):

  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

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

Left a couple of comments, especially tests would be great.
Also, I'm getting this when running rasa test nlu --cross-validation on the scaffold project:

Traceback (most recent call last):
  File "/Users/tobias/.pyenv/versions/3.7/bin/rasa", line 11, in <module>
    load_entry_point('rasa', 'console_scripts', 'rasa')()
  File "/Users/tobias/Workspace/stack/rasa/__main__.py", line 76, in main
    cmdline_arguments.func(cmdline_arguments)
  File "/Users/tobias/Workspace/stack/rasa/cli/test.py", line 143, in test_nlu
    perform_nlu_cross_validation(config, nlu_data, output, vars(args))
  File "/Users/tobias/Workspace/stack/rasa/test.py", line 206, in perform_nlu_cross_validation
    data, folds, nlu_config, output, **kwargs
  File "/Users/tobias/Workspace/stack/rasa/nlu/test.py", line 1271, in cross_validate
    train,
  File "/Users/tobias/Workspace/stack/rasa/nlu/test.py", line 1178, in combine_result
    ) = compute_metrics(interpreter, data)
  File "/Users/tobias/Workspace/stack/rasa/nlu/test.py", line 1359, in compute_metrics
    intent_results, "intent_target", "intent_prediction"
  File "/Users/tobias/Workspace/stack/rasa/nlu/test.py", line 1481, in _compute_metrics
    results, target_key, target_prediction
ValueError: not enough values to unpack (expected 2, got 0)

rasa/nlu/test.py Outdated Show resolved Hide resolved
rasa/nlu/test.py Outdated Show resolved Hide resolved
rasa/nlu/test.py Outdated Show resolved Hide resolved
rasa/nlu/test.py Outdated Show resolved Hide resolved
tests/nlu/base/test_evaluation.py Show resolved Hide resolved
changelog/4976.improvement.rst Outdated Show resolved Hide resolved
changelog/4976.improvement.rst Outdated Show resolved Hide resolved
dakshvar22 and others added 4 commits December 16, 2019 15:04
Co-Authored-By: Tobias Wochinger <t.wochinger@rasa.com>
Co-Authored-By: Tobias Wochinger <t.wochinger@rasa.com>
@dakshvar22
Copy link
Contributor Author

@wochinge Added tests and types. I couldn't reproduce the error that you got. What dataset and config did you use?

@dakshvar22 dakshvar22 self-assigned this Dec 17, 2019
Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

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

Good to go for me! Nice to see some love for the evaluation feature 🥇
Re the error with too few training data: create an issue and put in the maintenance board

tests/nlu/base/test_evaluation.py Outdated Show resolved Hide resolved
tests/nlu/base/test_evaluation.py Outdated Show resolved Hide resolved
tests/nlu/base/test_evaluation.py Outdated Show resolved Hide resolved
)

n_folds = 2
intent_results, entity_results, response_selection_results = cross_validate(
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: test if the report was written

@dakshvar22 dakshvar22 merged commit c094819 into master Dec 18, 2019
@dakshvar22 dakshvar22 deleted the add_response_selection_cv branch December 18, 2019 11:09
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.

2 participants