-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
add --num-threads
argument to train cli
#5086
Conversation
thanks for submitting this PR, we'll give it a review as soon as possible |
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.
Looks good so far 👍, however a couple of things are missing:
- The tests are failing, please fix them (see https://github.com/RasaHQ/rasa/blob/master/tests/cli/test_rasa_train.py).
- Please add a changelog entry (see https://github.com/RasaHQ/rasa/tree/master/changelog)
- Your branch is not up-to-date, please update
0474812
to
806822d
Compare
806822d
to
d0268a9
Compare
@mludv Are you still working on this? If yes, would be great if you could tackle my comments. Would be great if we can merge this soon. Thanks! |
Hi Tanja, |
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 need to update the CLI tests. Apart from that it looks good 👍
* add `--num-threads` argument to train cli * Update rasa/cli/train.py * Create 5086.feature.rst * fixed tests Co-authored-by: Tom Bocklisch <tom@rasa.com>
Hey guys, I did some work on issue #4141
Take a look and say what you think
Proposed changes:
--num-threads
cli arguments is added and forwarded to thekwargs
of the NLU componentstrain
method.Status (please check what you already did):
black
(please check Readme for instructions)