From aa9e72158bdb1db5c0d543a8f886ee41f3c1adbb Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Sat, 14 Sep 2024 18:54:26 +0530 Subject: [PATCH 1/7] Update clip.cpp Added a comprehensive validation check to ensure that critical hyperparameters (hidden_size, n_head, n_layer, image_size, patch_size, projection_dim, eps) are not set to invalid or zero values. a throw statement to handle invalid hyperparameter values by raising an invalid_argument exception. Enhanced the error message within the catch block to log the specific exception message, aiding in debugging. --- examples/llava/clip.cpp | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index 5dfb333d1be8c..940b373c4f7c7 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1270,15 +1270,22 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { // load vision model auto & vision_model = new_clip->vision_model; auto & hparams = vision_model.hparams; - hparams.hidden_size = get_u32(ctx, format(KEY_N_EMBD, "vision")); - hparams.n_head = get_u32(ctx, format(KEY_N_HEAD, "vision")); - hparams.n_intermediate = get_u32(ctx, format(KEY_N_FF, "vision")); - hparams.n_layer = get_u32(ctx, format(KEY_N_BLOCK, "vision")); - hparams.image_size = get_u32(ctx, KEY_IMAGE_SIZE); - hparams.patch_size = get_u32(ctx, KEY_PATCH_SIZE); - hparams.projection_dim = get_u32(ctx, format(KEY_PROJ_DIM, "vision")); - hparams.eps = get_f32(ctx, format(KEY_LAYER_NORM_EPS, "vision")); - + try{ + hparams.hidden_size = get_u32(ctx, format(KEY_N_EMBD, "vision")); + hparams.n_head = get_u32(ctx, format(KEY_N_HEAD, "vision")); + hparams.n_intermediate = get_u32(ctx, format(KEY_N_FF, "vision")); + hparams.n_layer = get_u32(ctx, format(KEY_N_BLOCK, "vision")); + hparams.image_size = get_u32(ctx, KEY_IMAGE_SIZE); + hparams.patch_size = get_u32(ctx, KEY_PATCH_SIZE); + hparams.projection_dim = get_u32(ctx, format(KEY_PROJ_DIM, "vision")); + hparams.eps = get_f32(ctx, format(KEY_LAYER_NORM_EPS, "vision")); + if (hparams.hidden_size == 0 || hparams.n_head == 0 || hparams.n_layer == 0 || hparams.n_intermediate || hparams.image_size == 0 || hparams.patch_size || hparams.projection_dim || hparams.eps) { + throw std::invalid_argument("Invalid hyperparameter values"); + } + } catch (const std::exception& e) { + std::cerr << "Error while loading hyperparameters: " << e.what() << std::endl; + return false; + } try { int idx = get_key_idx(ctx, KEY_IMAGE_GRID_PINPOINTS); int n = gguf_get_arr_n(ctx, idx); From 28d1c4566ac1cf7233b86c1933b5ad3740e220b8 Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Tue, 17 Sep 2024 15:13:48 +0530 Subject: [PATCH 2/7] Update examples/llava/clip.cpp Used fprintf instead of output streams. Co-authored-by: Clint Herron --- examples/llava/clip.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index 940b373c4f7c7..7155832b47bea 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1283,7 +1283,7 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { throw std::invalid_argument("Invalid hyperparameter values"); } } catch (const std::exception& e) { - std::cerr << "Error while loading hyperparameters: " << e.what() << std::endl; + fprintf(stderr, "Error while loading hyperparameters: %s\n", e.what()); return false; } try { From cba03408710588b3f8a7abac8bc72c1088f78c28 Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Tue, 17 Sep 2024 15:46:59 +0530 Subject: [PATCH 3/7] Refactored error handling for hyperparameter validation in clip.cpp Removed the try-catch block and throw statements Added validation checks for all hyperparameters, ensuring they are not set to invalid or zero values. Used fprintf to log error messages when invalid hyperparameters are encountered. --- examples/llava/clip.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index 7155832b47bea..3327cb266dc58 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1270,7 +1270,6 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { // load vision model auto & vision_model = new_clip->vision_model; auto & hparams = vision_model.hparams; - try{ hparams.hidden_size = get_u32(ctx, format(KEY_N_EMBD, "vision")); hparams.n_head = get_u32(ctx, format(KEY_N_HEAD, "vision")); hparams.n_intermediate = get_u32(ctx, format(KEY_N_FF, "vision")); @@ -1279,12 +1278,10 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { hparams.patch_size = get_u32(ctx, KEY_PATCH_SIZE); hparams.projection_dim = get_u32(ctx, format(KEY_PROJ_DIM, "vision")); hparams.eps = get_f32(ctx, format(KEY_LAYER_NORM_EPS, "vision")); - if (hparams.hidden_size == 0 || hparams.n_head == 0 || hparams.n_layer == 0 || hparams.n_intermediate || hparams.image_size == 0 || hparams.patch_size || hparams.projection_dim || hparams.eps) { - throw std::invalid_argument("Invalid hyperparameter values"); + if (hparams.hidden_size == 0 || hparams.n_head == 0 || hparams.n_layer == 0 || hparams.n_intermediate == 0 || hparams.image_size == 0 || hparams.patch_size == 0 || hparams.projection_dim == 0 || hparams.eps == 0) { + fprintf(stderr, "Error: Invalid hyperparameter values\n"); + return false; } - } catch (const std::exception& e) { - fprintf(stderr, "Error while loading hyperparameters: %s\n", e.what()); - return false; } try { int idx = get_key_idx(ctx, KEY_IMAGE_GRID_PINPOINTS); From e08b9077607b03490f26c9ea00249c9669c7d9e4 Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:57:22 +0530 Subject: [PATCH 4/7] Update clip.cpp used try-catch block. validation is made by checking all the hyperparameters are are positive and logs an error if they are not. included detailed logging of hyperparameters if needed for debugging. --- examples/llava/clip.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index 3327cb266dc58..c2969504f6442 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1270,6 +1270,7 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { // load vision model auto & vision_model = new_clip->vision_model; auto & hparams = vision_model.hparams; + try{ hparams.hidden_size = get_u32(ctx, format(KEY_N_EMBD, "vision")); hparams.n_head = get_u32(ctx, format(KEY_N_HEAD, "vision")); hparams.n_intermediate = get_u32(ctx, format(KEY_N_FF, "vision")); @@ -1278,11 +1279,24 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { hparams.patch_size = get_u32(ctx, KEY_PATCH_SIZE); hparams.projection_dim = get_u32(ctx, format(KEY_PROJ_DIM, "vision")); hparams.eps = get_f32(ctx, format(KEY_LAYER_NORM_EPS, "vision")); - if (hparams.hidden_size == 0 || hparams.n_head == 0 || hparams.n_layer == 0 || hparams.n_intermediate == 0 || hparams.image_size == 0 || hparams.patch_size == 0 || hparams.projection_dim == 0 || hparams.eps == 0) { - fprintf(stderr, "Error: Invalid hyperparameter values\n"); + if (hparams.hidden_size <= 0 || hparams.n_head <= 0 || hparams.n_layer <= 0 || hparams.n_intermediate <= 0 || hparams.image_size <= 0 || hparams.patch_size <= 0 || hparams.projection_dim <= 0 || hparams.eps <= 0) { + LOG_TEE("Error: Invalid hyperparameter values\n"); return false; } - } + // Optionally log loaded hyperparameters for debugging + LOG_TEE("Loaded hyperparameters:\n"); + LOG_TEE("Hidden size: %d\n", hparams.hidden_size); + LOG_TEE("Number of heads: %d\n", hparams.n_head); + LOG_TEE("Number of intermediate units: %d\n", hparams.n_intermediate); + LOG_TEE("Number of layers: %d\n", hparams.n_layer); + LOG_TEE("Image size: %d\n", hparams.image_size); + LOG_TEE("Patch size: %d\n", hparams.patch_size); + LOG_TEE("Projection dimension: %d\n", hparams.projection_dim); + LOG_TEE("Layer norm epsilon: %f\n", hparams.eps); + } catch (const std::exception& e) { + LOG_TEE("Error while loading hyperparameters: %s\n", e.what()); +     } + try { int idx = get_key_idx(ctx, KEY_IMAGE_GRID_PINPOINTS); int n = gguf_get_arr_n(ctx, idx); From 9bea433f739c3ea9e2ff3f4698fc3309e10cbbed Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Mon, 23 Sep 2024 17:09:51 +0530 Subject: [PATCH 5/7] Update clip.cpp by deleting optional logs --- examples/llava/clip.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index c2969504f6442..433bda19a4da3 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1283,16 +1283,6 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { LOG_TEE("Error: Invalid hyperparameter values\n"); return false; } - // Optionally log loaded hyperparameters for debugging - LOG_TEE("Loaded hyperparameters:\n"); - LOG_TEE("Hidden size: %d\n", hparams.hidden_size); - LOG_TEE("Number of heads: %d\n", hparams.n_head); - LOG_TEE("Number of intermediate units: %d\n", hparams.n_intermediate); - LOG_TEE("Number of layers: %d\n", hparams.n_layer); - LOG_TEE("Image size: %d\n", hparams.image_size); - LOG_TEE("Patch size: %d\n", hparams.patch_size); - LOG_TEE("Projection dimension: %d\n", hparams.projection_dim); - LOG_TEE("Layer norm epsilon: %f\n", hparams.eps); } catch (const std::exception& e) { LOG_TEE("Error while loading hyperparameters: %s\n", e.what());      } From 36d9bbce6bd33d093311987d3826b1ab11ee5114 Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Tue, 24 Sep 2024 07:27:35 +0530 Subject: [PATCH 6/7] Updated examples/llava/clip.cpp added space Co-authored-by: slaren --- examples/llava/clip.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index 433bda19a4da3..41b81196dd229 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1270,7 +1270,7 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { // load vision model auto & vision_model = new_clip->vision_model; auto & hparams = vision_model.hparams; - try{ + try { hparams.hidden_size = get_u32(ctx, format(KEY_N_EMBD, "vision")); hparams.n_head = get_u32(ctx, format(KEY_N_HEAD, "vision")); hparams.n_intermediate = get_u32(ctx, format(KEY_N_FF, "vision")); From 46d20e11e2fc513145f5c41f7eaeb4c81a7a4730 Mon Sep 17 00:00:00 2001 From: Tejaakshaykumar <147340353+Tejaakshaykumar@users.noreply.github.com> Date: Tue, 24 Sep 2024 07:31:49 +0530 Subject: [PATCH 7/7] Updated clip.cpp added return type in catch block --- examples/llava/clip.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/llava/clip.cpp b/examples/llava/clip.cpp index 41b81196dd229..1f3a4ce02aa54 100644 --- a/examples/llava/clip.cpp +++ b/examples/llava/clip.cpp @@ -1285,6 +1285,7 @@ struct clip_ctx * clip_model_load(const char * fname, const int verbosity = 1) { } } catch (const std::exception& e) { LOG_TEE("Error while loading hyperparameters: %s\n", e.what()); + return false;      } try {