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.

Sign up for GitHub

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

Jump to bottom

Separate sequence and sentence features #6045

Merged
merged 80 commits into from
Jun 26, 2020
Merged

Separate sequence and sentence features #6045

merged 80 commits into from
Jun 26, 2020

Conversation

tabergma
Copy link
Contributor

@tabergma tabergma commented Jun 19, 2020 •
edited
Loading

Proposed changes:

  • Separate sequence and sentence features.
  • DIET process sequence and sentence features independently. The features are concatenated just before the transformer.
  • This allows us to have different kind of features for the sequence and the sentence.
  • LexicalSyntacticFeaturizer does not produce sentence features anymore.
  • ConveRTFeaturizer does not duplicate the sequence features anymore as they don't need to fit the size of the sentence features anymore.

closes https://github.com/RasaHQ/research/issues/79

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

tabergma added 30 commits May 4, 2020 17:16
@tabergma
create Features class
d0c173b
@tabergma
Draft message.get_features
d0a22a8
@tabergma
fix get_sparse/dense_features
20bb3b5
@tabergma
padding lower dim features
9bd4e8b
@tabergma
update DIETClassifier
a9efca9
@tabergma
training works
313478d
@tabergma
prediction works
4b64f10
@tabergma
refactoring
c89d8af
@tabergma
Merge branch 'master' into nlu-configuration
b4f2a01
@tabergma
convert featurizer is independent from tokenizer
5a7c97f
@tabergma
set eager mode to False again
0bafdfd
@tabergma
naming
857f10f
@tabergma
naming
8775bab
@tabergma
check if additional ffn is needed
ca4a653
@tabergma
Merge branch 'master' into nlu-configuration
57280e6
@tabergma
fix concat bug
6f9685b
@tabergma
use sparse dense dim
062af61
@tabergma
remove convert tokenizer
707e1cc
@tabergma
remove not needed constants
05fda3f
@tabergma
start fixing tests
b14464c
@tabergma
use dense dim
096cb29
@tabergma
fix more tests
3f9df89
@tabergma
fix more tests
71c8c41
@tabergma
fix testing
892a122
@tabergma
Merge branch 'master' into nlu-configuration
c602b26
@tabergma
Merge branch 'master' into nlu-configuration
56341ab
@tabergma
Merge branch 'master' into nlu-configuration
d71fbdf
@tabergma
clean up
e9c69c2
@tabergma
fix check label features present
d1cdb4c
@tabergma
add option to not produce sentence features
3dc5b2d
@tabergma tabergma requested a review from Ghostvv June 25, 2020 09:49
Ghostvv
Ghostvv reviewed Jun 25, 2020
View reviewed changes
Copy link
Contributor

@Ghostvv Ghostvv left a comment

Choose a reason for hiding this comment

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

looks good. Let's update sequence_lengths calculation for clarity, because there is no need to calculate it for sentence features

rasa/nlu/classifiers/diet_classifier.py Outdated Show resolved Hide resolved
@tabergma
Copy link
Contributor Author

@Ghostvv Update the PR, looks fine from my side now. Currently testing one more time a couple of configurations locally.

@tabergma tabergma requested a review from Ghostvv June 26, 2020 07:52
Ghostvv
Ghostvv reviewed Jun 26, 2020
View reviewed changes
rasa/nlu/classifiers/diet_classifier.py Outdated Show resolved Hide resolved
rasa/nlu/classifiers/diet_classifier.py Outdated Show resolved Hide resolved
Ghostvv
Ghostvv approved these changes Jun 26, 2020
View reviewed changes
Copy link
Contributor

@Ghostvv Ghostvv left a comment

Choose a reason for hiding this comment

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

looks good! left a comment about label default features

rasa/nlu/classifiers/diet_classifier.py Show resolved Hide resolved
@tabergma tabergma merged commit 88ad06f into master Jun 26, 2020
@tabergma tabergma deleted the nlu-configuration branch June 26, 2020 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ghostvv Ghostvv Ghostvv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tabergma @Ghostvv