Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Multi-threaded inferencing support now lacking a unittest? #19082

Open
DickJC123 opened this issue Sep 3, 2020 · 3 comments
Open

Multi-threaded inferencing support now lacking a unittest? #19082

DickJC123 opened this issue Sep 3, 2020 · 3 comments

Comments

@DickJC123
Copy link
Contributor

While we generally insist that a new feature come with unittests at time of introduction, it's also important to ensure those tests are not removed or marked 'skipped' for too long to protect the feature from falling into disrepair.

Back in February, @anirudh2290's PR #16654 introduced multi-threaded inferencing, with new files ./src/imperative/cached_op_threadsafe.{h,cc} and a unittest under ./tests/cpp/thread_safety/. However in June, @eric-haibin-lin's PR #18598 "graph executor c api removal" removed the test files, but left the cached_op_threadsafe.{h,cc} files. Is there some other unittest of multi-threaded inferencing, and with which front end? If not, can the old test be reinstated or replaced?

@DickJC123
Copy link
Contributor Author

@eric-haibin-lin I work with a version of MXNet that has differences I have yet to PR, e.g. in the files imperative/cached_op.{h,cc}. When I went to merge PR #16654, there were merge conflicts, given the sizable refactoring, that I attempted to correct. However, since the threadsafe unittests were removed, I have no way of testing if my resolved merge preserved the functionality. Do you have a suggestion? Also @anirudh2290?

@szha
Copy link
Member

szha commented Sep 18, 2020

Looks like the removal was mainly due to the dependency on mxnet cpp package in the test. rewriting the test to use c api should be the way to go.

@barry-jin
Copy link
Contributor

Multi_threaded_inference test has been added back in #20131 along with the whole cpp-pakage and will be tested in ci/unix-gpu

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants