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

Persist ConveRT model locally #11409

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Persist ConveRT model locally #11409

wants to merge 24 commits into from
+7 −4

Conversation

bielrossi15
Copy link

Proposed changes:

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)

dependabot bot and others added 22 commits February 16, 2022 23:43
@dependabot
Bump google-github-actions/setup-gcloud from 0.4.0 to 0.5.1
9ef98ab 
Bumps [google-github-actions/setup-gcloud](https://github.com/google-github-actions/setup-gcloud) from 0.4.0 to 0.5.1.
- [Release notes](https://github.com/google-github-actions/setup-gcloud/releases)
- [Changelog](https://github.com/google-github-actions/setup-gcloud/blob/master/CHANGELOG.md)
- [Commits](google-github-actions/setup-gcloud@e0f83f2...04141d8)

---
updated-dependencies:
- dependency-name: google-github-actions/setup-gcloud
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot
Bump actions/github-script from 4.0.2 to 6
d5f04e2 
Bumps [actions/github-script](https://github.com/actions/github-script) from 4.0.2 to 6.
- [Release notes](https://github.com/actions/github-script/releases)
- [Commits](actions/github-script@a3e7071...9ac0880)

---
updated-dependencies:
- dependency-name: actions/github-script
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot
Bump pytest-timeout from 1.4.2 to 2.1.0
9b570d3 
Bumps [pytest-timeout](https://github.com/pytest-dev/pytest-timeout) from 1.4.2 to 2.1.0.
- [Release notes](https://github.com/pytest-dev/pytest-timeout/releases)
- [Commits](pytest-dev/pytest-timeout@1.4.2...2.1.0)

---
updated-dependencies:
- dependency-name: pytest-timeout
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
@SamuelNoB
Merge pull request #197 from FGA-GCES/dependabot-github_actions-googl…
384ba6d 
…e-github-actions-setup-gcloud-0.5.1

Bump google-github-actions/setup-gcloud from 0.4.0 to 0.5.1
@SamuelNoB
Merge pull request #203 from FGA-GCES/dependabot-pip-pytest-timeout-2…
761a0b1 
….1.0

Bump pytest-timeout from 1.4.2 to 2.1.0
@SamuelNoB
Merge pull request #198 from FGA-GCES/dependabot-github_actions-actio…
0e4c64c 
…ns-github-script-6

Bump actions/github-script from 4.0.2 to 6
@SamuelNoB
Revert "Bump google-github-actions/setup-gcloud from 0.4.0 to 0.5.1"
c502f1a
@SamuelNoB
Revert "Bump actions/github-script from 4.0.2 to 6"
13c59ab
@SamuelNoB
Merge pull request #213 from FGA-GCES/revert-198-dependabot-github_ac…
10d18c4 
…tions-actions-github-script-6

Revert "Bump actions/github-script from 4.0.2 to 6"
@SamuelNoB
Merge pull request #212 from FGA-GCES/revert-197-dependabot-github_ac…
ae61b2c 
…tions-google-github-actions-setup-gcloud-0.5.1

Revert "Bump google-github-actions/setup-gcloud from 0.4.0 to 0.5.1"
@SamuelNoB
Merge remote-tracking branch 'origin/main' into main
9ee2329
@SamuelNoB
Merge branch 'main' of https://github.com/FGA-GCES/rasa
78b861b
@SamuelNoB
Merge branch 'RasaHQ:main' into main
f297a6b
@SamuelNoB
merging
e486972
@SamuelNoB
Merge branch 'RasaHQ-main' into main
b3f03ea
@SamuelNoB
Merge remote-tracking branch 'upstream/main' into main
2c97846
@RochaCarla
Merge branch 'RasaHQ:main' into main
dbaf4c5
@AlGouvea
Merge branch 'RasaHQ:main' into main
12c9ba0
@VitorLamego
Merge branch 'RasaHQ:main' into main
fb37af2
@bielrossi15
Adding cache_dir argument inside conveRT create()
f0c07eb
@bielrossi15
Creating changelog for the specific feature
0133fc6
@bielrossi15
Formatting code using black
d314367
@bielrossi15 bielrossi15 requested a review from a team as a code owner August 2, 2022 14:26
@CLAassistant
Copy link

CLAassistant commented Aug 2, 2022 •
edited

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 5 committers have signed the CLA.

✅ AlGouvea
✅ SamuelNoB
✅ VitorLamego
✅ bielrossi15
❌ RochaCarla
You have signed the CLA already but the status is still pending? Let us recheck it.

@tmbo
Copy link
Member

tmbo commented Aug 24, 2022

@twerkmeister do you mind taking a look at this

@twerkmeister twerkmeister self-requested a review August 29, 2022 08:22
twerkmeister
twerkmeister reviewed Aug 29, 2022
View reviewed changes
rasa/nlu/featurizers/dense_featurizer/convert_featurizer.py
@@ -89,8 +88,11 @@ def create(
model_storage: ModelStorage,
resource: Resource,
execution_context: ExecutionContext,
cache_dir: str = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be removed and instead the cache_dir should be added to the config in get_default_config

Copy link
Author

Choose a reason for hiding this comment

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

Ok, it's done!

twerkmeister
twerkmeister requested changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@twerkmeister twerkmeister left a comment •
edited

Choose a reason for hiding this comment

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

Thanks for the submission! Unfortunately, quite a few things need changing, but it's doable. See comments for requested changes.

Also, the documentation needs to be updated. The new parameter needs to be explained for the ConveRTFeaturizer in the components.mdx file in the docs

@bielrossi15
Copy link
Author

Ok, i'll be working on these changes!

@bielrossi15 @twerkmeister
Update changelog/8229.enhancement.md
89cca30 
Co-authored-by: Thomas Werkmeister <thomas@werkmeister.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twerkmeister twerkmeister twerkmeister requested changes

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.

Persist ConveRT model in a local persistent cache
8 participants
@bielrossi15 @CLAassistant @tmbo @twerkmeister @SamuelNoB @RochaCarla @AlGouvea @VitorLamego