diff --git a/gpt4all-backend/llmodel.cpp b/gpt4all-backend/llmodel.cpp index f46b4c95..93bf4997 100644 --- a/gpt4all-backend/llmodel.cpp +++ b/gpt4all-backend/llmodel.cpp @@ -1,5 +1,4 @@ #include "llmodel.h" -#include "dlhandle.h" #include #include @@ -7,10 +6,7 @@ #include #include - - -static -bool requires_avxonly() { +static bool requires_avxonly() { #ifdef __x86_64__ #ifndef _MSC_VER return !__builtin_cpu_supports("avx2"); @@ -24,7 +20,6 @@ bool requires_avxonly() { #endif } - LLModel::Implementation::Implementation(Dlhandle &&dlhandle_) : dlhandle(std::move(dlhandle_)) { auto get_model_type = dlhandle.get("get_model_type"); assert(get_model_type); @@ -42,8 +37,9 @@ bool LLModel::Implementation::isImplementation(const Dlhandle &dl) { return dl.get("is_g4a_backend_model_implementation"); } - const std::vector &LLModel::getImplementationList() { + // NOTE: allocated on heap so we leak intentionally on exit so we have a chance to clean up the + // individual models without the cleanup of the static list interfering static auto* libs = new std::vector([] () { std::vector fres; @@ -51,6 +47,9 @@ const std::vector &LLModel::getImplementationList() { // Iterate over all libraries for (const auto& f : std::filesystem::directory_iterator(path)) { // Get path + // FIXME: Remove useless comment and avoid usage of 'auto' where having the type is + // helpful for code readability so someone doesn't have to look up the docs for what + // type is returned by 'path' as it is not std::string const auto& p = f.path(); // Check extension if (p.extension() != LIB_FILE_EXT) continue; @@ -76,6 +75,8 @@ const std::vector &LLModel::getImplementationList() { } const LLModel::Implementation* LLModel::getImplementation(std::ifstream& f, const std::string& buildVariant) { + // FIXME: Please remove all these useless comments as the code itself is more than enough in these + // instances to tell what is going on // Iterate over all libraries for (const auto& i : getImplementationList()) { f.seekg(0); diff --git a/gpt4all-backend/llmodel.h b/gpt4all-backend/llmodel.h index 6b6828bf..7c9bafc7 100644 --- a/gpt4all-backend/llmodel.h +++ b/gpt4all-backend/llmodel.h @@ -1,6 +1,7 @@ #ifndef LLMODEL_H #define LLMODEL_H -#include "dlhandle.h" + +#include "dlhandle.h" // FIXME: would be nice to move this into implementation file #include #include @@ -9,13 +10,13 @@ #include #include - class LLModel { public: class Implementation { LLModel *(*construct_)(); public: + // FIXME: Move the whole implementation details to cpp file Implementation(Dlhandle&&); static bool isImplementation(const Dlhandle&); @@ -30,6 +31,7 @@ public: return fres; } }; + struct PromptContext { std::vector logits; // logits of current context std::vector tokens; // current tokens in the context window @@ -62,16 +64,20 @@ public: virtual void setThreadCount(int32_t /*n_threads*/) {} virtual int32_t threadCount() const { return 1; } + // FIXME: This is unused?? const Implementation& getImplementation() const { return *implementation; } + // FIXME: Maybe have an 'ImplementationInfo' class for the GUI here, but the DLHandle stuff should + // be hidden in cpp file + // FIXME: Avoid usage of 'get' for getters static const std::vector& getImplementationList(); static const Implementation *getImplementation(std::ifstream& f, const std::string& buildVariant); static LLModel *construct(const std::string &modelPath, std::string buildVariant = "default"); protected: - const Implementation *implementation; + const Implementation *implementation; // FIXME: This is dangling! You don't initialize it in ctor either virtual void recalculateContext(PromptContext &promptCtx, std::function recalculate) = 0;