Skip to content

Commit 2cbfca8

Browse files
filipecosta90lantiga
filipecosta90
and
lantiga
authored
Code refactoring, consistent error messages, and RedisAI_ModelRun_RedisCommand optimizations (#321)
* [wip] simplified argument processing on RedisAI_ModelRun_RedisCommand (only 1 full-loop vs 5) * [add] code refactoring to reduce duplicates and reduce complexity. [add] enforced error reply string comparison on tests. [fix] error messages consistent across commands for tensor getting, etc... * [add] increased coverage for tensorset and remove duplicate code * [fix] ensuring slaves synced on tests_pytorch:test_pytorch_scriptrun. [add] quick refactoring to have consistent command description on redisai.c * [fix] ERR error normalization. * Fix function names, error message, correct warning Co-authored-by: Luca Antiga <luca.antiga@orobix.com>
1 parent 4ec5163 commit 2cbfca8

File tree

19 files changed

+1306
-1248
lines changed

19 files changed

+1306
-1248
lines changed

‎src/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
ADD_LIBRARY(redisai_obj OBJECT
22
util/dict.c
3+
util/queue.c
34
redisai.c
45
backends.c
56
model.c

‎src/backends/onnxruntime.c‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ RAI_Model *RAI_ModelCreateORT(RAI_Backend backend, const char* devicestr, RAI_Mo
347347
#else
348348
// TODO: Do dynamic device/provider check with GetExecutionProviderType or something on these lines
349349
if (device == RAI_DEVICE_GPU) {
350-
RAI_SetError(error, RAI_EMODELCREATE, "GPU requested but ONNX couldn't find CUDA");
350+
RAI_SetError(error, RAI_EMODELCREATE, "ERR GPU requested but ONNX couldn't find CUDA");
351351
return NULL;
352352
}
353353
#endif
@@ -408,13 +408,13 @@ int RAI_ModelRunORT(RAI_ModelRunCtx *mctx, RAI_Error *error)
408408
OrtSession *session = mctx->model->session;
409409

410410
if (session == NULL) {
411-
RAI_SetError(error, RAI_EMODELRUN, "ONNXRuntime session was not allocated\n");
411+
RAI_SetError(error, RAI_EMODELRUN, "ERR ONNXRuntime session was not allocated");
412412
return 1;
413413
}
414414

415415
const size_t nbatches = array_len(mctx->batches);
416416
if (nbatches == 0) {
417-
RAI_SetError(error, RAI_EMODELRUN, "No batches to run\n");
417+
RAI_SetError(error, RAI_EMODELRUN, "ERR No batches to run");
418418
return 1;
419419
}
420420

@@ -462,14 +462,14 @@ int RAI_ModelRunORT(RAI_ModelRunCtx *mctx, RAI_Error *error)
462462

463463
if (ninputs != n_input_nodes) {
464464
char msg[70];
465-
sprintf(msg, "Expected %li inputs but got %li", n_input_nodes, ninputs);
465+
sprintf(msg, "ERR Expected %li inputs but got %li", n_input_nodes, ninputs);
466466
RAI_SetError(error, RAI_EMODELRUN, msg);
467467
return 1;
468468
}
469469

470470
if (noutputs != n_output_nodes) {
471471
char msg[70];
472-
sprintf(msg, "Expected %li outputs but got %li", n_output_nodes, noutputs);
472+
sprintf(msg, "ERR Expected %li outputs but got %li", n_output_nodes, noutputs);
473473
RAI_SetError(error, RAI_EMODELRUN, msg);
474474
return 1;
475475
}
@@ -500,14 +500,14 @@ int RAI_ModelRunORT(RAI_ModelRunCtx *mctx, RAI_Error *error)
500500
status = OrtSessionGetInputTypeInfo(session, i, &typeinfo);
501501
const OrtTensorTypeAndShapeInfo* tensor_info = OrtCastTypeInfoToTensorInfo(typeinfo);
502502
ONNXTensorElementDataType type = OrtGetTensorElementType(tensor_info);
503-
// printf("Input %zu : type=%d\n", i, type);
503+
// printf("Input %zu : type=%d", i, type);
504504

505505
size_t num_dims = OrtGetDimensionsCount(tensor_info);
506-
// printf("Input %zu : num_dims=%zu\n", i, num_dims);
506+
// printf("Input %zu : num_dims=%zu", i, num_dims);
507507
input_node_dims.resize(num_dims);
508508
OrtGetDimensions(tensor_info, (int64_t*)input_node_dims.data(), num_dims);
509509
for (size_t j = 0; j < num_dims; j++) {
510-
// printf("Input %zu : dim %zu=%jd\n", i, j, input_node_dims[j]);
510+
// printf("Input %zu : dim %zu=%jd", i, j, input_node_dims[j]);
511511
}
512512

513513
OrtReleaseTypeInfo(typeinfo);
@@ -549,7 +549,7 @@ int RAI_ModelRunORT(RAI_ModelRunCtx *mctx, RAI_Error *error)
549549
RAI_TensorFree(output_tensor);
550550
}
551551
else {
552-
printf("ERR: non-tensor output from ONNX models, ignoring (currently unsupported).\n");
552+
printf("ERR: non-tensor output from ONNX models, ignoring (currently unsupported)");
553553
}
554554
}
555555

‎src/backends/tensorflow.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,8 @@ RAI_Model *RAI_ModelCreateTF(RAI_Backend backend, const char* devicestr, RAI_Mod
256256
TF_Operation* oper = TF_GraphOperationByName(model, inputs[i]);
257257
if (oper == NULL) {
258258
size_t len = strlen(inputs[i]);
259-
char* msg = RedisModule_Calloc(40 + len, sizeof(*msg));
260-
sprintf(msg, "Input node named \"%s\" not found in TF graph.", inputs[i]);
259+
char* msg = RedisModule_Calloc(60 + len, sizeof(*msg));
260+
sprintf(msg, "ERR Input node named \"%s\" not found in TF graph.", inputs[i]);
261261
RAI_SetError(error, RAI_EMODELIMPORT, msg);
262262
return NULL;
263263
}
@@ -267,8 +267,8 @@ RAI_Model *RAI_ModelCreateTF(RAI_Backend backend, const char* devicestr, RAI_Mod
267267
TF_Operation* oper = TF_GraphOperationByName(model, outputs[i]);
268268
if (oper == NULL) {
269269
size_t len = strlen(outputs[i]);
270-
char* msg = RedisModule_Calloc(40 + len, sizeof(*msg));
271-
sprintf(msg, "Output node named \"%s\" not found in TF graph.", outputs[i]);
270+
char* msg = RedisModule_Calloc(60 + len, sizeof(*msg));
271+
sprintf(msg, "ERR Output node named \"%s\" not found in TF graph", outputs[i]);
272272
RAI_SetError(error, RAI_EMODELIMPORT, msg);
273273
return NULL;
274274
}
@@ -336,7 +336,7 @@ RAI_Model *RAI_ModelCreateTF(RAI_Backend backend, const char* devicestr, RAI_Mod
336336
}
337337
}
338338
if (foundNoGPU == 1 && device == RAI_DEVICE_GPU) {
339-
RAI_SetError(error, RAI_EMODELCREATE, "GPU requested but TF couldn't find CUDA");
339+
RAI_SetError(error, RAI_EMODELCREATE, "ERR GPU requested but TF couldn't find CUDA");
340340
TF_DeleteDeviceList(deviceList);
341341
TF_DeleteStatus(deviceListStatus);
342342
// TODO: free other memory allocations
@@ -424,7 +424,7 @@ int RAI_ModelRunTF(RAI_ModelRunCtx* mctx, RAI_Error *error) {
424424

425425
const size_t nbatches = array_len(mctx->batches);
426426
if (nbatches == 0) {
427-
RAI_SetError(error, RAI_EMODELRUN, "No batches to run\n");
427+
RAI_SetError(error, RAI_EMODELRUN, "ERR No batches to run");
428428
return 1;
429429
}
430430

@@ -522,7 +522,7 @@ int RAI_ModelSerializeTF(RAI_Model *model, char **buffer, size_t *len, RAI_Error
522522
TF_GraphToGraphDef(model->model, tf_buffer, status);
523523

524524
if (TF_GetCode(status) != TF_OK) {
525-
RAI_SetError(error, RAI_EMODELSERIALIZE, "Error serializing TF model");
525+
RAI_SetError(error, RAI_EMODELSERIALIZE, "ERR Error serializing TF model");
526526
TF_DeleteBuffer(tf_buffer);
527527
TF_DeleteStatus(status);
528528
return 1;

‎src/backends/tflite.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ RAI_Model *RAI_ModelCreateTFLite(RAI_Backend backend, const char* devicestr, RAI
2727
RAI_Device device;
2828
int64_t deviceid;
2929
if (!parseDeviceStr(devicestr, &device, &deviceid)) {
30-
RAI_SetError(error, RAI_EMODELCONFIGURE, "Unsupported device");
30+
RAI_SetError(error, RAI_EMODELCONFIGURE, "ERR Unsupported device");
3131
return NULL;
3232
}
3333

@@ -39,7 +39,7 @@ RAI_Model *RAI_ModelCreateTFLite(RAI_Backend backend, const char* devicestr, RAI
3939
dl_device = kDLGPU;
4040
break;
4141
default:
42-
RAI_SetError(error, RAI_EMODELCONFIGURE, "Error configuring model: unsupported device\n");
42+
RAI_SetError(error, RAI_EMODELCONFIGURE, "ERR Error configuring model: unsupported device");
4343
return NULL;
4444
}
4545

@@ -86,7 +86,7 @@ int RAI_ModelRunTFLite(RAI_ModelRunCtx* mctx, RAI_Error *error) {
8686

8787
const size_t nbatches = array_len(mctx->batches);
8888
if (nbatches == 0) {
89-
RAI_SetError(error, RAI_EMODELRUN, "No batches to run\n");
89+
RAI_SetError(error, RAI_EMODELRUN, "ERR No batches to run");
9090
return 1;
9191
}
9292

@@ -149,13 +149,13 @@ int RAI_ModelRunTFLite(RAI_ModelRunCtx* mctx, RAI_Error *error) {
149149

150150
for(size_t i=0 ; i<noutputs; ++i) {
151151
if (outputs_dl[i] == NULL) {
152-
RAI_SetError(error, RAI_EMODELRUN, "Model did not generate the expected number of outputs.");
152+
RAI_SetError(error, RAI_EMODELRUN, "ERR Model did not generate the expected number of outputs");
153153
return 1;
154154
}
155155
RAI_Tensor* output_tensor = RAI_TensorCreateFromDLTensor(outputs_dl[i]);
156156
if (nbatches > 1 && RAI_TensorDim(output_tensor, 0) != total_batch_size) {
157157
RAI_TensorFree(output_tensor);
158-
RAI_SetError(error, RAI_EMODELRUN, "Model did not generate the expected batch size.");
158+
RAI_SetError(error, RAI_EMODELRUN, "ERR Model did not generate the expected batch size");
159159
return 1;
160160
}
161161
if (nbatches > 1) {

‎src/backends/torch.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ RAI_Model *RAI_ModelCreateTorch(RAI_Backend backend, const char* devicestr, RAI_
3535
dl_device = kDLGPU;
3636
break;
3737
default:
38-
RAI_SetError(error, RAI_EMODELCONFIGURE, "Error configuring model: unsupported device\n");
38+
RAI_SetError(error, RAI_EMODELCONFIGURE, "ERR Error configuring model: unsupported device");
3939
return NULL;
4040
}
4141

@@ -71,7 +71,7 @@ void RAI_ModelFreeTorch(RAI_Model* model, RAI_Error *error) {
7171
int RAI_ModelRunTorch(RAI_ModelRunCtx* mctx, RAI_Error *error) {
7272
const size_t nbatches = array_len(mctx->batches);
7373
if (nbatches == 0) {
74-
RAI_SetError(error, RAI_EMODELRUN, "No batches to run\n");
74+
RAI_SetError(error, RAI_EMODELRUN, "ERR No batches to run");
7575
return 1;
7676
}
7777

@@ -134,7 +134,7 @@ int RAI_ModelRunTorch(RAI_ModelRunCtx* mctx, RAI_Error *error) {
134134

135135
for(size_t i=0 ; i<noutputs; ++i) {
136136
if (outputs_dl[i] == NULL) {
137-
RAI_SetError(error, RAI_EMODELRUN, "Model did not generate the expected number of outputs.");
137+
RAI_SetError(error, RAI_EMODELRUN, "ERR Model did not generate the expected number of outputs");
138138
return 1;
139139
}
140140
RAI_Tensor* output_tensor = RAI_TensorCreateFromDLTensor(outputs_dl[i]);
@@ -188,7 +188,7 @@ RAI_Script *RAI_ScriptCreateTorch(const char* devicestr, const char *scriptdef,
188188
dl_device = kDLGPU;
189189
break;
190190
default:
191-
RAI_SetError(error, RAI_ESCRIPTCONFIGURE, "Error configuring script: unsupported device\n");
191+
RAI_SetError(error, RAI_ESCRIPTCONFIGURE, "ERR Error configuring script: unsupported device");
192192
break;
193193
}
194194

‎src/err.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ void RAI_SetError(RAI_Error *err, RAI_ErrorCode code, const char *detail) {
2626
if (detail) {
2727
err->detail = RedisModule_Strdup(detail);
2828
} else {
29-
err->detail = RedisModule_Strdup("Generic error");
29+
err->detail = RedisModule_Strdup("ERR Generic error");
3030
}
3131

3232
err->detail_oneline = RAI_Chomp(err->detail);

‎src/model.c‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -220,34 +220,34 @@ RAI_Model *RAI_ModelCreate(RAI_Backend backend, const char* devicestr, const cha
220220
RAI_Model *model;
221221
if (backend == RAI_BACKEND_TENSORFLOW) {
222222
if (!RAI_backends.tf.model_create_with_nodes) {
223-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TF.\n");
223+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TF");
224224
return NULL;
225225
}
226226
model = RAI_backends.tf.model_create_with_nodes(backend, devicestr, opts, ninputs, inputs, noutputs, outputs, modeldef, modellen, err);
227227
}
228228
else if (backend == RAI_BACKEND_TFLITE) {
229229
if (!RAI_backends.tflite.model_create) {
230-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TFLITE.\n");
230+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TFLITE");
231231
return NULL;
232232
}
233233
model = RAI_backends.tflite.model_create(backend, devicestr, opts, modeldef, modellen, err);
234234
}
235235
else if (backend == RAI_BACKEND_TORCH) {
236236
if (!RAI_backends.torch.model_create) {
237-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TORCH.\n");
237+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TORCH");
238238
return NULL;
239239
}
240240
model = RAI_backends.torch.model_create(backend, devicestr, opts, modeldef, modellen, err);
241241
}
242242
else if (backend == RAI_BACKEND_ONNXRUNTIME) {
243243
if (!RAI_backends.onnx.model_create) {
244-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: ONNX.\n");
244+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: ONNX");
245245
return NULL;
246246
}
247247
model = RAI_backends.onnx.model_create(backend, devicestr, opts, modeldef, modellen, err);
248248
}
249249
else {
250-
RAI_SetError(err, RAI_EUNSUPPORTEDBACKEND, "Unsupported backend.\n");
250+
RAI_SetError(err, RAI_EUNSUPPORTEDBACKEND, "ERR Unsupported backend\n");
251251
return NULL;
252252
}
253253

@@ -265,28 +265,28 @@ void RAI_ModelFree(RAI_Model* model, RAI_Error* err) {
265265

266266
if (model->backend == RAI_BACKEND_TENSORFLOW) {
267267
if (!RAI_backends.tf.model_free) {
268-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TF.\n");
268+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TF\n");
269269
return;
270270
}
271271
RAI_backends.tf.model_free(model, err);
272272
}
273273
else if (model->backend == RAI_BACKEND_TFLITE) {
274274
if (!RAI_backends.tflite.model_free) {
275-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TFLITE.\n");
275+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TFLITE");
276276
return;
277277
}
278278
RAI_backends.tflite.model_free(model, err);
279279
}
280280
else if (model->backend == RAI_BACKEND_TORCH) {
281281
if (!RAI_backends.torch.model_free) {
282-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TORCH.\n");
282+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TORCH");
283283
return;
284284
}
285285
RAI_backends.torch.model_free(model, err);
286286
}
287287
else if (model->backend == RAI_BACKEND_ONNXRUNTIME) {
288288
if (!RAI_backends.onnx.model_free) {
289-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: ONNX.\n");
289+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: ONNX");
290290
return;
291291
}
292292
RAI_backends.onnx.model_free(model, err);
@@ -428,34 +428,34 @@ int RAI_ModelRun(RAI_ModelRunCtx* mctx, RAI_Error* err) {
428428
switch (mctx->model->backend) {
429429
case RAI_BACKEND_TENSORFLOW:
430430
if (!RAI_backends.tf.model_run) {
431-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TF.\n");
431+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TF");
432432
return REDISMODULE_ERR;
433433
}
434434
ret = RAI_backends.tf.model_run(mctx, err);
435435
break;
436436
case RAI_BACKEND_TFLITE:
437437
if (!RAI_backends.tflite.model_run) {
438-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TFLITE.\n");
438+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TFLITE");
439439
return REDISMODULE_ERR;
440440
}
441441
ret = RAI_backends.tflite.model_run(mctx, err);
442442
break;
443443
case RAI_BACKEND_TORCH:
444444
if (!RAI_backends.torch.model_run) {
445-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TORCH.\n");
445+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TORCH");
446446
return REDISMODULE_ERR;
447447
}
448448
ret = RAI_backends.torch.model_run(mctx, err);
449449
break;
450450
case RAI_BACKEND_ONNXRUNTIME:
451451
if (!RAI_backends.onnx.model_run) {
452-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: ONNX.\n");
452+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: ONNX");
453453
return REDISMODULE_ERR;
454454
}
455455
ret = RAI_backends.onnx.model_run(mctx, err);
456456
break;
457457
default:
458-
RAI_SetError(err, RAI_EUNSUPPORTEDBACKEND, "Unsupported backend.\n");
458+
RAI_SetError(err, RAI_EUNSUPPORTEDBACKEND, "ERR Unsupported backend");
459459
return REDISMODULE_ERR;
460460
}
461461

@@ -473,34 +473,34 @@ int RAI_ModelSerialize(RAI_Model *model, char **buffer, size_t *len, RAI_Error *
473473
switch (model->backend) {
474474
case RAI_BACKEND_TENSORFLOW:
475475
if (!RAI_backends.tf.model_serialize) {
476-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TF.\n");
476+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TF");
477477
return REDISMODULE_ERR;
478478
}
479479
ret = RAI_backends.tf.model_serialize(model, buffer, len, err);
480480
break;
481481
case RAI_BACKEND_TFLITE:
482482
if (!RAI_backends.tflite.model_serialize) {
483-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TFLITE.\n");
483+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TFLITE");
484484
return REDISMODULE_ERR;
485485
}
486486
ret = RAI_backends.tflite.model_serialize(model, buffer, len, err);
487487
break;
488488
case RAI_BACKEND_TORCH:
489489
if (!RAI_backends.torch.model_serialize) {
490-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: TORCH.\n");
490+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: TORCH");
491491
return REDISMODULE_ERR;
492492
}
493493
ret = RAI_backends.torch.model_serialize(model, buffer, len, err);
494494
break;
495495
case RAI_BACKEND_ONNXRUNTIME:
496496
if (!RAI_backends.onnx.model_serialize) {
497-
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "Backend not loaded: ONNX.\n");
497+
RAI_SetError(err, RAI_EBACKENDNOTLOADED, "ERR Backend not loaded: ONNX");
498498
return REDISMODULE_ERR;
499499
}
500500
ret = RAI_backends.onnx.model_serialize(model, buffer, len, err);
501501
break;
502502
default:
503-
RAI_SetError(err, RAI_EUNSUPPORTEDBACKEND, "Unsupported backend.\n");
503+
RAI_SetError(err, RAI_EUNSUPPORTEDBACKEND, "ERR Unsupported backend");
504504
return REDISMODULE_ERR;
505505
}
506506

0 commit comments

Comments
 (0)