Skip to content

Commit c12c692

Browse files
author
DvirDukhan
committed
fixing PR comments. WIP
1 parent d97c40e commit c12c692

File tree

8 files changed

+84
-54
lines changed

8 files changed

+84
-54
lines changed

‎docs/commands.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ An array with alternating entries that represent the following key-value pairs:
521521
1. **DEVICE**: the script's device as a String
522522
2. **TAG**: the scripts's tag as a String
523523
3. **SOURCE**: the script's source code as a String
524-
* **ENTRY_POINTS** will return an array containing the script entry points
524+
4. **ENTRY_POINTS** will return an array containing the script entry points
525525

526526
**Examples**
527527

@@ -570,7 +570,7 @@ OK
570570

571571
## AI.SCRIPTEXECUTE
572572

573-
The **`AI.SCRIPTEXECUTE`** command runs a script stored as a key's value on its specified device. It a list keys, input tensors and addtional script args.
573+
The **`AI.SCRIPTEXECUTE`** command runs a script stored as a key's value on its specified device. It a list of keys, input tensors and addtional script args.
574574

575575
The run request is put in a queue and is executed asynchronously by a worker thread. The client that had issued the run request is blocked until the script run is completed. When needed, tensors data is automatically copied to the device prior to execution.
576576

@@ -657,6 +657,8 @@ redis> AI.TENSORGET result{tag} VALUES
657657
3) 1) "42"
658658
```
659659

660+
Note: for the time being, as `AI.SCRIPTSET` is still avialable to use, `AI.SCRIPTEXECUTE` still supports running functions that are part of scripts stored with `AI.SCRIPTSET` or imported from old RDB/AOF files. Meaning calling `AI.SCRIPTEXECUTE` over a function without the dedicated signature of `(tensors: List[Tensor], keys: List[str], args: List[str]` will yield a "best effort" execution to match the deprecated API `AI.SCRIPTRUN` function execution. This will map `INPUTS` tensors only, to their counterpart input arguments in the function, according to the order which they apear.
661+
660662
### Redis Commands support.
661663
In RedisAI TorchScript now supports simple (non-blocking) Redis commnands via the `redis.execute` API. The following script gets a key name (`x{1}`), and an `int` value (3). First, the script `SET`s the value in the key. Next, the script `GET`s the value back from the key, and sets it in a tensor which is eventually stored under the key 'y{1}'. Note that the inputs are `str` and `int`. The script sets and gets the value and set it into a tensor.
662664

‎src/backends/libtorch_c/torch_c.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ extern "C" void torchRunScript(void *scriptCtx, const char *fnName,
344344
* functions are not in the endpoint set to be executed in "best effort" manner.
345345
*/
346346
if(!torchScript_FunctionExists(ctx, fnName)) {
347-
throw std::runtime_error(std::string("Function does not exists: ") + fnName);
347+
throw std::runtime_error(std::string("Function does not exist: ") + fnName);
348348
}
349349
size_t nArgs = torchScript_FunctionArgumentCountByFunctionName(ctx, fnName);
350350
if(nArgs > inputsCtx->tensorCount) {

‎src/backends/torch.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ RAI_Script *RAI_ScriptCreateTorch(const char *devicestr, const char *scriptdef,
331331
if (!torchScript_FunctionExists(script, entryPoint)) {
332332
torchDeallocContext(script);
333333
char *errMsg;
334-
asprintf(&errMsg, "Fuction %s does not exists in the given script.", entryPoint);
334+
asprintf(&errMsg, "Function %s does not exist in the given script.", entryPoint);
335335
RAI_SetError(error, RAI_ESCRIPTCREATE, errMsg);
336336
free(errMsg);
337337
return NULL;
@@ -342,7 +342,7 @@ RAI_Script *RAI_ScriptCreateTorch(const char *devicestr, const char *scriptdef,
342342
if (argCount != 3) {
343343
torchDeallocContext(script);
344344
char *errMsg;
345-
asprintf(&errMsg, "Wrong number of inputs in fuction %s. Expected 3 but was %ld",
345+
asprintf(&errMsg, "Wrong number of inputs in function %s. Expected 3 but was %ld",
346346
entryPoint, argCount);
347347
RAI_SetError(error, RAI_ESCRIPTCREATE, errMsg);
348348
free(errMsg);
@@ -360,7 +360,7 @@ RAI_Script *RAI_ScriptCreateTorch(const char *devicestr, const char *scriptdef,
360360
torchDeallocContext(script);
361361
char *errMsg;
362362
asprintf(&errMsg,
363-
"Wrong inputs type in fuction %s. Expected signature similar to: def "
363+
"Wrong inputs type in function %s. Expected signature similar to: def "
364364
"%s(tensors: List[Tensor], keys: List[str], args: List[str])",
365365
entryPoint, entryPoint);
366366
RAI_SetError(error, RAI_ESCRIPTCREATE, errMsg);

‎src/redisai.c‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -834,14 +834,12 @@ int RedisAI_ScriptStore_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **ar
834834
AC_GetRString(&ac, &tag, 0);
835835
}
836836

837-
if (AC_IsAtEnd(&ac)) {
838-
return RedisModule_ReplyWithError(
839-
ctx, "ERR Insufficient arguments, missing script SOURCE and entry points");
840-
}
841-
842837
long long nEntryPoints;
843838
if (AC_AdvanceIfMatch(&ac, "ENTRY_POINTS")) {
844-
AC_GetLongLong(&ac, &nEntryPoints, 0);
839+
if (AC_GetLongLong(&ac, &nEntryPoints, 0) != AC_OK) {
840+
return RedisModule_ReplyWithError(
841+
ctx, "ERR Non numeric entry points number provided to AI.SCRIPTSTORE command");
842+
}
845843
} else {
846844
return RedisModule_ReplyWithError(
847845
ctx, "ERR Insufficient arguments, missing script entry points");
@@ -889,9 +887,6 @@ int RedisAI_ScriptStore_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **ar
889887
}
890888

891889
if (err.code != RAI_OK) {
892-
#ifdef RAI_PRINT_BACKEND_ERRORS
893-
printf("ERR: %s\n", err.detail);
894-
#endif
895890
int ret = RedisModule_ReplyWithError(ctx, err.detail_oneline);
896891
RAI_ClearError(&err);
897892
return ret;

‎tests/flow/test_data/script_bad.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
def bar(tensors: List[Tensor], keys: List[str], args: List[Tensor]):
2+
a = tensors[0]
3+
b = tensors[1]
4+
return a + b
5+
6+
def bar_variadic(tensors: List[Tensor], keys: List[str], args: List[str]):
7+
a = tensors[0]
8+
l = tensors[1:]
9+
return a + l[0]

‎tests/flow/tests_dag.py‎

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -395,13 +395,15 @@ def test_dagexecute_modelexecute_multidevice_resnet(env):
395395
'AI.DAGEXECUTE', 'KEYS', '1', image_key, '|>', 'AI.MODELEXECUTE', model_name_0,
396396
'INPUTS', 1, image_key, 'OUTPUTS', 1, temp_key1)
397397

398-
# check_error_message(env, con, "Wrong function name given to AI.SCRIPTEXECUTE command",
399-
# 'AI.DAGEXECUTE', 'KEYS', 1, '{1}',
400-
# '|>', 'AI.TENSORSET', image_key, 'UINT8', img.shape[1], img.shape[0], 3, 'BLOB', img.tobytes(),
401-
# '|>',
402-
# 'AI.SCRIPTEXECUTE', script_name, 'wrong_fn',
403-
# 'INPUTS', 1, image_key,
404-
# 'OUTPUTS', 1, temp_key1)
398+
ret = con.execute_command('AI.DAGEXECUTE',
399+
'KEYS', 1, '{1}','|>',
400+
'AI.TENSORSET', image_key, 'UINT8', img.shape[1], img.shape[0], 3, 'BLOB', img.tobytes(),'|>',
401+
'AI.SCRIPTEXECUTE', script_name, 'wrong_fn',
402+
'INPUTS', 1, image_key,
403+
'OUTPUTS', 1, temp_key1)
404+
env.assertEquals(b'OK', ret[0])
405+
env.assertEquals(type(ret[1]), redis.exceptions.ResponseError)
406+
env.assertEquals("Function does not exist: wrong_fn", ret[1].__str__())
405407

406408
check_error_message(env, con, "Number of keys given as INPUTS here does not match model definition",
407409
'AI.DAGEXECUTE', 'KEYS', 1, '{1}',
@@ -553,31 +555,39 @@ def test_dagexecute_modelexecute_multidevice_resnet_ensemble_alias(env):
553555
'INPUTS', 1, temp_key1,
554556
'OUTPUTS', 1, class_key_0)
555557

556-
# # The 'ensamble' function in script_name_0 expect to receive 2 inputs (not 1)
557-
# check_error_message(env, con, "Wrong number of inputs provided to AI.SCRIPTEXECUTE command",
558-
# 'AI.DAGEXECUTE',
559-
# 'PERSIST', '1', class_key_0, '|>',
560-
# 'AI.TENSORSET', image_key, 'UINT8', img.shape[1], img.shape[0], 3, 'BLOB', img.tobytes(),
561-
# '|>',
562-
# 'AI.SCRIPTEXECUTE', script_name_0, 'pre_process_3ch',
563-
# 'INPUTS', 1, image_key,
564-
# 'OUTPUTS', 1, temp_key1,
565-
# '|>',
566-
# 'AI.MODELEXECUTE', model_name_0,
567-
# 'INPUTS', 1, temp_key1,
568-
# 'OUTPUTS', 1, temp_key2_0,
569-
# '|>',
570-
# 'AI.MODELEXECUTE', model_name_1,
571-
# 'INPUTS', 1, temp_key1,
572-
# 'OUTPUTS', 1, temp_key2_1,
573-
# '|>',
574-
# 'AI.SCRIPTEXECUTE', script_name_0, 'ensemble',
575-
# 'INPUTS', 1, temp_key2_0,
576-
# 'OUTPUTS', 1, temp_key1,
577-
# '|>',
578-
# 'AI.SCRIPTEXECUTE', script_name_0, 'post_process',
579-
# 'INPUTS', 1, temp_key1,
580-
# 'OUTPUTS', 1, class_key_0)
558+
# The 'ensamble' function in script_name_0 expect to receive 2 inputs (not 1)
559+
ret = con.execute_command(
560+
'AI.DAGEXECUTE',
561+
'PERSIST', '1', class_key_0, '|>',
562+
'AI.TENSORSET', image_key, 'UINT8', img.shape[1], img.shape[0], 3, 'BLOB', img.tobytes(),
563+
'|>',
564+
'AI.SCRIPTEXECUTE', script_name_0, 'pre_process_3ch',
565+
'INPUTS', 1, image_key,
566+
'OUTPUTS', 1, temp_key1,
567+
'|>',
568+
'AI.MODELEXECUTE', model_name_0,
569+
'INPUTS', 1, temp_key1,
570+
'OUTPUTS', 1, temp_key2_0,
571+
'|>',
572+
'AI.MODELEXECUTE', model_name_1,
573+
'INPUTS', 1, temp_key1,
574+
'OUTPUTS', 1, temp_key2_1,
575+
'|>',
576+
'AI.SCRIPTEXECUTE', script_name_0, 'ensemble',
577+
'INPUTS', 1, temp_key2_0,
578+
'OUTPUTS', 1, temp_key1,
579+
'|>',
580+
'AI.SCRIPTEXECUTE', script_name_0, 'post_process',
581+
'INPUTS', 1, temp_key1,
582+
'OUTPUTS', 1, class_key_0)
583+
584+
env.assertEquals(b'OK', ret[0])
585+
env.assertEquals(b'OK', ret[1])
586+
env.assertEquals(b'OK', ret[2])
587+
env.assertEquals(b'OK', ret[3])
588+
env.assertEquals(b'NA', ret[5])
589+
env.assertEqual(type(ret[4]), redis.exceptions.ResponseError)
590+
env.assertTrue("list index out of range" in ret[4].__str__())
581591

582592
ret = con.execute_command(
583593
'AI.DAGEXECUTE',

‎tests/flow/tests_gears_llapi.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ def test_runtime_error(self):
209209
con = self.env.getConnection()
210210
ret = con.execute_command('rg.trigger', 'ScriptRun_AsyncRunError_test3')
211211
# This should raise an exception
212-
self.env.assertTrue(str(ret[0]).startswith("b'Function does not exists:"))
212+
self.env.assertTrue(str(ret[0]).startswith("b'Function does not exist:"))
213213

214214

215215
class TestDAGRunExecution:
@@ -335,7 +335,7 @@ def test_scriptexecute_op_runtime_error(self):
335335
con = self.env.getConnection()
336336
ret = con.execute_command('rg.trigger', 'DAGRun_test4')
337337
# This should raise an exception
338-
self.env.assertTrue(str(ret[0]).startswith("b'Function does not exists:"))
338+
self.env.assertTrue(str(ret[0]).startswith("b'Function does not exist:"))
339339

340340
def test_build_dag_from_string(self):
341341
con = self.env.getConnection()

‎tests/flow/tests_pytorch.py‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -235,16 +235,30 @@ def test_pytorch_scriptstore(env):
235235
return
236236

237237
con = env.getConnection()
238+
script = load_file_content('script.txt')
239+
old_script = load_file_content('old_script.txt')
240+
bad_script = load_file_content('script_bad.txt')
238241

239-
check_error(env, con, 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'SOURCE', 'return 1')
242+
check_error_message(env, con, "wrong number of arguments for 'AI.SCRIPTSTORE' command", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'SOURCE', 'return 1')
240243

241-
check_error(env, con, 'AI.SCRIPTSTORE', 'nope')
244+
check_error_message(env, con, "wrong number of arguments for 'AI.SCRIPTSTORE' command", 'AI.SCRIPTSTORE', 'nope')
242245

243-
check_error(env, con, 'AI.SCRIPTSTORE', 'nope', 'SOURCE')
246+
check_error_message(env, con, "wrong number of arguments for 'AI.SCRIPTSTORE' command", 'AI.SCRIPTSTORE', 'nope', 'SOURCE')
244247

245-
check_error(env, con, 'AI.SCRIPTSTORE', 'more', DEVICE)
248+
check_error_message(env, con, "wrong number of arguments for 'AI.SCRIPTSTORE' command", 'AI.SCRIPTSTORE', 'more', DEVICE)
249+
250+
check_error_message(env, con, "Insufficient arguments, missing script entry points", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'NO_ENTRY_POINTS', 2, 'bar', 'bar_variadic', 'SOURCE', script)
251+
252+
check_error_message(env, con, "Non numeric entry points number provided to AI.SCRIPTSTORE command", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'ENTRY_POINTS', 'ENTRY_POINTS', 'bar', 'bar_variadic', 'SOURCE', script)
253+
254+
check_error_message(env, con, "Function bar1 does not exist in the given script.", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'ENTRY_POINTS', 2, 'bar', 'bar1', 'SOURCE', script)
255+
256+
check_error_message(env, con, "Insufficient arguments, missing script SOURCE", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'ENTRY_POINTS', 2, 'bar', 'bar_variadic', 'SOURCE')
257+
258+
check_error_message(env, con, "Wrong number of inputs in function bar. Expected 3 but was 2", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'ENTRY_POINTS', 2, 'bar', 'bar_variadic', 'SOURCE', old_script)
259+
260+
check_error_message(env, con, "Wrong inputs type in function bar. Expected signature similar to: def bar(tensors: List[Tensor], keys: List[str], args: List[str])", 'AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'ENTRY_POINTS', 2, 'bar', 'bar_variadic', 'SOURCE', bad_script)
246261

247-
script = load_file_content('script.txt')
248262

249263
ret = con.execute_command('AI.SCRIPTSTORE', 'ket{1}', DEVICE, 'ENTRY_POINTS', 2, 'bar', 'bar_variadic', 'SOURCE', script)
250264
env.assertEqual(ret, b'OK')

0 commit comments

Comments
 (0)