Run evaluation in a separate process when using HTTP API #1778
Conversation
|
Code Climate has analyzed commit 3f667d4 and detected 3 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
…to eval_model_unload_fix
MetcalfeTom
requested review from
wochinge and
ricwo
and removed request for
wochinge
There was a problem hiding this comment.
Nice work 👍 The number of workers in the process pool used to be determined only by the training processes, but now evaluation makes use of the pool in the same way. I think it would be good to rename the Data Router kwarg to something like max_process_pool_workers and similarly adjust something like current_pool_processes in the evaluation and training process. What do you think?
There was a problem hiding this comment.
Great work @MetcalfeTom . Left a few comments regarding the debug statements, but looks good otherwise!
Co-Authored-By: MetcalfeTom <t.metcalfe@rasa.com>
There was a problem hiding this comment.
Thanks for addressing the comments, looks great 💯 !
From reading over the changes again, do you think calling the new "pool" attributes and classes "worker" processes instead would make more sense? To me this sounds a bit more idiomatic but I don't mind too much. so e.g. MaxWorkerProcessError, self.worker_processes, self.current_worker_processes. To me "pool" is very much a python multiprocessing term and "worker" sounds more general
|
Agreed that it's a better term |
Fixes #1733
Proposed changes:
rasa_nlu.testin that thread using the model path and return the resultsStatus (please check what you already did):
- [ ] updated the documentation