-
Notifications
You must be signed in to change notification settings - Fork 715
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
Supported WASI-NN streaming extension for NNRPC. #3386
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Arvind Ganesh <arvind.cganesh@gmail.com>
Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR. |
Signed-off-by: Arvind Ganesh <arvind.cganesh@gmail.com>
fa3a2b6
to
b792f9a
Compare
Signed-off-by: Arvind Ganesh <arvind.cganesh@gmail.com>
Signed-off-by: Arvind Ganesh <arvind.cganesh@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3386 +/- ##
==========================================
- Coverage 79.93% 79.84% -0.09%
==========================================
Files 251 253 +2
Lines 34368 34937 +569
Branches 5966 6148 +182
==========================================
+ Hits 27471 27897 +426
- Misses 5507 5621 +114
- Partials 1390 1419 +29 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Arvind Ganesh <arvind.cganesh@gmail.com>
@@ -155,12 +155,30 @@ TEST(WasiNNTest, OpenVINOBackend) { | |||
EXPECT_TRUE(FuncInst->isHostFunction()); | |||
auto &HostFuncGetOutput = | |||
dynamic_cast<WasmEdge::Host::WasiNNGetOutput &>(FuncInst->getHostFunc()); | |||
// Get the function "get_output_single". |
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.
I got confused. Why do you add these tests to the OpenVINO backend? The OpenVINO backend does not implement these LLM-related extension functions. That's why the unit tests will fail when trying to get the non-existing functions.
Req.set_resource_handle(Context); | ||
Req.set_index(Index); | ||
wasi_ephemeral_nn::GetOutputResult Res; | ||
auto Status = Stub->GetOutput(&ClientContext, Req, &Res); |
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.
auto Status = Stub->GetOutput(&ClientContext, Req, &Res); | |
auto Status = Stub->GetOutputSingle(&ClientContext, Req, &Res); |
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.
ping @arvganesh
Hi @arvganesh |
Addresses #3319. This PR did the following for the following functions:
ComputeSingle
,GetOutputSingle
, andFiniSingle
.ComputeSingle
,GetOutputSingle
,FiniSingle