Create model execute command #680

alonre24 · 2021-04-12T12:58:23Z

AI.MODELRUN command is now deprecated by AI.MODELEXECUTE.
In enterprise cluster, only AI.MODELEXECUTE will be available.

…hich is deprecated for enterprise cluster)

…ated.c

codecov · 2021-04-12T17:15:01Z

Codecov Report

Merging #680 (a4cceeb) into master (96b5761) will increase coverage by 0.52%.
The diff coverage is 83.29%.

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   78.37%   78.90%   +0.52%     
==========================================
  Files          41       43       +2     
  Lines        6535     6850     +315     
==========================================
+ Hits         5122     5405     +283     
- Misses       1413     1445      +32

Impacted Files	Coverage Δ
src/execution/DAG/dag_parser.c	`95.32% <ø> (ø)`
src/execution/utils.c	`71.42% <71.42%> (ø)`
src/redis_ai_objects/tensor.c	`86.20% <75.00%> (-0.12%)`	⬇️
src/execution/deprecated.c	`78.06% <78.06%> (ø)`
src/backends/onnxruntime.c	`75.79% <79.59%> (+2.68%)`	⬆️
src/redisai.c	`85.65% <87.71%> (+0.51%)`	⬆️
src/execution/command_parser.c	`88.12% <93.50%> (+4.78%)`	⬆️
src/redis_ai_objects/model.c	`78.68% <97.14%> (+1.53%)`	⬆️
src/redis_ai_objects/script.c	`82.50% <100.00%> (ø)`
src/redismodule.h	`100.00% <100.00%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96b5761...a4cceeb. Read the comment docs.

src/execution/command_parser.c

src/execution/deprecated.c

src/redis_ai_objects/model.c

src/redis_ai_objects/tensor.c

src/redisai.c

Moved RAI_GetTensoe/ModelFromKeyspace functions to execution/utils, and call the function that validates that this key is located in the current shard from within (relevant to enterprise only).

docs/commands.md

src/execution/DAG/dag_parser.c

src/execution/command_parser.c

src/execution/utils.h

src/redis_ai_objects/tensor.h

DvirDukhan · 2021-04-13T12:50:50Z

src/redisai.c

+extern int rlecMajorVersion;
+extern int rlecMinorVersion;
+extern int rlecPatchVersion;
+extern int rlecBuild;


why do you need those here after they moved to utils?

Because we use them in the onLoad function also...

src/execution/utils.c

DvirDukhan · 2021-04-13T12:53:38Z

src/execution/utils.c

+            return false;
+        }
+    }
+    RedisModule_Log(ctx, "warning", "could not load %s from keyspace, key doesn't exist",


notice that you log this message in RAI_GetModelFromKeyspace
please consolidate the error messages

You right, the logging in RAI_GetModelFromKeyspace should occur only in LITE version where we don't call VerifyKeyInThisShard

so why we have it two places?

DvirDukhan · 2021-04-13T12:55:33Z

src/execution/command_parser.c

+        if (!VerifyKeyInThisShard(ctx, outkeys[i])) { // Relevant for enterprise cluster.
+            RAI_SetError(err, RAI_EMODELRUN,
+                         "ERR CROSSSLOT Keys in request don't hash to the same slot");
+            return REDISMODULE_ERR;


move this to the actual parsing phase. fail as soon as possible when you can

We can't because the parsing function is used from dag command as well (and in DAG we do not take the keys from key space...)
But ModelRunCtx_SetParams is called only from "pure" AI.MODELEXECUTE.

… to depracted.c - Replace modelrun and modelset with modelexecute and modelstroe in onnx and pytorch tests. - Create a new test file (currently still empty) for testing deprcated APIs.

…Use "tests_commands" to test the new commands syntax (not for a specific backend), and test the deprecated commands only in "test_deprecated". Update AI.MODELSTORE documentation.

docs/intro.md

tests/flow/includes.py

docs/commands.md

DvirDukhan · 2021-04-19T11:46:13Z

tests/flow/tests_onnx.py

@@ -150,42 +74,30 @@ def test_onnx_modelrun_batchdim_mismatch(env):
        return

    con = env.getConnection()
+    model_pb = load_from_file('batchdim_mismatch.onnx')


remove enitre test

…isAI into MODELEXECUTE_command

DvirDukhan · 2021-04-20T06:22:20Z

src/backends/onnxruntime.c

+    array_new_on_stack(const char *, 5, input_names)
+        array_new_on_stack(const char *, 5, output_names) array_new_on_stack(OrtValue *, 5, inputs)
+            array_new_on_stack(OrtValue *, 5, outputs) OrtTensorTypeAndShapeInfo *info = NULL;


Suggested change

array_new_on_stack(const char *, 5, input_names)

array_new_on_stack(const char *, 5, output_names) array_new_on_stack(OrtValue *, 5, inputs)

array_new_on_stack(OrtValue *, 5, outputs) OrtTensorTypeAndShapeInfo *info = NULL;

int stack_arr_size = 5;

array_new_on_stack(const char *, stack_arr_size , input_names);

array_new_on_stack(const char *, stack_arr_size , output_names);

array_new_on_stack(OrtValue *, stack_arr_size , inputs);

array_new_on_stack(OrtValue *, stack_arr_size , outputs);

OrtTensorTypeAndShapeInfo *info = NULL;

you also need to free inputs and outputs in the end

Create model execute command (in addition to the model run command, w…

51b4c2d

…hich is deprecated for enterprise cluster)

alonre24 requested a review from DvirDukhan April 12, 2021 12:58

alonre24 added 2 commits April 12, 2021 16:16

Refactor - put AI.MODELRUN functions in a separate file called deprec…

b646b02

…ated.c

Small fix for valgrind

01cd46d

DvirDukhan requested changes Apr 12, 2021

View reviewed changes

alonre24 added 2 commits April 13, 2021 13:21

PR fixes.

28f54bc

Moved RAI_GetTensoe/ModelFromKeyspace functions to execution/utils, and call the function that validates that this key is located in the current shard from within (relevant to enterprise only).

Documentation

606bb89

DvirDukhan requested changes Apr 13, 2021

View reviewed changes

alonre24 and others added 4 commits April 13, 2021 16:55

More PR fixes

d6673e8

- Add modelstore command (new version for modelset) and move modelset…

7df134a

… to depracted.c - Replace modelrun and modelset with modelexecute and modelstroe in onnx and pytorch tests. - Create a new test file (currently still empty) for testing deprcated APIs.

Refactor tests: use the new commands instead of the deprecated ones. …

11649ea

…Use "tests_commands" to test the new commands syntax (not for a specific backend), and test the deprecated commands only in "test_deprecated". Update AI.MODELSTORE documentation.

Merge branch 'master' into MODELEXECUTE_command

c57e3ab

DvirDukhan reviewed Apr 19, 2021

View reviewed changes

alonre24 added 3 commits April 19, 2021 18:53

PR fixes + fix leak in onnx modelrun

5cebe5a

Merge branch 'MODELEXECUTE_command' of https://github.com/RedisAI/Red…

d3efe52

…isAI into MODELEXECUTE_command

Fix in LLAPI test module loading

928f7cc

DvirDukhan reviewed Apr 20, 2021

View reviewed changes

fix format + leak in onnx

a4cceeb

DvirDukhan approved these changes Apr 20, 2021

View reviewed changes

alonre24 merged commit a874faf into master Apr 20, 2021

alonre24 deleted the MODELEXECUTE_command branch April 20, 2021 08:12

-    array_new_on_stack(const char *, 5, input_names)
-        array_new_on_stack(const char *, 5, output_names) array_new_on_stack(OrtValue *, 5, inputs)
-            array_new_on_stack(OrtValue *, 5, outputs) OrtTensorTypeAndShapeInfo *info = NULL;
+    int stack_arr_size = 5;
+    array_new_on_stack(const char *, stack_arr_size , input_names);
+    array_new_on_stack(const char *, stack_arr_size , output_names);
+    array_new_on_stack(OrtValue *, stack_arr_size , inputs);
+    array_new_on_stack(OrtValue *, stack_arr_size , outputs);
+     OrtTensorTypeAndShapeInfo *info = NULL;

Create model execute command #680

Create model execute command #680

Uh oh!

Conversation

alonre24 commented Apr 12, 2021

Uh oh!

codecov bot commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Apr 12, 2021 •

edited

Loading