Fix shell-out helpers and pass read-only parameters by const reference

example/vlm.cpp

@@ -6,8 +6,14 @@
 #include <memory>
 #include <mlx/io.h>
 #include <optional>
+#include <spawn.h>
 #include <string>
+#include <sys/wait.h>
 #include <variant>
+#include <vector>
+
+extern char **environ;
+
 int main() {
   // std::string ModelPath = "../../Llama-3.2-11B-Vision-Instruct-4bit/";
   // auto Model = mllama::Model::fromPretrained(ModelPath, {64, 4});
@@ -32,9 +38,19 @@ int main() {
   mx::array TokenArr =
       mx::array(TokenList.data(), {static_cast<int>(TokenList.size())});
   mx::save("output.npy", TokenArr);
-  std::system((std::string("python3.10 ../example/decode.py ") + ModelPath +
-               " output.npy")
-                  .c_str());
+  // Run the detokenizer without a shell so ModelPath is passed verbatim.
+  std::string Python = "python3.10";
+  std::string Script = "../example/decode.py";
+  std::string Output = "output.npy";
+  std::vector<char *> Argv = {Python.data(), Script.data(), ModelPath.data(),
+                              Output.data(), nullptr};
+  pid_t Pid;
+  if (posix_spawnp(&Pid, Argv[0], nullptr, nullptr, Argv.data(), environ) ==
+      0) {
+    waitpid(Pid, nullptr, 0);
+  } else {
+    spdlog::error("Failed to run {}", Script);
+  }
   const auto End{std::chrono::steady_clock::now()};
   const std::chrono::duration<double> ElapsedSeconds{End - Start};
   spdlog::info("Elapsed time: {} s. TPS: {}.", ElapsedSeconds.count(),

mlx/activations.cpp

@@ -2,16 +2,16 @@
 #include "math.h"
 #include <mlx/ops.h>
 namespace mlx::core {
-mx::array gelu(mx::array X) {
+mx::array gelu(const mx::array &X) {
   // auto Result = X * (1 + mx::erf(X / std::sqrt(2))) / 2;
   auto Result = X *
                 (mx::array({1}, X.dtype()) +
                  mx::erf(X / mx::array({std::sqrt(2)}, X.dtype()))) /
                 mx::array({2}, X.dtype());
   return Result;
 }
-mx::array silu(mx::array X) { return X * mx::sigmoid(X); }
-mx::array geluApprox(mx::array X) {
+mx::array silu(const mx::array &X) { return X * mx::sigmoid(X); }
+mx::array geluApprox(const mx::array &X) {
   return mx::array({0.5}, X.dtype()) * X *
          (mx::array({1}, X.dtype()) +
           mx::tanh(mx::array({std::sqrt(2.0 / M_PI)}, X.dtype()) *

mlx/activations.h

@@ -3,7 +3,7 @@
 #include <cmath>
 #include <mlx/array.h>
 namespace mlx::core {
-mx::array gelu(mx::array X);
-mx::array silu(mx::array X);
-mx::array geluApprox(mx::array X);
+mx::array gelu(const mx::array &X);
+mx::array silu(const mx::array &X);
+mx::array geluApprox(const mx::array &X);
 } // namespace mlx::core

mlx/base.cpp

@@ -1,13 +1,15 @@
 #include "base.h"
 #include "../model/utils.h"
 #include "spdlog/spdlog.h"
+#include <cstdint>
 #include <memory>
 #include <mlx/array.h>
+#include <mlx/ops.h>
 #include <unordered_map>
 
 namespace mlx::core::nn {
 
-mx::array &Module::registerParameter(std::string Name, mx::array &&W) {
+mx::array &Module::registerParameter(const std::string &Name, mx::array &&W) {
   Parameters.insert({Name, W});
   return Parameters.at(Name);
 }
@@ -38,7 +40,7 @@ std::shared_ptr<nn::Module> Module::toQuantized(
   }
   return shared_from_this();
 }
-void Module::apply(std::string Key, mx::array Value) {
+void Module::apply(std::string Key, const mx::array &Value) {
   std::vector<std::string> SplitKey = splitString(Key, '.');
   if (SplitKey.size() == 1) {
     if (Parameters.find(Key) == Parameters.end()) {
@@ -76,29 +78,17 @@ Module::getWeigts(const std::string &Prefix) {
 
 } // namespace mlx::core::nn
 
+// 64-bit FNV-1a over the array's row-major bytes.
 uint64_t fnv1aHash(const mx::array &X) {
-  std::string FileName = "./temp_array.npy";
-  mx::save(FileName.c_str(), X);
-  std::string Command = "python3.10 ../hash_script.py " + FileName;
-
-  FILE *Pipe = popen(Command.c_str(), "r");
-  if (!Pipe) {
-    throw std::runtime_error("Failed to open pipe for Python script.");
-  }
-
-  char Buffer[128];
-  std::string Result;
-  while (fgets(Buffer, sizeof(Buffer), Pipe) != nullptr) {
-    Result += Buffer;
-  }
-  pclose(Pipe);
-
-  try {
-    uint64_t HashVal = std::stoull(Result);
-    return HashVal;
-  } catch (const std::exception &e) {
-    throw std::runtime_error("Failed to parse hash value from Python output.");
+  mx::array Contiguous = mx::contiguous(X);
+  Contiguous.eval();
+  const auto *Bytes = Contiguous.data<uint8_t>();
+  uint64_t Hash = 14695981039346656037ULL;
+  for (size_t I = 0; I < Contiguous.nbytes(); ++I) {
+    Hash ^= Bytes[I];
+    Hash *= 1099511628211ULL;
   }
+  return Hash;
 }
 
 void debugArray(const mx::array &X, const std::string &Name) {

mlx/base.h

@@ -16,17 +16,18 @@ class Module : public std::enable_shared_from_this<Module> {
   std::string Name;
   std::unordered_map<std::string, mx::array> Parameters{};
   std::unordered_map<std::string, std::shared_ptr<Module>> Submodules{};
-  mx::array &registerParameter(std::string Name, mx::array &&W);
+  mx::array &registerParameter(const std::string &Name, mx::array &&W);
   std::unordered_map<std::string, mx::array>
   getWeigts(const std::string &Prefix = "model");
   virtual std::shared_ptr<nn::Module> toQuantized(
       int GroupSize = 64, int Bits = 4, const std::string &Prefix = "",
       const std::unordered_map<std::string, mx::array> &Parameters = {});
   virtual bool hasQuantize() { return false; }
   void update(std::unordered_map<std::string, mx::array> Parameters);
-  void apply(std::string Key, mx::array Parameters);
+  void apply(std::string Key, const mx::array &Parameters);
   template <typename T>
-  void registerModule(std::string ModuleName, std::shared_ptr<T> M) {
+  void registerModule(const std::string &ModuleName,
+                      const std::shared_ptr<T> &M) {
     using DecayedT = std::decay_t<T>;
     if (!std::is_base_of<Module, DecayedT>::value) {
       spdlog::error("Invalid subModule.");
@@ -42,7 +43,7 @@ class Module : public std::enable_shared_from_this<Module> {
     }
   }
   template <typename T>
-  void registerLayer(std::string ModuleName,
+  void registerLayer(const std::string &ModuleName,
                      std::vector<std::shared_ptr<T>> &Layers) {
     if (!std::is_base_of<Module, T>::value) {
       spdlog::error("Invalid subModule.");

mlx/convolution.cpp

@@ -7,7 +7,7 @@
 
 namespace mlx::core::nn {
 
-mx::array Conv1d::forward(mx::array Input) {
+mx::array Conv1d::forward(const mx::array &Input) {
   auto Y = mx::conv1d(Input, Parameters.at("weight"), Stride, Padding, Dilation,
                       Groups);
   if (Parameters.find("bias") != Parameters.end()) {
@@ -16,7 +16,7 @@ mx::array Conv1d::forward(mx::array Input) {
   return Y;
 }
 
-mx::array Conv2d::forward(mx::array Input) {
+mx::array Conv2d::forward(const mx::array &Input) {
   auto Y = mx::conv2d(Input, Parameters.at("weight"), Stride, Padding, Dilation,
                       Groups);
   if (Parameters.find("bias") != Parameters.end()) {

mlx/convolution.h

@@ -21,7 +21,7 @@ class Conv1d : public nn::Module {
       registerParameter("bias", mx::zeros({OutChannels}));
     }
   }
-  mx::array forward(mx::array Input);
+  mx::array forward(const mx::array &Input);
 };
 
 class Conv2d : public nn::Module {
@@ -50,7 +50,7 @@ class Conv2d : public nn::Module {
       registerParameter("bias", mx::zeros({OutChannels}));
     }
   }
-  mx::array forward(mx::array Input);
+  mx::array forward(const mx::array &Input);
 };
 
 } // namespace mlx::core::nn

mlx/embedding.cpp

@@ -9,7 +9,7 @@ namespace mlx::core::nn {
 mx::array Embedding::forward(mx::array Input) {
   return take(Parameters.at("weight"), Input, 0);
 }
-mx::array Embedding::asLinear(mx::array Input) {
+mx::array Embedding::asLinear(const mx::array &Input) {
   return matmul(Input, transpose(Parameters.at("weight")));
 }
 std::shared_ptr<nn::Module>

mlx/embedding.h

@@ -13,7 +13,7 @@ class Embedding : public Module {
                       mx::random::normal({NumEmbeddings, Dims}, 0.0, Scale));
   }
   virtual mx::array forward(mx::array Input);
-  mx::array asLinear(mx::array Input);
+  mx::array asLinear(const mx::array &Input);
   std::shared_ptr<nn::Module>
   toQuantized(int GroupSize = 64, int Bits = 4, const std::string &Prefix = "",
               const std::unordered_map<std::string, mx::array> &Parameters = {})

mlx/normalization.cpp

@@ -2,10 +2,10 @@
 #include <mlx/array.h>
 #include <mlx/fast.h>
 namespace mlx::core::nn {
-mx::array RMSNorm::forward(mx::array Input) {
+mx::array RMSNorm::forward(const mx::array &Input) {
   return mx::fast::rms_norm(Input, Parameters.at("weight"), Eps);
 }
-mx::array LayerNorm::forward(mx::array Input) {
+mx::array LayerNorm::forward(const mx::array &Input) {
   std::optional<mx::array> Weight = {};
   std::optional<mx::array> Bias = {};
   if (Parameters.find("weight") != Parameters.end()) {

mlx/normalization.h

@@ -10,7 +10,7 @@ class RMSNorm : public nn::Module {
   RMSNorm(int Dims, float Eps = 1e-5) : Eps(Eps) {
     registerParameter("weight", mx::ones({Dims}));
   }
-  mx::array forward(mx::array Input);
+  mx::array forward(const mx::array &Input);
 };
 
 class LayerNorm : public nn::Module {
@@ -27,7 +27,7 @@ class LayerNorm : public nn::Module {
       }
     }
   }
-  mx::array forward(mx::array Input);
+  mx::array forward(const mx::array &Input);
 };
 
 } // namespace mlx::core::nn

mlx/positional_encoding.cpp

@@ -1,7 +1,7 @@
 #include "positional_encoding.h"
 
 namespace mlx::core::nn {
-mx::array RoPE::forward(mx::array Input, int Offset) {
+mx::array RoPE::forward(const mx::array &Input, int Offset) {
   return mx::fast::rope(Input, Dims, Tranditional, Base, Scale, Offset);
 }
 

mlx/positional_encoding.h

@@ -14,7 +14,7 @@ class RoPE : public Module {
   RoPE(int Dims, bool Traditional = false, float Base = 10000,
        float Scale = 1.0)
       : Dims(Dims), Tranditional(Traditional), Base(Base), Scale(Scale) {}
-  mx::array forward(mx::array Input, int Offset = 0);
+  mx::array forward(const mx::array &Input, int Offset = 0);
 };
 
 } // namespace mlx::core::nn

mlx/quantized.cpp

@@ -22,9 +22,9 @@ mx::array QuantizedLinear::forward(mx::array Input) {
   }
   return Out;
 }
-std::shared_ptr<QuantizedEmbedding>
-QuantizedEmbedding::fromEmbedding(std::shared_ptr<Embedding> EmbeddingModule,
-                                  int GroupSize, int Bits) {
+std::shared_ptr<QuantizedEmbedding> QuantizedEmbedding::fromEmbedding(
+    const std::shared_ptr<Embedding> &EmbeddingModule, int GroupSize,
+    int Bits) {
   auto EmbeddingShape = EmbeddingModule->Parameters.at("weight").shape();
   auto QuantizedModel = std::make_shared<QuantizedEmbedding>(QuantizedEmbedding(
       EmbeddingShape[0], EmbeddingShape[1], GroupSize, Bits));
@@ -39,8 +39,8 @@ QuantizedEmbedding::fromEmbedding(std::shared_ptr<Embedding> EmbeddingModule,
   return QuantizedModel;
 }
 std::shared_ptr<QuantizedLinear>
-QuantizedLinear::fromLinear(std::shared_ptr<Linear> LinearModule, int GroupSize,
-                            int Bits) {
+QuantizedLinear::fromLinear(const std::shared_ptr<Linear> &LinearModule,
+                            int GroupSize, int Bits) {
   auto LinearShape = LinearModule->Parameters.at("weight").shape();
   auto OutputDims = LinearShape[0];
   auto InputDims = LinearShape[1];

mlx/quantized.h

@@ -28,8 +28,8 @@ class QuantizedEmbedding : public Embedding {
   }
   mx::array forward(mx::array Input) override;
   static std::shared_ptr<QuantizedEmbedding>
-  fromEmbedding(std::shared_ptr<Embedding> EmbeddingModule, int GroupSize = 64,
-                int Bits = 4);
+  fromEmbedding(const std::shared_ptr<Embedding> &EmbeddingModule,
+                int GroupSize = 64, int Bits = 4);
 };
 
 class QuantizedLinear : public Linear {
@@ -52,7 +52,7 @@ class QuantizedLinear : public Linear {
   }
   mx::array forward(mx::array Input) override;
   static std::shared_ptr<QuantizedLinear>
-  fromLinear(std::shared_ptr<Linear> LinearModule, int GroupSize = 64,
+  fromLinear(const std::shared_ptr<Linear> &LinearModule, int GroupSize = 64,
              int Bits = 4);
 };
 

model/converter.cpp

@@ -6,7 +6,7 @@
 #include <vector>
 
 std::unordered_map<std::string, mx::array>
-weightsToMlx(std::string WeightPath) {
+weightsToMlx(const std::string &WeightPath) {
   const std::filesystem::path Path(WeightPath);
   if (std::filesystem::is_directory(Path)) {
     std::unordered_map<std::string, mx::array> Loaded;
@@ -33,7 +33,7 @@ weightsToMlx(std::string WeightPath) {
 }
 
 std::unordered_map<std::string, mx::array>
-llamaToMlxllm(std::string WeightPath) {
+llamaToMlxllm(const std::string &WeightPath) {
   std::unordered_map<std::string, mx::array> ModelWeights;
   auto Weight = weightsToMlx(WeightPath);
   for (auto &[K, V] : Weight) {

model/converter.h

@@ -3,7 +3,8 @@
 #include "mlx/mlx.h"
 #include <cstring>
 
-std::unordered_map<std::string, mx::array> weightsToMlx(std::string WeightPath);
+std::unordered_map<std::string, mx::array>
+weightsToMlx(const std::string &WeightPath);
 
 std::unordered_map<std::string, mx::array>
-llamaToMlxllm(std::string WeightPath);
+llamaToMlxllm(const std::string &WeightPath);