From 66f2063da25b1da851801132f20f9cb675831740 Mon Sep 17 00:00:00 2001 From: l3utterfly Date: Tue, 3 Oct 2023 03:33:52 +0800 Subject: [PATCH 1/5] fixed floating point comparison issues --- llama.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/llama.cpp b/llama.cpp index 05b570bd12ee2..5205be576fb5a 100644 --- a/llama.cpp +++ b/llama.cpp @@ -930,6 +930,8 @@ static const size_t kB = 1024; static const size_t MB = kB*kB; static const size_t GB = kB*kB*kB; +const double EPSILON = 1e-9; + struct llama_hparams { bool vocab_only; uint32_t n_vocab; @@ -948,7 +950,22 @@ struct llama_hparams { float rope_freq_scale_train; bool operator!=(const llama_hparams & other) const { - return static_cast(memcmp(this, &other, sizeof(llama_hparams))); // NOLINT + if(this->vocab_only != other.vocab_only) return true; + if(this->n_vocab != other.n_vocab) return true; + if(this->n_ctx_train != other.n_ctx_train) return true; + if(this->n_embd != other.n_embd) return true; + if(this->n_head != other.n_head) return true; + if(this->n_head_kv != other.n_head_kv) return true; + if(this->n_layer != other.n_layer) return true; + if(this->n_rot != other.n_rot) return true; + if(this->n_ff != other.n_ff) return true; + + if(std::abs(this->f_norm_eps - other.f_norm_eps) > EPSILON) return true; + if(std::abs(this->f_norm_rms_eps - other.f_norm_rms_eps) > EPSILON) return true; + if(std::abs(this->rope_freq_base_train - other.rope_freq_base_train) > EPSILON) return true; + if(std::abs(this->rope_freq_scale_train - other.rope_freq_scale_train) > EPSILON) return true; + + return false; } uint32_t n_gqa() const { From 16f45c4deceadb1f18283fb0e384349f1f2cf389 Mon Sep 17 00:00:00 2001 From: l3utterfly Date: Wed, 4 Oct 2023 18:01:50 +0800 Subject: [PATCH 2/5] updated implementation for hparam comparison to handle inf and NaN --- llama.cpp | 63 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/llama.cpp b/llama.cpp index 5205be576fb5a..e5a72a2bac060 100644 --- a/llama.cpp +++ b/llama.cpp @@ -123,6 +123,28 @@ static void replace_all(std::string & s, const std::string & search, const std:: } s = std::move(result); } + +bool is_float_eq(float a, float b, float abs_tol) { + // Check for non-negative tolerance + if (abs_tol < 0.0) { + throw std::invalid_argument("Tolerance must be non-negative"); + } + + // Exact equality check + if (a == b) { + return true; + } + + // Check for infinities + if (std::isinf(a) || std::isinf(b)) { + return false; + } + + // Regular comparison using the provided absolute tolerance + double diff = std::fabs(b - a); + return (diff <= abs_tol); +} + #ifdef GGML_USE_CPU_HBM #include #endif @@ -930,8 +952,6 @@ static const size_t kB = 1024; static const size_t MB = kB*kB; static const size_t GB = kB*kB*kB; -const double EPSILON = 1e-9; - struct llama_hparams { bool vocab_only; uint32_t n_vocab; @@ -949,23 +969,30 @@ struct llama_hparams { float rope_freq_base_train; float rope_freq_scale_train; - bool operator!=(const llama_hparams & other) const { - if(this->vocab_only != other.vocab_only) return true; - if(this->n_vocab != other.n_vocab) return true; - if(this->n_ctx_train != other.n_ctx_train) return true; - if(this->n_embd != other.n_embd) return true; - if(this->n_head != other.n_head) return true; - if(this->n_head_kv != other.n_head_kv) return true; - if(this->n_layer != other.n_layer) return true; - if(this->n_rot != other.n_rot) return true; - if(this->n_ff != other.n_ff) return true; - - if(std::abs(this->f_norm_eps - other.f_norm_eps) > EPSILON) return true; - if(std::abs(this->f_norm_rms_eps - other.f_norm_rms_eps) > EPSILON) return true; - if(std::abs(this->rope_freq_base_train - other.rope_freq_base_train) > EPSILON) return true; - if(std::abs(this->rope_freq_scale_train - other.rope_freq_scale_train) > EPSILON) return true; + bool operator==(const llama_hparams & other) const { + if (this->vocab_only != other.vocab_only) return false; + if (this->n_vocab != other.n_vocab) return false; + if (this->n_ctx_train != other.n_ctx_train) return false; + if (this->n_embd != other.n_embd) return false; + if (this->n_head != other.n_head) return false; + if (this->n_head_kv != other.n_head_kv) return false; + if (this->n_layer != other.n_layer) return false; + if (this->n_rot != other.n_rot) return false; + if (this->n_ff != other.n_ff) return false; - return false; + const float EPSILON = 1e-9; + + if (!is_float_eq(this->f_norm_eps, other.f_norm_eps, EPSILON)) return false; + if (!is_float_eq(this->f_norm_rms_eps, other.f_norm_rms_eps, EPSILON)) return false; + if (!is_float_eq(this->rope_freq_base_train, other.rope_freq_base_train, EPSILON)) return false; + if (!is_float_eq(this->rope_freq_scale_train, other.rope_freq_scale_train, EPSILON)) return false; + + return true; + } + + // implement != explicitly using the "==" implementation above so we don't get a warning about it + bool operator!=(const llama_hparams & other) const { + return !(*this == other); } uint32_t n_gqa() const { From d9cb48f063240ceed32ad0a7f5856bf366a6819a Mon Sep 17 00:00:00 2001 From: l3utterfly Date: Wed, 4 Oct 2023 22:23:43 +0800 Subject: [PATCH 3/5] fixed code review comments --- llama.cpp | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/llama.cpp b/llama.cpp index e5a72a2bac060..135b040dc262c 100644 --- a/llama.cpp +++ b/llama.cpp @@ -124,7 +124,7 @@ static void replace_all(std::string & s, const std::string & search, const std:: s = std::move(result); } -bool is_float_eq(float a, float b, float abs_tol) { +static bool is_float_eq(float a, float b, float abs_tol) { // Check for non-negative tolerance if (abs_tol < 0.0) { throw std::invalid_argument("Tolerance must be non-negative"); @@ -969,30 +969,25 @@ struct llama_hparams { float rope_freq_base_train; float rope_freq_scale_train; - bool operator==(const llama_hparams & other) const { - if (this->vocab_only != other.vocab_only) return false; - if (this->n_vocab != other.n_vocab) return false; - if (this->n_ctx_train != other.n_ctx_train) return false; - if (this->n_embd != other.n_embd) return false; - if (this->n_head != other.n_head) return false; - if (this->n_head_kv != other.n_head_kv) return false; - if (this->n_layer != other.n_layer) return false; - if (this->n_rot != other.n_rot) return false; - if (this->n_ff != other.n_ff) return false; + bool operator!=(const llama_hparams & other) const { + if (this->vocab_only != other.vocab_only) return true; + if (this->n_vocab != other.n_vocab) return true; + if (this->n_ctx_train != other.n_ctx_train) return true; + if (this->n_embd != other.n_embd) return true; + if (this->n_head != other.n_head) return true; + if (this->n_head_kv != other.n_head_kv) return true; + if (this->n_layer != other.n_layer) return true; + if (this->n_rot != other.n_rot) return true; + if (this->n_ff != other.n_ff) return true; const float EPSILON = 1e-9; - if (!is_float_eq(this->f_norm_eps, other.f_norm_eps, EPSILON)) return false; - if (!is_float_eq(this->f_norm_rms_eps, other.f_norm_rms_eps, EPSILON)) return false; - if (!is_float_eq(this->rope_freq_base_train, other.rope_freq_base_train, EPSILON)) return false; - if (!is_float_eq(this->rope_freq_scale_train, other.rope_freq_scale_train, EPSILON)) return false; - - return true; - } + if (!is_float_eq(this->f_norm_eps, other.f_norm_eps, EPSILON)) return true; + if (!is_float_eq(this->f_norm_rms_eps, other.f_norm_rms_eps, EPSILON)) return true; + if (!is_float_eq(this->rope_freq_base_train, other.rope_freq_base_train, EPSILON)) return true; + if (!is_float_eq(this->rope_freq_scale_train, other.rope_freq_scale_train, EPSILON)) return true; - // implement != explicitly using the "==" implementation above so we don't get a warning about it - bool operator!=(const llama_hparams & other) const { - return !(*this == other); + return false; } uint32_t n_gqa() const { From bde943e6d6570b7b02d5ddd7a89dd73eaa25151f Mon Sep 17 00:00:00 2001 From: Cebtenzzre Date: Wed, 4 Oct 2023 11:13:02 -0400 Subject: [PATCH 4/5] minor simplification --- llama.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llama.cpp b/llama.cpp index 135b040dc262c..b8093da85a6b9 100644 --- a/llama.cpp +++ b/llama.cpp @@ -141,8 +141,7 @@ static bool is_float_eq(float a, float b, float abs_tol) { } // Regular comparison using the provided absolute tolerance - double diff = std::fabs(b - a); - return (diff <= abs_tol); + return std::fabs(b - a) <= abs_tol; } #ifdef GGML_USE_CPU_HBM From 4cffcb845a8ac4baddfc3be14bfa247c8da65c90 Mon Sep 17 00:00:00 2001 From: Cebtenzzre Date: Wed, 4 Oct 2023 14:02:14 -0400 Subject: [PATCH 5/5] rename is_float_eq -> is_float_close --- llama.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/llama.cpp b/llama.cpp index b8093da85a6b9..f3e9857981cf4 100644 --- a/llama.cpp +++ b/llama.cpp @@ -124,7 +124,7 @@ static void replace_all(std::string & s, const std::string & search, const std:: s = std::move(result); } -static bool is_float_eq(float a, float b, float abs_tol) { +static bool is_float_close(float a, float b, float abs_tol) { // Check for non-negative tolerance if (abs_tol < 0.0) { throw std::invalid_argument("Tolerance must be non-negative"); @@ -981,10 +981,10 @@ struct llama_hparams { const float EPSILON = 1e-9; - if (!is_float_eq(this->f_norm_eps, other.f_norm_eps, EPSILON)) return true; - if (!is_float_eq(this->f_norm_rms_eps, other.f_norm_rms_eps, EPSILON)) return true; - if (!is_float_eq(this->rope_freq_base_train, other.rope_freq_base_train, EPSILON)) return true; - if (!is_float_eq(this->rope_freq_scale_train, other.rope_freq_scale_train, EPSILON)) return true; + if (!is_float_close(this->f_norm_eps, other.f_norm_eps, EPSILON)) return true; + if (!is_float_close(this->f_norm_rms_eps, other.f_norm_rms_eps, EPSILON)) return true; + if (!is_float_close(this->rope_freq_base_train, other.rope_freq_base_train, EPSILON)) return true; + if (!is_float_close(this->rope_freq_scale_train, other.rope_freq_scale_train, EPSILON)) return true; return false; }