From 723067f0c8ae144b7c70c7f664df4affe7ac950f Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 09:11:08 -0500
Subject: [PATCH 01/25] introduce FrozenKeyIdxBiMap data structure

---
 CMakePresets.json                          |   2 +-
 src/clib/CMakeLists.txt                    |   1 +
 src/clib/utils/FrozenKeyIdxBiMap.hpp       | 226 ++++++++++++++++++
 tests/unit/CMakeLists.txt                  |   4 +
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 262 +++++++++++++++++++++
 5 files changed, 494 insertions(+), 1 deletion(-)
 create mode 100644 src/clib/utils/FrozenKeyIdxBiMap.hpp
 create mode 100644 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp

diff --git a/CMakePresets.json b/CMakePresets.json
index b91a0cd74..037914b50 100644
--- a/CMakePresets.json
+++ b/CMakePresets.json
@@ -51,7 +51,7 @@
           "cacheVariables" : {
             "CMAKE_COMPILE_WARNING_AS_ERROR": "ON",
             "CMAKE_C_FLAGS": "-Wall -Wpedantic -Wextra -Wno-error=unused-variable -Wno-error=newline-eof -Wno-unused-parameter",
-            "CMAKE_CXX_FLAGS": "-Wall -Wpedantic -Wno-c++17-attribute-extensions -Wno-unused-parameter"
+            "CMAKE_CXX_FLAGS": "-Wall -Wpedantic -Wno-c++17-attribute-extensions -Wno-unused-parameter -Wno-error=self-assign"
           }
       },
       {
diff --git a/src/clib/CMakeLists.txt b/src/clib/CMakeLists.txt
index 6e3b757c8..f865f97e7 100644
--- a/src/clib/CMakeLists.txt
+++ b/src/clib/CMakeLists.txt
@@ -125,6 +125,7 @@ add_library(Grackle_Grackle
   solve_rate_cool_g-cpp.cpp solve_rate_cool_g-cpp.h
   step_rate_newton_raphson.hpp
   time_deriv_0d.hpp
+  utils/FrozenKeyIdxBiMap.hpp
   utils-cpp.cpp utils-cpp.hpp
   utils-field.hpp
   fortran_func_wrappers.hpp
diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/utils/FrozenKeyIdxBiMap.hpp
new file mode 100644
index 000000000..7bf0b2772
--- /dev/null
+++ b/src/clib/utils/FrozenKeyIdxBiMap.hpp
@@ -0,0 +1,226 @@
+//===----------------------------------------------------------------------===//
+//
+// See the LICENSE file for license and copyright information
+// SPDX-License-Identifier: NCSA AND BSD-3-Clause
+//
+//===----------------------------------------------------------------------===//
+///
+/// @file
+/// Declares the internal FrozenKeyIdxBiMap type
+///
+/// All insertions occur during initialization. This simplifies a lot of
+/// bookkeeping.
+///
+/// The underlying implementation of this type is *highly* suboptimal (it's
+/// simplistic at the cost of speed). PR #270 introduce a drop-in replacement
+/// that maintains the exact same API
+///
+//===----------------------------------------------------------------------===//
+#ifndef UTILS_FROZENKEYIDXBIMAP_HPP
+#define UTILS_FROZENKEYIDXBIMAP_HPP
+
+#include <cstdint>
+#include <cstdlib>
+#include <cstring>
+#include <limits>
+
+#include "grackle.h"
+#include "status_reporting.h"
+
+// the motivation for these constants are provided in PR #270 (they are related
+// to some optimizations in the FrozenKeyIdxBiMap implementation)
+namespace grackle::impl::bimap {
+/// specifies an invalid value of the map (we state that you can't store the
+/// maximum u16 value)
+inline constexpr std::uint16_t invalid_val =
+    std::numeric_limits<std::uint16_t>::max();
+
+/// specifies maximum allowed length of a key (excluding the null character).
+inline constexpr std::uint16_t keylen_max = 29;
+}
+
+// these are just here for to make it easier for us to adopt changes from PR
+// #270 (then we can delete these macros)
+#define STRU16MAP_INVALID_VAL grackle::impl::bimap::invalid_val
+#define STRU16MAP_KEYLEN_MAX grackle::impl::bimap::keylen_max
+
+namespace grackle::impl {
+
+enum class BiMapMode {
+  REFS_KEYDATA = 0,
+  COPIES_KEYDATA = 1
+};
+
+/// @brief This is a bidirectional map (bimap). It is specialized to map `n`
+/// unique string keys to unique indexes with values of `0` through `n-1` and
+/// vice versa. The ordering of keys is set at initialization and frozen.
+///
+/// This is primarily intended to be used in the implementation of Maps of
+/// arrays (where the values could be part of a single contiguous array or are
+/// individual arrays), but this but may be broadly useful for other
+/// applications.
+///
+/// This operates in 2 modes:
+/// 1. @ref BiMapMode::REFS_KEYDATA This is the default, where we operate
+///    under the assumption that the allocations holding the string characters
+///    outlive  the bimap. In this mode the bimap is intended to hold
+///    string-literals. (which are live for the entirety of a program). This
+///    minimizes memory usage.
+/// 2. @ref BiMapMode::COPIES_KEYDATA Under this mode, the bimap copies the
+///    data of all keys. This is useful for testing purposes. In the long-term,
+///    if we allow dynamic extension of chemistry networks, it will also be
+///    useful. If we are implement the optimizations described down below
+///    (where we directly embed the string in the hash-table-rows), this will
+///    probably be a quite a bit faster
+///
+/// Replacement in PR #270
+/// ======================
+/// The current implementation is extremely oversimplified and inefficient! It
+/// doesn't even use a hash table. The purpose is to create a simple abstract
+/// data structure for which the implementation will be dramatically improved
+/// by PR #270 (but the interface won't be touched at all).
+///
+/// The PR with the improved version, also updates this docstring with a
+/// detailed explanation of design decisions (like why the contents
+/// are "frozen") and highlights a number of potential improvements.
+///
+/// > [!note]
+/// > The contents of this struct should be considered an implementation
+/// > detail! Always prefer the associated functions (they are defined in such
+/// > a way that they should be inlined
+struct FrozenKeyIdxBiMap{
+  // don't forget to update FrozenKeyIdxBiMap_clone when changing members
+
+  /// the number of contained strings
+  int length;
+  /// array of keys
+  const char** keys;
+  /// indicates whether the map "owns" the memory holding the characters in
+  /// each key or just references it
+  BiMapMode mode;
+};
+
+/// Constructs a new FrozenKeyIdxBiMap
+///
+/// @param[out] out Pointer where the allocated type is stored
+/// @param[in]  keys Sequence of 1 or more unique strings. Each string must
+///     include at least 1 non-null character and be null-terminated
+/// @param[in]  key_count The length of keys
+/// @param[in]  mode specifies handling of keys. This will be passed on to any
+///     clones that are made.
+inline int new_FrozenKeyIdxBiMap(
+  FrozenKeyIdxBiMap** out, const char* keys[], int key_count,
+  BiMapMode mode
+) {
+
+  // check the specified keys
+  long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
+  if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
+    return GrPrintAndReturnErr(
+      "key_count must be positive and cannot exceed %lld", max_keys
+    );
+  } else if (keys == nullptr) {
+    return GrPrintAndReturnErr("keys must not be a nullptr");
+  }
+  for (int i = 0; i < key_count; i++) {
+    GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
+    std::size_t n_chrs_without_nul = std::strlen(keys[i]);
+    if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap::keylen_max) {
+      return GrPrintAndReturnErr(
+        "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
+        "exceeding %d",
+        keys[i], i, bimap::keylen_max
+      );
+    }
+    // check uniqueness
+    for (int j = 0; j < i; j++) {
+      if (strcmp(keys[i], keys[j]) == 0) {
+        return GrPrintAndReturnErr("\"%s\" key repeats", keys[i]);
+      }
+    }
+  }
+
+  // now, actually construct the result
+  const char** out_keys = nullptr;
+  switch (mode) {
+    case BiMapMode::REFS_KEYDATA: {
+      out_keys = new const char*[key_count];
+      for (int i = 0; i < key_count; i++) {
+        out_keys[i] = keys[i];
+      }
+      break;
+    }
+    case BiMapMode::COPIES_KEYDATA: {
+      char** tmp_keys = new char*[key_count];
+      for (int i = 0; i < key_count; i++) {
+        std::size_t n_chrs_without_nul = std::strlen(keys[i]);
+        tmp_keys[i] = new char[n_chrs_without_nul + 1];
+        std::memcpy(tmp_keys[i], keys[i], n_chrs_without_nul + 1);
+      }
+      out_keys = (const char**)tmp_keys;
+      break;
+    }
+    default: return GrPrintAndReturnErr("unknown mode");
+  }
+
+  (*out) = new FrozenKeyIdxBiMap;
+  (*out)->length = key_count;
+  (*out)->keys = out_keys;
+
+  return GR_SUCCESS;
+}
+
+/// Destroys the specified FrozenKeyIdxBiMap
+inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
+  if (ptr->mode == BiMapMode::COPIES_KEYDATA) {
+    for (int i = 0; i < ptr->length; i++) {
+      delete[] ptr->keys[i];
+    }
+  }
+  delete[] ptr->keys;
+  delete ptr;
+}
+
+
+/// Makes a clone of the specified FrozenKeyIdxBiMap (the clone inherites the
+/// original BiMapMode).
+int FrozenKeyIdxBiMap_clone(FrozenKeyIdxBiMap** out,
+                            const FrozenKeyIdxBiMap* ptr) {
+  return new_FrozenKeyIdxBiMap(out, ptr->keys, ptr->length, ptr->mode);
+};
+
+/// returns the value associated with the key or (if the key can't be found)
+/// @ref grackle::impl::bimap::invalid_val
+inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
+  const FrozenKeyIdxBiMap* map, const char* key
+)
+{
+  GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
+  for (int i = 0; i < map->length; i++) {
+    if (std::strcmp(map->keys[i], key) == 0) {
+      return static_cast<std::uint16_t>(i);
+    }
+  }
+  return bimap::invalid_val;
+}
+
+/// checks if the map contains a key
+inline int FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
+                                      const char* key) {
+  return FrozenKeyIdxBiMap_idx_from_key(map, key) != bimap::invalid_val;
+}
+
+inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map)
+{ return map->length; }
+
+/// Return the ith key (this is effectively a reverse lookup)
+inline const char* FrozenKeyIdxBiMap_key_from_idx(
+  const FrozenKeyIdxBiMap* map, std::uint16_t i
+) {
+  if (i >= map->length) { return nullptr; }
+  return map->keys[i]; // this can't be a nullptr
+}
+
+}  // namespace grackle::impl
+
+#endif  // UTILS_FROZENKEYIDXBIMAP_HPP
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
index 9e05050ba..b6c9e0ba1 100644
--- a/tests/unit/CMakeLists.txt
+++ b/tests/unit/CMakeLists.txt
@@ -48,6 +48,10 @@ target_compile_definitions(grtest_utils
 
 # start declaring targets for tests
 # ---------------------------------
+add_executable(testFrozenKeyIdxBiMap test_unit_FrozenKeyIdxBiMap.cpp)
+target_link_libraries(testFrozenKeyIdxBiMap testdeps)
+gtest_discover_tests(testFrozenKeyIdxBiMap)
+
 add_executable(runInterpolationTests test_unit_interpolators_g.cpp)
 target_link_libraries(runInterpolationTests testdeps)
 
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
new file mode 100644
index 000000000..83aabc0ca
--- /dev/null
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -0,0 +1,262 @@
+//===----------------------------------------------------------------------===//
+//
+// See the LICENSE file for license and copyright information
+// SPDX-License-Identifier: NCSA AND BSD-3-Clause
+//
+//===----------------------------------------------------------------------===//
+///
+/// @file
+/// tests the @ref grackle::impl::FrozenKeyIdxBiMap
+///
+//===----------------------------------------------------------------------===//
+
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+#include "utils/FrozenKeyIdxBiMap.hpp"
+#include "grackle.h"
+
+class FrozenKeyIdxBiMapConstructorSuite :
+  public testing::TestWithParam<grackle::impl::BiMapMode> {
+  // You can implement all the usual fixture class members here.
+  // To access the test parameter, call GetParam() from class
+  // TestWithParam<T>.
+
+};
+
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, Simple) {
+  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
+  const char* keys[] = {"denisty", "internal_energy"};
+
+  EXPECT_EQ(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
+  ASSERT_NE(tmp, nullptr);
+  grackle::impl::drop_FrozenKeyIdxBiMap(tmp);
+}
+
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
+  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
+
+  const char* first_key = "density";
+  std::string long_key(STRU16MAP_KEYLEN_MAX, 'A');
+  const char* keys[2] = {first_key, long_key.data()};
+  EXPECT_EQ(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
+  ASSERT_NE(tmp, nullptr);
+  grackle::impl::drop_FrozenKeyIdxBiMap(tmp);
+}
+
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, TooLongKey) {
+  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
+
+  const char* first_key = "density";
+  std::string long_key(STRU16MAP_KEYLEN_MAX+1, 'A');
+  const char* keys[2] = {first_key, long_key.data()};
+  EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
+  ASSERT_EQ(tmp, nullptr);
+}
+
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, 0LenKey) {
+  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
+
+  const char* keys[2] = {"density",""};
+  EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
+  ASSERT_EQ(tmp, nullptr);
+}
+
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, NoKeys) {
+  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
+
+  EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, nullptr, 0, GetParam()), GR_SUCCESS);
+  ASSERT_EQ(tmp, nullptr); // if this fails, we'll leak memory (but not much
+                           // can be done)
+}
+
+INSTANTIATE_TEST_SUITE_P(
+  , /* <- leaving Instantiation name empty */
+  FrozenKeyIdxBiMapConstructorSuite,
+  testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA, grackle::impl::BiMapMode::COPIES_KEYDATA),
+  [](const testing::TestParamInfo<FrozenKeyIdxBiMapConstructorSuite::ParamType>& info){
+    if (info.param == grackle::impl::BiMapMode::REFS_KEYDATA) {
+      return std::string("BIMAP_REFS_KEYDATA");
+    } else {
+      return std::string("BIMAP_COPIES_KEYDATA");
+    }
+  }
+);
+
+
+/// helper function to initialize a map from a vector
+int new_FrozenKeyIdxBiMap(grackle::impl::FrozenKeyIdxBiMap** out,
+                          const std::vector<std::string>& vec_,
+                          grackle::impl::BiMapMode mode)
+{
+  std::size_t key_count = vec_.size();
+
+  // create a vector of pointers
+  std::vector<const char*> key_ptr_l(key_count, nullptr);
+  for (std::size_t i = 0; i < key_count; i++) {
+    key_ptr_l[i] = vec_[i].c_str();
+  }
+
+  return new_FrozenKeyIdxBiMap(out, key_ptr_l.data(), key_count, mode);
+}
+
+class FrozenKeyIdxBiMapGeneralSuite :
+  public testing::TestWithParam<grackle::impl::BiMapMode>
+{
+protected:
+  std::vector<std::string> ordered_keys;
+  grackle::impl::FrozenKeyIdxBiMap* bimap_p = nullptr;
+
+  // we use SetUp/Teardown instead of constructor and destructor so we can
+  // perform some sanity checks with ASSERTIONs
+  void SetUp() override {
+    ordered_keys = std::vector<std::string>{
+      "internal_energy", "density", "metal_density"
+    };
+
+    grackle::impl::BiMapMode mode = this->GetParam();
+    ASSERT_EQ(
+        new_FrozenKeyIdxBiMap(&this->bimap_p, ordered_keys, mode),
+        GR_SUCCESS
+    );
+
+  }
+
+  void TearDown() override {
+    if (bimap_p != nullptr) { grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p); }
+  }
+
+  bool ReusesOriginalKeyPtrs(const grackle::impl::FrozenKeyIdxBiMap* p) const {
+    for (int i = 0; i < 3; i++) {
+      const char* orig_key_ptr = ordered_keys[i].c_str();
+      if (grackle::impl::FrozenKeyIdxBiMap_key_from_idx(p, i) != orig_key_ptr) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+};
+
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_ContainedKey) {
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "density"),
+    1
+  );
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "internal_energy"),
+    0
+  );
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "metal_density"),
+    2
+  );
+}
+
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_AbsentKey) {
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
+    STRU16MAP_INVALID_VAL
+  );
+}
+
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_AbsentIrregularKeys) {
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
+    STRU16MAP_INVALID_VAL
+  );
+
+  std::string key(STRU16MAP_KEYLEN_MAX+1, 'A');
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, key.data()),
+    STRU16MAP_INVALID_VAL
+  );
+}
+
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdx_InvalidIdx) {
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 3), nullptr);
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p,
+                                                  STRU16MAP_INVALID_VAL),
+    nullptr
+  );
+}
+
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdx_ValidIdx) {
+  EXPECT_EQ(
+    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 2)),
+    std::string("metal_density")
+  );
+  EXPECT_EQ(
+    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 1)),
+    std::string("density")
+  );
+  EXPECT_EQ(
+    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 0)),
+    std::string("internal_energy")
+  );
+
+  // check whether the bimap is using pointers to the keys used during init
+  if (GetParam() == grackle::impl::BiMapMode::REFS_KEYDATA) {
+    EXPECT_TRUE(ReusesOriginalKeyPtrs(bimap_p));
+  } else {
+    EXPECT_FALSE(ReusesOriginalKeyPtrs(bimap_p));
+  }
+}
+
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
+  grackle::impl::FrozenKeyIdxBiMap* clone_p = nullptr;
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_clone(&clone_p, bimap_p),
+    GR_SUCCESS
+  );
+  ASSERT_NE(clone_p, nullptr);
+
+  // for the sake of robustly checking everything, we delete bimap_p
+  grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p);
+  bimap_p = nullptr;
+
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "internal_energy"),
+    0
+  );
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "notAKey"),
+    STRU16MAP_INVALID_VAL
+  );
+
+  EXPECT_EQ(
+    grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 3),
+    nullptr
+  );
+  EXPECT_EQ(
+    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 1)),
+    std::string("density")
+  );
+
+  // check whether the clone is using pointers to the keys used during init
+  if (GetParam() == grackle::impl::BiMapMode::REFS_KEYDATA) {
+    EXPECT_TRUE(ReusesOriginalKeyPtrs(clone_p));
+  } else {
+    EXPECT_FALSE(ReusesOriginalKeyPtrs(clone_p));
+  }
+
+  // finally, cleanup the clone
+  grackle::impl::drop_FrozenKeyIdxBiMap(clone_p);
+}
+
+
+INSTANTIATE_TEST_SUITE_P(
+  , /* <- leaving Instantiation name empty */
+  FrozenKeyIdxBiMapGeneralSuite,
+  testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
+                  grackle::impl::BiMapMode::COPIES_KEYDATA),
+  [](const testing::TestParamInfo<FrozenKeyIdxBiMapGeneralSuite::ParamType>& info){
+    if (info.param == grackle::impl::BiMapMode::REFS_KEYDATA) {
+      return std::string("BIMAP_REFS_KEYDATA");
+    } else {
+      return std::string("BIMAP_COPIES_KEYDATA");
+    }
+  }
+);

From cdecbb5222583ef7cf0b6f18d2ea8b1cf59f2a8d Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 09:18:30 -0500
Subject: [PATCH 02/25] address clang-tidy issues

---
 .clang-tidy                                |  1 -
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 10 +++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/.clang-tidy b/.clang-tidy
index 34b430c8a..fc03db36d 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -84,7 +84,6 @@ Checks: >
   readability-reference-to-constructed-temporary,
   readability-simplify-boolean-expr,
   readability-simplify-subscript-expr,
-  readability-static-accessed-through-instance,
   readability-string-compare,
   readability-uniqueptr-delete-release,
 
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 83aabc0ca..b645b3e50 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -139,7 +139,7 @@ class FrozenKeyIdxBiMapGeneralSuite :
 
 };
 
-TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_ContainedKey) {
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "density"),
     1
@@ -154,14 +154,14 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_ContainedKey) {
   );
 }
 
-TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_AbsentKey) {
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
     STRU16MAP_INVALID_VAL
   );
 }
 
-TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_AbsentIrregularKeys) {
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
     STRU16MAP_INVALID_VAL
@@ -174,7 +174,7 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKey_AbsentIrregularKeys) {
   );
 }
 
-TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdx_InvalidIdx) {
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 3), nullptr);
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p,
@@ -183,7 +183,7 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdx_InvalidIdx) {
   );
 }
 
-TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdx_ValidIdx) {
+TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
   EXPECT_EQ(
     std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 2)),
     std::string("metal_density")

From ce3c40471e71f14f58bed610ab552f20ea3a9ab5 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 09:25:08 -0500
Subject: [PATCH 03/25] a few minor tweaks

---
 src/clib/utils/FrozenKeyIdxBiMap.hpp       |  3 +++
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 18 +++++++++---------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/utils/FrozenKeyIdxBiMap.hpp
index 7bf0b2772..093be15e5 100644
--- a/src/clib/utils/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/utils/FrozenKeyIdxBiMap.hpp
@@ -15,6 +15,9 @@
 /// simplistic at the cost of speed). PR #270 introduce a drop-in replacement
 /// that maintains the exact same API
 ///
+/// If we decide to more fully embrace C++, it would make a lot of sense to
+/// convert this to a full-blown class.
+///
 //===----------------------------------------------------------------------===//
 #ifndef UTILS_FROZENKEYIDXBIMAP_HPP
 #define UTILS_FROZENKEYIDXBIMAP_HPP
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index b645b3e50..94a977e9d 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -38,7 +38,7 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
   grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
 
   const char* first_key = "density";
-  std::string long_key(STRU16MAP_KEYLEN_MAX, 'A');
+  std::string long_key(grackle::impl::bimap::keylen_max, 'A');
   const char* keys[2] = {first_key, long_key.data()};
   EXPECT_EQ(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
   ASSERT_NE(tmp, nullptr);
@@ -49,7 +49,7 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, TooLongKey) {
   grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
 
   const char* first_key = "density";
-  std::string long_key(STRU16MAP_KEYLEN_MAX+1, 'A');
+  std::string long_key(grackle::impl::bimap::keylen_max+1, 'A');
   const char* keys[2] = {first_key, long_key.data()};
   EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
   ASSERT_EQ(tmp, nullptr);
@@ -157,28 +157,28 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
-    STRU16MAP_INVALID_VAL
+    grackle::impl::bimap::invalid_val
   );
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
-    STRU16MAP_INVALID_VAL
+    grackle::impl::bimap::invalid_val
   );
 
-  std::string key(STRU16MAP_KEYLEN_MAX+1, 'A');
+  std::string key(grackle::impl::bimap::keylen_max+1, 'A');
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, key.data()),
-    STRU16MAP_INVALID_VAL
+    grackle::impl::bimap::invalid_val
   );
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 3), nullptr);
   EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p,
-                                                  STRU16MAP_INVALID_VAL),
+    grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
+      bimap_p, grackle::impl::bimap::invalid_val),
     nullptr
   );
 }
@@ -223,7 +223,7 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   );
   EXPECT_EQ(
     grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "notAKey"),
-    STRU16MAP_INVALID_VAL
+    grackle::impl::bimap::invalid_val
   );
 
   EXPECT_EQ(

From 660431f6aeecedf733365ae790b3d50a252f7cdd Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 10:46:05 -0500
Subject: [PATCH 04/25] Refactor for consistency with other internal types

There is some unforunate consequences, but if we don't do this you end
up with some pretty confusing looking code... (the confusion primarily
arises if new_FrozenKeyIdxBiMap or drop_FrozenKeyIdxBiMap has different
behavior from other functions with similar names). I really think it
would be better to make this into a simple C++ class with a constructor
and destructor, but thats a topic for another time
---
 src/clib/utils/FrozenKeyIdxBiMap.hpp       | 73 +++++++++++++++-------
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 73 +++++++++++-----------
 2 files changed, 88 insertions(+), 58 deletions(-)

diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/utils/FrozenKeyIdxBiMap.hpp
index 093be15e5..a7eb21a37 100644
--- a/src/clib/utils/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/utils/FrozenKeyIdxBiMap.hpp
@@ -12,11 +12,14 @@
 /// bookkeeping.
 ///
 /// The underlying implementation of this type is *highly* suboptimal (it's
-/// simplistic at the cost of speed). PR #270 introduce a drop-in replacement
-/// that maintains the exact same API
+/// simplistic at the cost of speed). PR #270 introduce a replacement that
+/// maintains almost the exact same API (the new_FrozenKeyIdxBiMap,
+/// FrozenKeyIdxBiMap_clone, and drop_FrozenKeyIdxBiMap functions will need to
+/// be tweaked)
 ///
-/// If we decide to more fully embrace C++, it would make a lot of sense to
-/// convert this to a full-blown class.
+/// If we decide to more fully embrace C++, it would make a LOT of sense to
+/// convert this to a full-blown class (and delete copy constructor/copy
+/// assignement OR adopt reference counting).
 ///
 //===----------------------------------------------------------------------===//
 #ifndef UTILS_FROZENKEYIDXBIMAP_HPP
@@ -105,40 +108,51 @@ struct FrozenKeyIdxBiMap{
 
 /// Constructs a new FrozenKeyIdxBiMap
 ///
-/// @param[out] out Pointer where the allocated type is stored
 /// @param[in]  keys Sequence of 1 or more unique strings. Each string must
 ///     include at least 1 non-null character and be null-terminated
 /// @param[in]  key_count The length of keys
 /// @param[in]  mode specifies handling of keys. This will be passed on to any
 ///     clones that are made.
-inline int new_FrozenKeyIdxBiMap(
-  FrozenKeyIdxBiMap** out, const char* keys[], int key_count,
-  BiMapMode mode
+///
+/// > [!note]
+/// > If this function returns `bimap`, then the caller should invoke
+/// > `FrozenKeyIdxBiMap_is_ok(&bimap)` to test whether there was an error.
+/// > This is pretty ugly/clunky, but its the only practical way to achieve
+/// > comparable behavior to other internal datatypes (ideally, we would make
+/// > this a simple C++ class instead)
+inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
+  const char* keys[], int key_count, BiMapMode mode
 ) {
+  // this will be returned if there is an error
+  FrozenKeyIdxBiMap erroneous_obj{0, nullptr, BiMapMode::REFS_KEYDATA};
 
   // check the specified keys
   long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
   if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
-    return GrPrintAndReturnErr(
+    GrPrintErrMsg(
       "key_count must be positive and cannot exceed %lld", max_keys
     );
+    return erroneous_obj;
   } else if (keys == nullptr) {
-    return GrPrintAndReturnErr("keys must not be a nullptr");
+    GrPrintErrMsg("keys must not be a nullptr");
+    return erroneous_obj;
   }
   for (int i = 0; i < key_count; i++) {
     GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
     std::size_t n_chrs_without_nul = std::strlen(keys[i]);
     if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap::keylen_max) {
-      return GrPrintAndReturnErr(
+      GrPrintErrMsg(
         "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
         "exceeding %d",
         keys[i], i, bimap::keylen_max
       );
+      return erroneous_obj;
     }
     // check uniqueness
     for (int j = 0; j < i; j++) {
       if (strcmp(keys[i], keys[j]) == 0) {
-        return GrPrintAndReturnErr("\"%s\" key repeats", keys[i]);
+        GrPrintErrMsg("\"%s\" key repeats", keys[i]);
+        return erroneous_obj;
       }
     }
   }
@@ -163,14 +177,22 @@ inline int new_FrozenKeyIdxBiMap(
       out_keys = (const char**)tmp_keys;
       break;
     }
-    default: return GrPrintAndReturnErr("unknown mode");
+    default: {
+      GrPrintErrMsg("unknown mode");
+      return erroneous_obj;
+    }
   }
 
-  (*out) = new FrozenKeyIdxBiMap;
-  (*out)->length = key_count;
-  (*out)->keys = out_keys;
+  return FrozenKeyIdxBiMap{
+    /* length = */ key_count,
+    /* keys = */ out_keys,
+    /* mode = */ mode
+  };
+}
 
-  return GR_SUCCESS;
+/// returns whether new_FrozenKeyIdxBiMap constructed a valid object
+inline bool FrozenKeyIdxBiMap_is_ok(FrozenKeyIdxBiMap* ptr) {
+  return (ptr->length > 0);
 }
 
 /// Destroys the specified FrozenKeyIdxBiMap
@@ -180,16 +202,23 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
       delete[] ptr->keys[i];
     }
   }
-  delete[] ptr->keys;
-  delete ptr;
+  if (ptr->keys != nullptr) {
+    delete[] ptr->keys;
+  }
 }
 
 
 /// Makes a clone of the specified FrozenKeyIdxBiMap (the clone inherites the
 /// original BiMapMode).
-int FrozenKeyIdxBiMap_clone(FrozenKeyIdxBiMap** out,
-                            const FrozenKeyIdxBiMap* ptr) {
-  return new_FrozenKeyIdxBiMap(out, ptr->keys, ptr->length, ptr->mode);
+///
+/// > [!note]
+/// > If this function returns `bimap`, then the caller should invoke
+/// > `FrozenKeyIdxBiMap_is_ok(&bimap)` to test whether there was an error.
+/// > This is pretty ugly/clunky, but its the only practical way to achieve
+/// > comparable behavior to other internal datatypes (ideally, we would make
+/// > this a simple C++ class instead)
+FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
+  return new_FrozenKeyIdxBiMap(ptr->keys, ptr->length, ptr->mode);
 };
 
 /// returns the value associated with the key or (if the key can't be found)
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 94a977e9d..8bfe41a68 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -26,49 +26,49 @@ class FrozenKeyIdxBiMapConstructorSuite :
 };
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, Simple) {
-  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
   const char* keys[] = {"denisty", "internal_energy"};
 
-  EXPECT_EQ(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
-  ASSERT_NE(tmp, nullptr);
-  grackle::impl::drop_FrozenKeyIdxBiMap(tmp);
+  grackle::impl::FrozenKeyIdxBiMap tmp
+    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+
+  EXPECT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
+  grackle::impl::drop_FrozenKeyIdxBiMap(&tmp);
 }
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
-  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
-
   const char* first_key = "density";
   std::string long_key(grackle::impl::bimap::keylen_max, 'A');
   const char* keys[2] = {first_key, long_key.data()};
-  EXPECT_EQ(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
-  ASSERT_NE(tmp, nullptr);
-  grackle::impl::drop_FrozenKeyIdxBiMap(tmp);
+
+  grackle::impl::FrozenKeyIdxBiMap tmp
+    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+
+  ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
+  grackle::impl::drop_FrozenKeyIdxBiMap(&tmp);
 }
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, TooLongKey) {
-  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
-
   const char* first_key = "density";
   std::string long_key(grackle::impl::bimap::keylen_max+1, 'A');
   const char* keys[2] = {first_key, long_key.data()};
-  EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
-  ASSERT_EQ(tmp, nullptr);
+
+  grackle::impl::FrozenKeyIdxBiMap tmp
+    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+  ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, 0LenKey) {
-  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
-
   const char* keys[2] = {"density",""};
-  EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, keys, 2, GetParam()), GR_SUCCESS);
-  ASSERT_EQ(tmp, nullptr);
+  grackle::impl::FrozenKeyIdxBiMap tmp
+    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+  ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, NoKeys) {
-  grackle::impl::FrozenKeyIdxBiMap* tmp = nullptr;
 
-  EXPECT_NE(new_FrozenKeyIdxBiMap(&tmp, nullptr, 0, GetParam()), GR_SUCCESS);
-  ASSERT_EQ(tmp, nullptr); // if this fails, we'll leak memory (but not much
-                           // can be done)
+  grackle::impl::FrozenKeyIdxBiMap tmp
+    = grackle::impl::new_FrozenKeyIdxBiMap(nullptr, 0, GetParam());
+  ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
 INSTANTIATE_TEST_SUITE_P(
@@ -86,9 +86,8 @@ INSTANTIATE_TEST_SUITE_P(
 
 
 /// helper function to initialize a map from a vector
-int new_FrozenKeyIdxBiMap(grackle::impl::FrozenKeyIdxBiMap** out,
-                          const std::vector<std::string>& vec_,
-                          grackle::impl::BiMapMode mode)
+grackle::impl::FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
+  const std::vector<std::string>& vec_, grackle::impl::BiMapMode mode)
 {
   std::size_t key_count = vec_.size();
 
@@ -98,7 +97,7 @@ int new_FrozenKeyIdxBiMap(grackle::impl::FrozenKeyIdxBiMap** out,
     key_ptr_l[i] = vec_[i].c_str();
   }
 
-  return new_FrozenKeyIdxBiMap(out, key_ptr_l.data(), key_count, mode);
+  return new_FrozenKeyIdxBiMap(key_ptr_l.data(), key_count, mode);
 }
 
 class FrozenKeyIdxBiMapGeneralSuite :
@@ -116,15 +115,19 @@ class FrozenKeyIdxBiMapGeneralSuite :
     };
 
     grackle::impl::BiMapMode mode = this->GetParam();
-    ASSERT_EQ(
-        new_FrozenKeyIdxBiMap(&this->bimap_p, ordered_keys, mode),
-        GR_SUCCESS
-    );
+    grackle::impl::FrozenKeyIdxBiMap tmp
+      = new_FrozenKeyIdxBiMap(ordered_keys, mode);
+    ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 
+    bimap_p = new grackle::impl::FrozenKeyIdxBiMap;
+    (*bimap_p) = tmp;
   }
 
   void TearDown() override {
-    if (bimap_p != nullptr) { grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p); }
+    if (bimap_p != nullptr) {
+      grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p);
+      delete bimap_p;
+    }
   }
 
   bool ReusesOriginalKeyPtrs(const grackle::impl::FrozenKeyIdxBiMap* p) const {
@@ -206,12 +209,10 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
-  grackle::impl::FrozenKeyIdxBiMap* clone_p = nullptr;
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_clone(&clone_p, bimap_p),
-    GR_SUCCESS
-  );
-  ASSERT_NE(clone_p, nullptr);
+  grackle::impl::FrozenKeyIdxBiMap clone = 
+    grackle::impl::FrozenKeyIdxBiMap_clone(bimap_p);
+  ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&clone));
+  grackle::impl::FrozenKeyIdxBiMap* clone_p = &clone;
 
   // for the sake of robustly checking everything, we delete bimap_p
   grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p);

From 1f42941e364f142eb27ade9ad9e88d29ff7c897c Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 10:51:08 -0500
Subject: [PATCH 05/25] restore clang-tidy setting from a few commits back

---
 .clang-tidy                                | 1 +
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.clang-tidy b/.clang-tidy
index fc03db36d..34b430c8a 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -84,6 +84,7 @@ Checks: >
   readability-reference-to-constructed-temporary,
   readability-simplify-boolean-expr,
   readability-simplify-subscript-expr,
+  readability-static-accessed-through-instance,
   readability-string-compare,
   readability-uniqueptr-delete-release,
 
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 8bfe41a68..dfff5ba99 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -114,7 +114,7 @@ class FrozenKeyIdxBiMapGeneralSuite :
       "internal_energy", "density", "metal_density"
     };
 
-    grackle::impl::BiMapMode mode = this->GetParam();
+    grackle::impl::BiMapMode mode = GetParam();
     grackle::impl::FrozenKeyIdxBiMap tmp
       = new_FrozenKeyIdxBiMap(ordered_keys, mode);
     ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));

From a6aa793ff5c1eb81d827e5dda4924d5df70ad0ed Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 10:53:21 -0500
Subject: [PATCH 06/25] apply clang-format

---
 src/clib/utils/FrozenKeyIdxBiMap.hpp       |  55 +++----
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 181 +++++++++------------
 2 files changed, 99 insertions(+), 137 deletions(-)

diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/utils/FrozenKeyIdxBiMap.hpp
index a7eb21a37..c884c9bbf 100644
--- a/src/clib/utils/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/utils/FrozenKeyIdxBiMap.hpp
@@ -43,7 +43,7 @@ inline constexpr std::uint16_t invalid_val =
 
 /// specifies maximum allowed length of a key (excluding the null character).
 inline constexpr std::uint16_t keylen_max = 29;
-}
+}  // namespace grackle::impl::bimap
 
 // these are just here for to make it easier for us to adopt changes from PR
 // #270 (then we can delete these macros)
@@ -52,10 +52,7 @@ inline constexpr std::uint16_t keylen_max = 29;
 
 namespace grackle::impl {
 
-enum class BiMapMode {
-  REFS_KEYDATA = 0,
-  COPIES_KEYDATA = 1
-};
+enum class BiMapMode { REFS_KEYDATA = 0, COPIES_KEYDATA = 1 };
 
 /// @brief This is a bidirectional map (bimap). It is specialized to map `n`
 /// unique string keys to unique indexes with values of `0` through `n-1` and
@@ -94,7 +91,7 @@ enum class BiMapMode {
 /// > The contents of this struct should be considered an implementation
 /// > detail! Always prefer the associated functions (they are defined in such
 /// > a way that they should be inlined
-struct FrozenKeyIdxBiMap{
+struct FrozenKeyIdxBiMap {
   // don't forget to update FrozenKeyIdxBiMap_clone when changing members
 
   /// the number of contained strings
@@ -120,18 +117,16 @@ struct FrozenKeyIdxBiMap{
 /// > This is pretty ugly/clunky, but its the only practical way to achieve
 /// > comparable behavior to other internal datatypes (ideally, we would make
 /// > this a simple C++ class instead)
-inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
-  const char* keys[], int key_count, BiMapMode mode
-) {
+inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
+                                               int key_count, BiMapMode mode) {
   // this will be returned if there is an error
   FrozenKeyIdxBiMap erroneous_obj{0, nullptr, BiMapMode::REFS_KEYDATA};
 
   // check the specified keys
   long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
   if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
-    GrPrintErrMsg(
-      "key_count must be positive and cannot exceed %lld", max_keys
-    );
+    GrPrintErrMsg("key_count must be positive and cannot exceed %lld",
+                  max_keys);
     return erroneous_obj;
   } else if (keys == nullptr) {
     GrPrintErrMsg("keys must not be a nullptr");
@@ -142,10 +137,9 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
     std::size_t n_chrs_without_nul = std::strlen(keys[i]);
     if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap::keylen_max) {
       GrPrintErrMsg(
-        "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
-        "exceeding %d",
-        keys[i], i, bimap::keylen_max
-      );
+          "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
+          "exceeding %d",
+          keys[i], i, bimap::keylen_max);
       return erroneous_obj;
     }
     // check uniqueness
@@ -183,11 +177,9 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
     }
   }
 
-  return FrozenKeyIdxBiMap{
-    /* length = */ key_count,
-    /* keys = */ out_keys,
-    /* mode = */ mode
-  };
+  return FrozenKeyIdxBiMap{/* length = */ key_count,
+                           /* keys = */ out_keys,
+                           /* mode = */ mode};
 }
 
 /// returns whether new_FrozenKeyIdxBiMap constructed a valid object
@@ -207,7 +199,6 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
   }
 }
 
-
 /// Makes a clone of the specified FrozenKeyIdxBiMap (the clone inherites the
 /// original BiMapMode).
 ///
@@ -224,9 +215,7 @@ FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
 /// returns the value associated with the key or (if the key can't be found)
 /// @ref grackle::impl::bimap::invalid_val
 inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
-  const FrozenKeyIdxBiMap* map, const char* key
-)
-{
+    const FrozenKeyIdxBiMap* map, const char* key) {
   GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
   for (int i = 0; i < map->length; i++) {
     if (std::strcmp(map->keys[i], key) == 0) {
@@ -242,15 +231,17 @@ inline int FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
   return FrozenKeyIdxBiMap_idx_from_key(map, key) != bimap::invalid_val;
 }
 
-inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map)
-{ return map->length; }
+inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map) {
+  return map->length;
+}
 
 /// Return the ith key (this is effectively a reverse lookup)
-inline const char* FrozenKeyIdxBiMap_key_from_idx(
-  const FrozenKeyIdxBiMap* map, std::uint16_t i
-) {
-  if (i >= map->length) { return nullptr; }
-  return map->keys[i]; // this can't be a nullptr
+inline const char* FrozenKeyIdxBiMap_key_from_idx(const FrozenKeyIdxBiMap* map,
+                                                  std::uint16_t i) {
+  if (i >= map->length) {
+    return nullptr;
+  }
+  return map->keys[i];  // this can't be a nullptr
 }
 
 }  // namespace grackle::impl
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index dfff5ba99..61e45cf60 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -17,19 +17,18 @@
 #include "utils/FrozenKeyIdxBiMap.hpp"
 #include "grackle.h"
 
-class FrozenKeyIdxBiMapConstructorSuite :
-  public testing::TestWithParam<grackle::impl::BiMapMode> {
+class FrozenKeyIdxBiMapConstructorSuite
+    : public testing::TestWithParam<grackle::impl::BiMapMode> {
   // You can implement all the usual fixture class members here.
   // To access the test parameter, call GetParam() from class
   // TestWithParam<T>.
-
 };
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, Simple) {
   const char* keys[] = {"denisty", "internal_energy"};
 
-  grackle::impl::FrozenKeyIdxBiMap tmp
-    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+  grackle::impl::FrozenKeyIdxBiMap tmp =
+      grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
 
   EXPECT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
   grackle::impl::drop_FrozenKeyIdxBiMap(&tmp);
@@ -40,8 +39,8 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
   std::string long_key(grackle::impl::bimap::keylen_max, 'A');
   const char* keys[2] = {first_key, long_key.data()};
 
-  grackle::impl::FrozenKeyIdxBiMap tmp
-    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+  grackle::impl::FrozenKeyIdxBiMap tmp =
+      grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
 
   ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
   grackle::impl::drop_FrozenKeyIdxBiMap(&tmp);
@@ -49,46 +48,44 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, TooLongKey) {
   const char* first_key = "density";
-  std::string long_key(grackle::impl::bimap::keylen_max+1, 'A');
+  std::string long_key(grackle::impl::bimap::keylen_max + 1, 'A');
   const char* keys[2] = {first_key, long_key.data()};
 
-  grackle::impl::FrozenKeyIdxBiMap tmp
-    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+  grackle::impl::FrozenKeyIdxBiMap tmp =
+      grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
   ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, 0LenKey) {
-  const char* keys[2] = {"density",""};
-  grackle::impl::FrozenKeyIdxBiMap tmp
-    = grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
+  const char* keys[2] = {"density", ""};
+  grackle::impl::FrozenKeyIdxBiMap tmp =
+      grackle::impl::new_FrozenKeyIdxBiMap(keys, 2, GetParam());
   ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, NoKeys) {
-
-  grackle::impl::FrozenKeyIdxBiMap tmp
-    = grackle::impl::new_FrozenKeyIdxBiMap(nullptr, 0, GetParam());
+  grackle::impl::FrozenKeyIdxBiMap tmp =
+      grackle::impl::new_FrozenKeyIdxBiMap(nullptr, 0, GetParam());
   ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
 INSTANTIATE_TEST_SUITE_P(
-  , /* <- leaving Instantiation name empty */
-  FrozenKeyIdxBiMapConstructorSuite,
-  testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA, grackle::impl::BiMapMode::COPIES_KEYDATA),
-  [](const testing::TestParamInfo<FrozenKeyIdxBiMapConstructorSuite::ParamType>& info){
-    if (info.param == grackle::impl::BiMapMode::REFS_KEYDATA) {
-      return std::string("BIMAP_REFS_KEYDATA");
-    } else {
-      return std::string("BIMAP_COPIES_KEYDATA");
-    }
-  }
-);
-
+    , /* <- leaving Instantiation name empty */
+    FrozenKeyIdxBiMapConstructorSuite,
+    testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
+                    grackle::impl::BiMapMode::COPIES_KEYDATA),
+    [](const testing::TestParamInfo<
+        FrozenKeyIdxBiMapConstructorSuite::ParamType>& info) {
+      if (info.param == grackle::impl::BiMapMode::REFS_KEYDATA) {
+        return std::string("BIMAP_REFS_KEYDATA");
+      } else {
+        return std::string("BIMAP_COPIES_KEYDATA");
+      }
+    });
 
 /// helper function to initialize a map from a vector
 grackle::impl::FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
-  const std::vector<std::string>& vec_, grackle::impl::BiMapMode mode)
-{
+    const std::vector<std::string>& vec_, grackle::impl::BiMapMode mode) {
   std::size_t key_count = vec_.size();
 
   // create a vector of pointers
@@ -100,9 +97,8 @@ grackle::impl::FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(
   return new_FrozenKeyIdxBiMap(key_ptr_l.data(), key_count, mode);
 }
 
-class FrozenKeyIdxBiMapGeneralSuite :
-  public testing::TestWithParam<grackle::impl::BiMapMode>
-{
+class FrozenKeyIdxBiMapGeneralSuite
+    : public testing::TestWithParam<grackle::impl::BiMapMode> {
 protected:
   std::vector<std::string> ordered_keys;
   grackle::impl::FrozenKeyIdxBiMap* bimap_p = nullptr;
@@ -110,13 +106,12 @@ class FrozenKeyIdxBiMapGeneralSuite :
   // we use SetUp/Teardown instead of constructor and destructor so we can
   // perform some sanity checks with ASSERTIONs
   void SetUp() override {
-    ordered_keys = std::vector<std::string>{
-      "internal_energy", "density", "metal_density"
-    };
+    ordered_keys =
+        std::vector<std::string>{"internal_energy", "density", "metal_density"};
 
     grackle::impl::BiMapMode mode = GetParam();
-    grackle::impl::FrozenKeyIdxBiMap tmp
-      = new_FrozenKeyIdxBiMap(ordered_keys, mode);
+    grackle::impl::FrozenKeyIdxBiMap tmp =
+        new_FrozenKeyIdxBiMap(ordered_keys, mode);
     ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 
     bimap_p = new grackle::impl::FrozenKeyIdxBiMap;
@@ -139,66 +134,50 @@ class FrozenKeyIdxBiMapGeneralSuite :
     }
     return true;
   }
-
 };
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "density"),
+            1);
   EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "density"),
-    1
-  );
+      grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "internal_energy"),
+      0);
   EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "internal_energy"),
-    0
-  );
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "metal_density"),
-    2
-  );
+      grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "metal_density"),
+      2);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
-    grackle::impl::bimap::invalid_val
-  );
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
+            grackle::impl::bimap::invalid_val);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
-    grackle::impl::bimap::invalid_val
-  );
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
+            grackle::impl::bimap::invalid_val);
 
-  std::string key(grackle::impl::bimap::keylen_max+1, 'A');
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, key.data()),
-    grackle::impl::bimap::invalid_val
-  );
+  std::string key(grackle::impl::bimap::keylen_max + 1, 'A');
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, key.data()),
+            grackle::impl::bimap::invalid_val);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 3), nullptr);
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
-      bimap_p, grackle::impl::bimap::invalid_val),
-    nullptr
-  );
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
+                bimap_p, grackle::impl::bimap::invalid_val),
+            nullptr);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
   EXPECT_EQ(
-    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 2)),
-    std::string("metal_density")
-  );
+      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 2)),
+      std::string("metal_density"));
   EXPECT_EQ(
-    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 1)),
-    std::string("density")
-  );
+      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 1)),
+      std::string("density"));
   EXPECT_EQ(
-    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 0)),
-    std::string("internal_energy")
-  );
+      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 0)),
+      std::string("internal_energy"));
 
   // check whether the bimap is using pointers to the keys used during init
   if (GetParam() == grackle::impl::BiMapMode::REFS_KEYDATA) {
@@ -209,8 +188,8 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
-  grackle::impl::FrozenKeyIdxBiMap clone = 
-    grackle::impl::FrozenKeyIdxBiMap_clone(bimap_p);
+  grackle::impl::FrozenKeyIdxBiMap clone =
+      grackle::impl::FrozenKeyIdxBiMap_clone(bimap_p);
   ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&clone));
   grackle::impl::FrozenKeyIdxBiMap* clone_p = &clone;
 
@@ -219,22 +198,15 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   bimap_p = nullptr;
 
   EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "internal_energy"),
-    0
-  );
-  EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "notAKey"),
-    grackle::impl::bimap::invalid_val
-  );
+      grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "internal_energy"),
+      0);
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "notAKey"),
+            grackle::impl::bimap::invalid_val);
 
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 3), nullptr);
   EXPECT_EQ(
-    grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 3),
-    nullptr
-  );
-  EXPECT_EQ(
-    std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 1)),
-    std::string("density")
-  );
+      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 1)),
+      std::string("density"));
 
   // check whether the clone is using pointers to the keys used during init
   if (GetParam() == grackle::impl::BiMapMode::REFS_KEYDATA) {
@@ -247,17 +219,16 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   grackle::impl::drop_FrozenKeyIdxBiMap(clone_p);
 }
 
-
 INSTANTIATE_TEST_SUITE_P(
-  , /* <- leaving Instantiation name empty */
-  FrozenKeyIdxBiMapGeneralSuite,
-  testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
-                  grackle::impl::BiMapMode::COPIES_KEYDATA),
-  [](const testing::TestParamInfo<FrozenKeyIdxBiMapGeneralSuite::ParamType>& info){
-    if (info.param == grackle::impl::BiMapMode::REFS_KEYDATA) {
-      return std::string("BIMAP_REFS_KEYDATA");
-    } else {
-      return std::string("BIMAP_COPIES_KEYDATA");
-    }
-  }
-);
+    , /* <- leaving Instantiation name empty */
+    FrozenKeyIdxBiMapGeneralSuite,
+    testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
+                    grackle::impl::BiMapMode::COPIES_KEYDATA),
+    [](const testing::TestParamInfo<FrozenKeyIdxBiMapGeneralSuite::ParamType>&
+           info) {
+      if (info.param == grackle::impl::BiMapMode::REFS_KEYDATA) {
+        return std::string("BIMAP_REFS_KEYDATA");
+      } else {
+        return std::string("BIMAP_COPIES_KEYDATA");
+      }
+    });

From 19d4f7e64948cf1fbf6a9062a2aa66d28f4bb6c4 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 19 Nov 2025 12:31:00 -0500
Subject: [PATCH 07/25] a minor tweak to rerun CI

---
 src/clib/utils/FrozenKeyIdxBiMap.hpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/utils/FrozenKeyIdxBiMap.hpp
index c884c9bbf..a9537a5c0 100644
--- a/src/clib/utils/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/utils/FrozenKeyIdxBiMap.hpp
@@ -18,8 +18,8 @@
 /// be tweaked)
 ///
 /// If we decide to more fully embrace C++, it would make a LOT of sense to
-/// convert this to a full-blown class (and delete copy constructor/copy
-/// assignement OR adopt reference counting).
+/// convert this to a full-blown class (we would probably delete the copy
+/// constructor And copy assignement methods OR adopt reference counting).
 ///
 //===----------------------------------------------------------------------===//
 #ifndef UTILS_FROZENKEYIDXBIMAP_HPP

From 18020b69fcab5710bbda52223d3355d07fd9375c Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Tue, 9 Dec 2025 19:37:25 -0500
Subject: [PATCH 08/25] use FrozenKeyIdxBiMap within GrainSpeciesInfo (1/3)

One minor change involved formally making it illegal to construct a
GrainSpeciesInfo instance that holds 0 dust species. That's perfectly
fine. `GrainSpeciesInfo` holds some redundant info that we will remove
in the a subsequent commit
---
 src/clib/dust/grain_species_info.cpp   | 68 ++++++++++++++++++--------
 src/clib/dust/grain_species_info.hpp   | 12 +++++
 src/clib/utils/FrozenKeyIdxBiMap.hpp   | 21 ++++----
 tests/unit/test_grain_species_info.cpp | 12 +----
 4 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/clib/dust/grain_species_info.cpp b/src/clib/dust/grain_species_info.cpp
index 3a6a76f80..d60eef6c4 100644
--- a/src/clib/dust/grain_species_info.cpp
+++ b/src/clib/dust/grain_species_info.cpp
@@ -15,6 +15,7 @@
 
 #include "LUT.hpp"
 #include "grain_species_info.hpp"
+#include "../utils/FrozenKeyIdxBiMap.hpp"
 
 // The following logic effectively does 2 (related things):
 // 1. it serves as a human-readable registry of all known grain species and
@@ -85,18 +86,23 @@ grackle::impl::GrainSpeciesInfoEntry mk_gsp_info_entry_helper_(
                                               out_ingredient_ptr};
 }
 
+// ugh, I don't like this...
+grackle::impl::GrainSpeciesInfo mk_invalid_GrainSpeciesInfo() {
+  return {-1, nullptr, grackle::impl::mk_invalid_FrozenKeyIdxBiMap()};
+}
+
 }  // anonymous namespace
 
 grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     int dust_species_parameter) {
-  GrainSpeciesInfo out{-1, nullptr};  // indicates an error
-  out.n_species = get_n_grain_species(dust_species_parameter);
-
-  if (out.n_species <= 0) {
-    return out;
+  int n_species = get_n_grain_species(dust_species_parameter);
+  if (n_species <= 0) {
+    return mk_invalid_GrainSpeciesInfo();
   }
 
-  out.species_info = new GrainSpeciesInfoEntry[out.n_species];
+  // names is allocated with the max number of known grain species
+  const char* names[OnlyGrainSpLUT::NUM_ENTRIES];
+  GrainSpeciesInfoEntry* species_info = new GrainSpeciesInfoEntry[n_species];
 
   // At the time of writing:
   // - we **only** use h2rate_carbonaceous_coef_table for the AC_dust
@@ -126,7 +132,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         {1, SpLUT::SiOI, 44.},
         {2, SpLUT::H2O, 18.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[0] = mk_gsp_info_entry_helper_(
+    names[0] = "MgSiO3_dust";
+    species_info[0] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::MgSiO3_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::MgSiO3_dust,
         /* name = */ "MgSiO3_dust",
@@ -140,7 +147,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         // {coef, species_idx, particle mass}
         {1, SpLUT::CI, 12.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[1] = mk_gsp_info_entry_helper_(
+    names[1] = "AC_dust";
+    species_info[1] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::AC_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::AC_dust,
         /* name = */ "AC_dust",
@@ -156,7 +164,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         // {coef, species_idx, particle mass}
         {1, SpLUT::SiI, 28.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[2] = mk_gsp_info_entry_helper_(
+    names[2] = "SiM_dust";
+    species_info[2] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::SiM_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::SiM_dust,
         /* name = */ "SiM_dust",
@@ -170,7 +179,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         // {coef, species_idx, particle mass}
         {1, SpLUT::Fe, 56.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[3] = mk_gsp_info_entry_helper_(
+    names[3] = "FeM_dust";
+    species_info[3] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::FeM_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::FeM_dust,
         /* name = */ "FeM_dust",
@@ -186,7 +196,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         {1, SpLUT::SiOI, 44.},
         {3, SpLUT::H2O, 18.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[4] = mk_gsp_info_entry_helper_(
+    names[4] = "Mg2SiO4_dust";
+    species_info[4] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::Mg2SiO4_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::Mg2SiO4_dust,
         /* name = */ "Mg2SiO4_dust",
@@ -201,7 +212,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         {3, SpLUT::Fe, 56.},
         {4, SpLUT::H2O, 18.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[5] = mk_gsp_info_entry_helper_(
+    names[5] = "Fe3O4_dust";
+    species_info[5] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::Fe3O4_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::Fe3O4_dust,
         /* name = */ "Fe3O4_dust",
@@ -215,7 +227,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         // {coef, species_idx, particle mass}
         {1, SpLUT::SiO2I, 60.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[6] = mk_gsp_info_entry_helper_(
+    names[6] = "SiO2_dust";
+    species_info[6] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::SiO2_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::SiO2_dust,
         /* name = */ "SiO2_dust",
@@ -230,7 +243,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         {1, SpLUT::Mg, 24.},
         {1, SpLUT::H2O, 18.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[7] = mk_gsp_info_entry_helper_(
+    names[7] = "MgO_dust";
+    species_info[7] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::MgO_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::MgO_dust,
         /* name = */ "MgO_dust",
@@ -245,7 +259,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         {1, SpLUT::Fe, 56.},
         {1, SpLUT::S, 32.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[8] = mk_gsp_info_entry_helper_(
+    names[8] = "FeS_dust";
+    species_info[8] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::FeS_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::FeS_dust,
         /* name = */ "FeS_dust",
@@ -260,7 +275,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         {2, SpLUT::Al, 27.},
         {3, SpLUT::H2O, 18.},
         GRIMPL_INGREDIENT_LIST_SENTINEL};
-    out.species_info[9] = mk_gsp_info_entry_helper_(
+    names[9] = "Al2O3_dust";
+    species_info[9] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::Al2O3_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::Al2O3_dust,
         /* name = */ "Al2O3_dust",
@@ -277,7 +293,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
 
     // nominal growth rxn: "0.5CO + 0.5CH2 + 1.2N -> ref_org_dust"
     // nuclide ratios: C:H:O:N = 1:1:0.5:1.2
-    out.species_info[10] = mk_gsp_info_entry_helper_(
+    names[10] = "ref_org_dust";
+    species_info[10] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::ref_org_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::ref_org_dust,
         /* name = */ "ref_org_dust",
@@ -288,7 +305,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
 
     // nominal growth rxn: "CO + 2H2I -> vol_org_dust"
     // effective formula: CH3OH
-    out.species_info[11] = mk_gsp_info_entry_helper_(
+    names[11] = "vol_org_dust";
+    species_info[11] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::vol_org_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::vol_org_dust,
         /* name = */ "vol_org_dust",
@@ -298,7 +316,8 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         /* growth_ingredients = */ nullptr);
 
     // nominal growth rxn: "H2O -> H2O_ice_dust"
-    out.species_info[12] = mk_gsp_info_entry_helper_(
+    names[12] = "H2O_ice_dust";
+    species_info[12] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::H2O_ice_dust,
         /* onlygrainsp_idx = */ OnlyGrainSpLUT::H2O_ice_dust,
         /* name = */ "H2O_ice_dust",
@@ -308,7 +327,16 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
         /* growth_ingredients = */ nullptr);
   }
 
-  return out;
+  GrainSpeciesInfo out{
+      n_species, species_info,
+      new_FrozenKeyIdxBiMap(names, n_species, BiMapMode::COPIES_KEYDATA)};
+
+  if (FrozenKeyIdxBiMap_is_ok(&out.name_map)) {
+    return out;
+  } else {
+    drop_GrainSpeciesInfo(&out);
+    return mk_invalid_GrainSpeciesInfo();
+  }
 }
 
 #undef GRIMPL_INGREDIENT_LIST_SENTINEL
diff --git a/src/clib/dust/grain_species_info.hpp b/src/clib/dust/grain_species_info.hpp
index 1f20c52ea..6785624d9 100644
--- a/src/clib/dust/grain_species_info.hpp
+++ b/src/clib/dust/grain_species_info.hpp
@@ -13,6 +13,8 @@
 #ifndef GRAIN_SPECIES_INFO_HPP
 #define GRAIN_SPECIES_INFO_HPP
 
+#include "../utils/FrozenKeyIdxBiMap.hpp"
+
 namespace grackle::impl {
 
 /// holds information about a single gas species that is an ingredient for
@@ -116,6 +118,15 @@ struct GrainSpeciesInfo {
   /// an out.species_infoay of length of length @ref n_species where each entry
   /// holds info about a separate grain species
   GrainSpeciesInfoEntry* species_info;
+
+  /// maps between grain species names and the associated index. The mapping is
+  /// **ALWAYS** consistent with ``OnlyGrainSpLUT``.
+  ///
+  /// @note
+  /// An argument could be made for storing this separately from the rest of
+  /// the struct since the core grackle calculations don't (or at least
+  /// shouldn't) use this data structure during the calculation.
+  FrozenKeyIdxBiMap name_map;
 };
 
 /// return the number of grain species
@@ -164,6 +175,7 @@ inline void drop_GrainSpeciesInfo(GrainSpeciesInfo* ptr) {
     }
   }
   delete[] ptr->species_info;
+  drop_FrozenKeyIdxBiMap(&ptr->name_map);
   // the following 2 lines are not strictly necessary, but they may help us
   // avoid a double-free and a dangling pointer
   ptr->n_species = 0;
diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/utils/FrozenKeyIdxBiMap.hpp
index a9537a5c0..bdb3537a2 100644
--- a/src/clib/utils/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/utils/FrozenKeyIdxBiMap.hpp
@@ -103,6 +103,12 @@ struct FrozenKeyIdxBiMap {
   BiMapMode mode;
 };
 
+// ugh, it's unfortunate that we need to make this... but for now its useful.
+// Ideally, we would refactor so that we can get rid of this function.
+inline FrozenKeyIdxBiMap mk_invalid_FrozenKeyIdxBiMap() {
+  return FrozenKeyIdxBiMap{0, nullptr, BiMapMode::REFS_KEYDATA};
+}
+
 /// Constructs a new FrozenKeyIdxBiMap
 ///
 /// @param[in]  keys Sequence of 1 or more unique strings. Each string must
@@ -119,18 +125,15 @@ struct FrozenKeyIdxBiMap {
 /// > this a simple C++ class instead)
 inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
                                                int key_count, BiMapMode mode) {
-  // this will be returned if there is an error
-  FrozenKeyIdxBiMap erroneous_obj{0, nullptr, BiMapMode::REFS_KEYDATA};
-
   // check the specified keys
   long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
   if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
     GrPrintErrMsg("key_count must be positive and cannot exceed %lld",
                   max_keys);
-    return erroneous_obj;
+    return mk_invalid_FrozenKeyIdxBiMap();
   } else if (keys == nullptr) {
     GrPrintErrMsg("keys must not be a nullptr");
-    return erroneous_obj;
+    return mk_invalid_FrozenKeyIdxBiMap();
   }
   for (int i = 0; i < key_count; i++) {
     GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
@@ -140,13 +143,13 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
           "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
           "exceeding %d",
           keys[i], i, bimap::keylen_max);
-      return erroneous_obj;
+      return mk_invalid_FrozenKeyIdxBiMap();
     }
     // check uniqueness
     for (int j = 0; j < i; j++) {
       if (strcmp(keys[i], keys[j]) == 0) {
         GrPrintErrMsg("\"%s\" key repeats", keys[i]);
-        return erroneous_obj;
+        return mk_invalid_FrozenKeyIdxBiMap();
       }
     }
   }
@@ -173,7 +176,7 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
     }
     default: {
       GrPrintErrMsg("unknown mode");
-      return erroneous_obj;
+      return mk_invalid_FrozenKeyIdxBiMap();
     }
   }
 
@@ -208,7 +211,7 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
 /// > This is pretty ugly/clunky, but its the only practical way to achieve
 /// > comparable behavior to other internal datatypes (ideally, we would make
 /// > this a simple C++ class instead)
-FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
+inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
   return new_FrozenKeyIdxBiMap(ptr->keys, ptr->length, ptr->mode);
 };
 
diff --git a/tests/unit/test_grain_species_info.cpp b/tests/unit/test_grain_species_info.cpp
index 19bd87f4e..c09e7ccbf 100644
--- a/tests/unit/test_grain_species_info.cpp
+++ b/tests/unit/test_grain_species_info.cpp
@@ -194,17 +194,7 @@ INSTANTIATE_TEST_SUITE_P(
 // check the GrainSpeciesInfo object when constructed from dust_species
 // parameters that hold extreme values
 TEST(GrainSpeciesInfoTestMisc, DustSpeciesExtremeValues) {
-  {
-    unique_GrainSpeciesInfo_ptr ptr = make_unique_GrainSpeciesInfo(0);
-    EXPECT_EQ(ptr->n_species, 0)
-        << "GrainSpeciesInfo::n_species should be 0 when the dust_species "
-        << "parameter is 0.";
-    EXPECT_EQ(ptr->species_info, nullptr)
-        << "GrainSpeciesInfo::species_info should be a nullptr when "
-        << "dust_species parameter is 0.";
-  }
-
-  int invalid_dust_species_values[2] = {-42423, MAX_dust_species_VAL + 1};
+  int invalid_dust_species_values[3] = {-42423, 0, MAX_dust_species_VAL + 1};
   for (auto dust_species_param : invalid_dust_species_values) {
     unique_GrainSpeciesInfo_ptr ptr =
         make_unique_GrainSpeciesInfo(dust_species_param);

From 00865dd5ffe9af1b3729ab04a09997adc8d3df72 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Tue, 9 Dec 2025 20:23:00 -0500
Subject: [PATCH 09/25] use FrozenKeyIdxBiMap within GrainSpeciesInfo (2/3)

Modify tests to start using `FrozenKeyIdxBiMap` rather than
`GrainSpeciesInfoEntry::onlygrainsp_idx` and
`GrainSpeciesInfoEntry::name`. In the next commit, we'll remove these
members from `GrainSpeciesInfoEntry`
---
 tests/unit/test_grain_species_info.cpp | 53 ++++++++++++++++++--------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/tests/unit/test_grain_species_info.cpp b/tests/unit/test_grain_species_info.cpp
index c09e7ccbf..b941d55a0 100644
--- a/tests/unit/test_grain_species_info.cpp
+++ b/tests/unit/test_grain_species_info.cpp
@@ -17,6 +17,7 @@
 
 #include "LUT.hpp"
 #include "dust/grain_species_info.hpp"
+#include "utils/FrozenKeyIdxBiMap.hpp"
 
 namespace {  // stuff in an anonymous namespace is local to this file
 
@@ -130,27 +131,48 @@ class GrainSpeciesInfoTest : public testing::TestWithParam<int> {
   unique_GrainSpeciesInfo_ptr grain_species_info_;
 };
 
+// the following test is somewhat redundant with the SpeciesLUTCompare tests
+// and is more work to maintain
 TEST_P(GrainSpeciesInfoTest, CheckOnlyGrainSpeciesLUTConsistency) {
+  // construct a vector with an element for each entry in OnlyGrainSpeciesLUT
+  std::vector<SpNameIndexPair> ref_l{
+      {"MgSiO3_dust", OnlyGrainSpLUT::MgSiO3_dust},
+      {"AC_dust", OnlyGrainSpLUT::AC_dust},
+      {"SiM_dust", OnlyGrainSpLUT::SiM_dust},
+      {"FeM_dust", OnlyGrainSpLUT::FeM_dust},
+      {"Mg2SiO4_dust", OnlyGrainSpLUT::Mg2SiO4_dust},
+      {"Fe3O4_dust", OnlyGrainSpLUT::Fe3O4_dust},
+      {"SiO2_dust", OnlyGrainSpLUT::SiO2_dust},
+      {"MgO_dust", OnlyGrainSpLUT::MgO_dust},
+      {"FeS_dust", OnlyGrainSpLUT::FeS_dust},
+      {"Al2O3_dust", OnlyGrainSpLUT::Al2O3_dust},
+      {"ref_org_dust", OnlyGrainSpLUT::ref_org_dust},
+      {"vol_org_dust", OnlyGrainSpLUT::vol_org_dust},
+      {"H2O_ice_dust", OnlyGrainSpLUT::H2O_ice_dust},
+  };
+
   const int n_species = grain_species_info_->n_species;
   for (int i = 0; i < n_species; i++) {
-    // sanity check!
-    ASSERT_NE(grain_species_info_->species_info[i].name, nullptr);
-    // actual check!
-    EXPECT_EQ(i, grain_species_info_->species_info[i].onlygrainsp_idx)
-        << "element " << i << " of the GrainSpeciesInfo::species_info array "
-        << "doesn't seem to be synchronized with the OnlyGrainSpeciesLUT "
-        << "enumeration. At face value (there aren't other related bugs), "
-        << "OnlyGrainSpeciesLUT::" << grain_species_info_->species_info[i].name
-        << " seems to have a value of "
-        << grain_species_info_->species_info[i].onlygrainsp_idx;
+    const char* name = grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
+        &grain_species_info_->name_map, static_cast<std::uint16_t>(i));
+
+    ASSERT_NE(name, nullptr);      // sanity check!
+    EXPECT_EQ(i, ref_l[i].index);  // sanity check!
+
+    // actual check:
+    EXPECT_EQ(std::string(name), ref_l[i].name)
+        << "the grain species associated with index " << i << " in the "
+        << "GrainSpeciesInfo instance doesn't seem to be synchronized with "
+        << "the OnlyGrainSpeciesLUT enumeration.";
   }
 }
 
 TEST_P(GrainSpeciesInfoTest, SublimationTemperature) {
   const int n_species = grain_species_info_->n_species;
   for (int i = 0; i < n_species; i++) {
-    // sanity check!
-    ASSERT_NE(grain_species_info_->species_info[i].name, nullptr);
+    const char* name = grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
+        &grain_species_info_->name_map, static_cast<std::uint16_t>(i));
+    ASSERT_NE(name, nullptr);  // sanity check!
     // actual check!
     EXPECT_GT(grain_species_info_->species_info[i].sublimation_temperature, 0)
         << "element " << i << " of the GrainSpeciesInfo::species_info array "
@@ -165,10 +187,11 @@ TEST_P(GrainSpeciesInfoTest, SpeciesLUTCompare) {
 
   const int n_species = grain_species_info_->n_species;
   for (int i = 0; i < n_species; i++) {
-    // sanity check!
-    ASSERT_NE(grain_species_info_->species_info[i].name, nullptr);
+    const char* name_cstr = grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
+        &grain_species_info_->name_map, static_cast<std::uint16_t>(i));
+    ASSERT_NE(name_cstr, nullptr);  // sanity check!
     // actual check!
-    std::string actual_name(grain_species_info_->species_info[i].name);
+    std::string actual_name(name_cstr);
     EXPECT_EQ(actual_name, ref_list[i].name)
         << "according to the reference list, the grain species at index " << i
         << " should be `" << ref_list[i].name << "` (not `" << actual_name

From fa08c26c6c74d98a4c90c5f38181a3a28fad3b8c Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Tue, 9 Dec 2025 20:28:26 -0500
Subject: [PATCH 10/25] use FrozenKeyIdxBiMap within GrainSpeciesInfo (3/3)

Delete `GrainSpeciesInfoEntry::onlygrainsp_idx` and
`GrainSpeciesInfoEntry::name`.
---
 src/clib/dust/grain_species_info.cpp | 33 ++--------------------------
 src/clib/dust/grain_species_info.hpp | 13 -----------
 2 files changed, 2 insertions(+), 44 deletions(-)

diff --git a/src/clib/dust/grain_species_info.cpp b/src/clib/dust/grain_species_info.cpp
index d60eef6c4..d82aa4340 100644
--- a/src/clib/dust/grain_species_info.cpp
+++ b/src/clib/dust/grain_species_info.cpp
@@ -52,9 +52,8 @@ namespace {  // stuff inside an anonymous namespace is local to this file
 /// - the ingredient list in the returned instance is **NOT** terminated by
 ///   the sentinel
 grackle::impl::GrainSpeciesInfoEntry mk_gsp_info_entry_helper_(
-    int species_idx, int onlygrainsp_idx, const char* name,
-    bool h2dust_uses_carbonaceous_table, double sublimation_temperature,
-    double bulk_density_cgs,
+    int species_idx, bool h2dust_uses_carbonaceous_table,
+    double sublimation_temperature, double bulk_density_cgs,
     const grackle::impl::GrainGrowthIngredient* growth_ingredients) {
   using grackle::impl::GrainGrowthIngredient;
 
@@ -77,8 +76,6 @@ grackle::impl::GrainSpeciesInfoEntry mk_gsp_info_entry_helper_(
   }
 
   return grackle::impl::GrainSpeciesInfoEntry{species_idx,
-                                              onlygrainsp_idx,
-                                              name,
                                               h2dust_uses_carbonaceous_table,
                                               sublimation_temperature,
                                               bulk_density_cgs,
@@ -135,8 +132,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[0] = "MgSiO3_dust";
     species_info[0] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::MgSiO3_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::MgSiO3_dust,
-        /* name = */ "MgSiO3_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1222.0,
         /* bulk_density_cgs = */ 3.20185,
@@ -150,8 +145,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[1] = "AC_dust";
     species_info[1] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::AC_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::AC_dust,
-        /* name = */ "AC_dust",
         /* h2dust_uses_carbonaceous_table = */ true,
         /* sublimation_temperature = */ 1800.0,
         /* bulk_density_cgs = */ 2.27949,
@@ -167,8 +160,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[2] = "SiM_dust";
     species_info[2] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::SiM_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::SiM_dust,
-        /* name = */ "SiM_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1500.0,
         /* bulk_density_cgs = */ 2.34118,
@@ -182,8 +173,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[3] = "FeM_dust";
     species_info[3] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::FeM_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::FeM_dust,
-        /* name = */ "FeM_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1500.0,
         /* bulk_density_cgs = */ 7.95995,
@@ -199,8 +188,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[4] = "Mg2SiO4_dust";
     species_info[4] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::Mg2SiO4_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::Mg2SiO4_dust,
-        /* name = */ "Mg2SiO4_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1277.0,
         /* bulk_density_cgs = */ 3.22133,
@@ -215,8 +202,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[5] = "Fe3O4_dust";
     species_info[5] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::Fe3O4_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::Fe3O4_dust,
-        /* name = */ "Fe3O4_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1500.0,
         /* bulk_density_cgs = */ 5.25096,
@@ -230,8 +215,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[6] = "SiO2_dust";
     species_info[6] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::SiO2_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::SiO2_dust,
-        /* name = */ "SiO2_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1500.0,
         /* bulk_density_cgs = */ 2.66235,
@@ -246,8 +229,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[7] = "MgO_dust";
     species_info[7] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::MgO_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::MgO_dust,
-        /* name = */ "MgO_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1500.0,
         /* bulk_density_cgs = */ 3.58157,
@@ -262,8 +243,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[8] = "FeS_dust";
     species_info[8] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::FeS_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::FeS_dust,
-        /* name = */ "FeS_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 680.0,
         /* bulk_density_cgs = */ 4.87265,
@@ -278,8 +257,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[9] = "Al2O3_dust";
     species_info[9] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::Al2O3_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::Al2O3_dust,
-        /* name = */ "Al2O3_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 1500.0,
         /* bulk_density_cgs = */ 4.01610,
@@ -296,8 +273,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[10] = "ref_org_dust";
     species_info[10] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::ref_org_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::ref_org_dust,
-        /* name = */ "ref_org_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 575.0,
         /* bulk_density_cgs = */ 1.5,
@@ -308,8 +283,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[11] = "vol_org_dust";
     species_info[11] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::vol_org_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::vol_org_dust,
-        /* name = */ "vol_org_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 375.0,
         /* bulk_density_cgs = */ 1.0,
@@ -319,8 +292,6 @@ grackle::impl::GrainSpeciesInfo grackle::impl::new_GrainSpeciesInfo(
     names[12] = "H2O_ice_dust";
     species_info[12] = mk_gsp_info_entry_helper_(
         /* species_idx = */ SpLUT::H2O_ice_dust,
-        /* onlygrainsp_idx = */ OnlyGrainSpLUT::H2O_ice_dust,
-        /* name = */ "H2O_ice_dust",
         /* h2dust_uses_carbonaceous_table = */ false,
         /* sublimation_temperature = */ 153.0,
         /* bulk_density_cgs = */ 0.92,
diff --git a/src/clib/dust/grain_species_info.hpp b/src/clib/dust/grain_species_info.hpp
index 6785624d9..b306c154e 100644
--- a/src/clib/dust/grain_species_info.hpp
+++ b/src/clib/dust/grain_species_info.hpp
@@ -41,19 +41,6 @@ struct GrainSpeciesInfoEntry {
   /// the species index of the grain in the #GrainSpLUT lookup table
   int species_idx;
 
-  /// the species index of the grain in the #OnlyGrainSpLUT lookup table
-  ///
-  /// @note
-  /// It's frankly a little redundant to track this information (since an
-  /// instance of this struct is found at this index of an out.species_infoay)
-  int onlygrainsp_idx;
-
-  /// name of the dust species
-  ///
-  /// @note
-  /// This primarily exists for debuging purposes
-  const char* name;
-
   /// indicates whether to use the carbonaceous or silicate coefficient table
   /// to computing contributions of the grain species to the total h2dust rate
   /// (or the rate of H2 formation)

From 721d1a0d0794e334a508676709857782c675ff53 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 10 Dec 2025 10:33:45 -0500
Subject: [PATCH 11/25] fix a few typos

---
 src/clib/dust/grain_species_info.hpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/clib/dust/grain_species_info.hpp b/src/clib/dust/grain_species_info.hpp
index b306c154e..bddafc11d 100644
--- a/src/clib/dust/grain_species_info.hpp
+++ b/src/clib/dust/grain_species_info.hpp
@@ -81,17 +81,17 @@ struct GrainSpeciesInfoEntry {
 /// Relationship with OnlyGrainSpLUT
 /// --------------------------------
 /// In the short term, the index of each species in the
-/// @ref GrainSpeciesInfo::species_info out.species_infoay is dictated by the
+/// @ref GrainSpeciesInfo::species_info out.species_info is dictated by the
 /// order of enumerators in the OnlyGrainSpLUT enumeration.
 ///
 /// In the medium term, we plan to entirely eliminate the OnlyGrainSpLUT
-/// enumeration because all of the grain species can be treated very uniformly
-/// uniformly. At the time of writing, just about every place where we would
-/// use OnlyGrainSpLUT corresponds to a location where would enumerate every
+/// enumeration because all of the grain species can be treated very uniformly.
+/// At the time of writing, just about every place where we would use
+/// OnlyGrainSpLUT corresponds to a location where we would enumerate every
 /// possible grain species and perform nearly identical operations on each
 /// species. In each case, it is straight-forward to replace these blocks of
 /// logic with for-loops (we just need to encode species-specific variations in
-/// the calculations in out.species_infoays that have the same ordering as the
+/// the calculations in out.species_info that have the same ordering as the
 /// species). To phrase it another way, in nearly all of the places where we
 /// would use OnlyGrainSpLUT, we don't need to know the grain species identity.
 ///
@@ -102,7 +102,7 @@ struct GrainSpeciesInfo {
   /// number of grain species considered for the current Grackle configuration
   int n_species;
 
-  /// an out.species_infoay of length of length @ref n_species where each entry
+  /// an out.species_info of length of length @ref n_species where each entry
   /// holds info about a separate grain species
   GrainSpeciesInfoEntry* species_info;
 

From dbb6b5d3fcb99042fe6c7de8d2c05ba9077ef954 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Fri, 12 Dec 2025 12:10:19 -0500
Subject: [PATCH 12/25] move FrozenKeyIdxBiMap to support subdirectory

---
 src/clib/CMakeLists.txt                           | 2 +-
 src/clib/{utils => support}/FrozenKeyIdxBiMap.hpp | 0
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp        | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename src/clib/{utils => support}/FrozenKeyIdxBiMap.hpp (100%)

diff --git a/src/clib/CMakeLists.txt b/src/clib/CMakeLists.txt
index f865f97e7..e5c57b16b 100644
--- a/src/clib/CMakeLists.txt
+++ b/src/clib/CMakeLists.txt
@@ -124,8 +124,8 @@ add_library(Grackle_Grackle
   scale_fields.cpp scale_fields.hpp
   solve_rate_cool_g-cpp.cpp solve_rate_cool_g-cpp.h
   step_rate_newton_raphson.hpp
+  support/FrozenKeyIdxBiMap.hpp
   time_deriv_0d.hpp
-  utils/FrozenKeyIdxBiMap.hpp
   utils-cpp.cpp utils-cpp.hpp
   utils-field.hpp
   fortran_func_wrappers.hpp
diff --git a/src/clib/utils/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
similarity index 100%
rename from src/clib/utils/FrozenKeyIdxBiMap.hpp
rename to src/clib/support/FrozenKeyIdxBiMap.hpp
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 61e45cf60..a928829ab 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -14,7 +14,7 @@
 #include <vector>
 
 #include <gtest/gtest.h>
-#include "utils/FrozenKeyIdxBiMap.hpp"
+#include "support/FrozenKeyIdxBiMap.hpp"
 #include "grackle.h"
 
 class FrozenKeyIdxBiMapConstructorSuite

From b9261c3a6a8c47e46655fecbec3db3539265bf42 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Fri, 12 Dec 2025 12:16:45 -0500
Subject: [PATCH 13/25] fixing some paths

---
 src/clib/dust/grain_species_info.cpp   | 2 +-
 src/clib/dust/grain_species_info.hpp   | 2 +-
 tests/unit/test_grain_species_info.cpp | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/clib/dust/grain_species_info.cpp b/src/clib/dust/grain_species_info.cpp
index d82aa4340..38a9d59cc 100644
--- a/src/clib/dust/grain_species_info.cpp
+++ b/src/clib/dust/grain_species_info.cpp
@@ -15,7 +15,7 @@
 
 #include "LUT.hpp"
 #include "grain_species_info.hpp"
-#include "../utils/FrozenKeyIdxBiMap.hpp"
+#include "../support/FrozenKeyIdxBiMap.hpp"
 
 // The following logic effectively does 2 (related things):
 // 1. it serves as a human-readable registry of all known grain species and
diff --git a/src/clib/dust/grain_species_info.hpp b/src/clib/dust/grain_species_info.hpp
index bddafc11d..e64a59b06 100644
--- a/src/clib/dust/grain_species_info.hpp
+++ b/src/clib/dust/grain_species_info.hpp
@@ -13,7 +13,7 @@
 #ifndef GRAIN_SPECIES_INFO_HPP
 #define GRAIN_SPECIES_INFO_HPP
 
-#include "../utils/FrozenKeyIdxBiMap.hpp"
+#include "../support/FrozenKeyIdxBiMap.hpp"
 
 namespace grackle::impl {
 
diff --git a/tests/unit/test_grain_species_info.cpp b/tests/unit/test_grain_species_info.cpp
index b941d55a0..dcefec082 100644
--- a/tests/unit/test_grain_species_info.cpp
+++ b/tests/unit/test_grain_species_info.cpp
@@ -17,7 +17,7 @@
 
 #include "LUT.hpp"
 #include "dust/grain_species_info.hpp"
-#include "utils/FrozenKeyIdxBiMap.hpp"
+#include "support/FrozenKeyIdxBiMap.hpp"
 
 namespace {  // stuff in an anonymous namespace is local to this file
 

From c287fa655680394bcea1fd52fd512f8b84ed73fc Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Mon, 5 Jan 2026 15:10:36 -0500
Subject: [PATCH 14/25] Add support for a map with 0 entries

Also made a few assorted fixes
---
 src/clib/support/FrozenKeyIdxBiMap.hpp | 58 +++++++++++++-------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index a9537a5c0..e9ddebada 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -22,8 +22,8 @@
 /// constructor And copy assignement methods OR adopt reference counting).
 ///
 //===----------------------------------------------------------------------===//
-#ifndef UTILS_FROZENKEYIDXBIMAP_HPP
-#define UTILS_FROZENKEYIDXBIMAP_HPP
+#ifndef SUPPORT_FROZENKEYIDXBIMAP_HPP
+#define SUPPORT_FROZENKEYIDXBIMAP_HPP
 
 #include <cstdint>
 #include <cstdlib>
@@ -54,9 +54,9 @@ namespace grackle::impl {
 
 enum class BiMapMode { REFS_KEYDATA = 0, COPIES_KEYDATA = 1 };
 
-/// @brief This is a bidirectional map (bimap). It is specialized to map `n`
-/// unique string keys to unique indexes with values of `0` through `n-1` and
-/// vice versa. The ordering of keys is set at initialization and frozen.
+/// This is a bidirectional map (bimap). It is specialized to map @c n
+/// unique string keys to unique indexes with values of @c 0` through @c (n-1)
+/// and  vice versa. The ordering of keys is set at initialization and frozen.
 ///
 /// This is primarily intended to be used in the implementation of Maps of
 /// arrays (where the values could be part of a single contiguous array or are
@@ -76,21 +76,21 @@ enum class BiMapMode { REFS_KEYDATA = 0, COPIES_KEYDATA = 1 };
 ///    (where we directly embed the string in the hash-table-rows), this will
 ///    probably be a quite a bit faster
 ///
-/// Replacement in PR #270
-/// ======================
+/// @par Replacement in PR #270
 /// The current implementation is extremely oversimplified and inefficient! It
 /// doesn't even use a hash table. The purpose is to create a simple abstract
 /// data structure for which the implementation will be dramatically improved
 /// by PR #270 (but the interface won't be touched at all).
 ///
+/// @par
 /// The PR with the improved version, also updates this docstring with a
 /// detailed explanation of design decisions (like why the contents
 /// are "frozen") and highlights a number of potential improvements.
 ///
-/// > [!note]
-/// > The contents of this struct should be considered an implementation
-/// > detail! Always prefer the associated functions (they are defined in such
-/// > a way that they should be inlined
+/// @note
+/// The contents of this struct should be considered an implementation
+/// detail! Always prefer the associated functions (they are defined in such
+/// a way that they should be inlined)
 struct FrozenKeyIdxBiMap {
   // don't forget to update FrozenKeyIdxBiMap_clone when changing members
 
@@ -111,16 +111,16 @@ struct FrozenKeyIdxBiMap {
 /// @param[in]  mode specifies handling of keys. This will be passed on to any
 ///     clones that are made.
 ///
-/// > [!note]
-/// > If this function returns `bimap`, then the caller should invoke
-/// > `FrozenKeyIdxBiMap_is_ok(&bimap)` to test whether there was an error.
-/// > This is pretty ugly/clunky, but its the only practical way to achieve
-/// > comparable behavior to other internal datatypes (ideally, we would make
-/// > this a simple C++ class instead)
+/// @note
+/// If this function returns \c bimap, then the caller should invoke
+/// \c FrozenKeyIdxBiMap_is_ok(&bimap) to test whether there was an error.
+/// This is pretty ugly/clunky, but it's the only practical way to achieve
+/// comparable behavior to other internal datatypes (ideally, we would make
+/// this a simple C++ class instead)
 inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
                                                int key_count, BiMapMode mode) {
   // this will be returned if there is an error
-  FrozenKeyIdxBiMap erroneous_obj{0, nullptr, BiMapMode::REFS_KEYDATA};
+  FrozenKeyIdxBiMap erroneous_obj{-1, nullptr, BiMapMode::REFS_KEYDATA};
 
   // check the specified keys
   long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
@@ -183,8 +183,8 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
 }
 
 /// returns whether new_FrozenKeyIdxBiMap constructed a valid object
-inline bool FrozenKeyIdxBiMap_is_ok(FrozenKeyIdxBiMap* ptr) {
-  return (ptr->length > 0);
+inline bool FrozenKeyIdxBiMap_is_ok(const FrozenKeyIdxBiMap* ptr) {
+  return (ptr->length != -1);
 }
 
 /// Destroys the specified FrozenKeyIdxBiMap
@@ -199,16 +199,16 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
   }
 }
 
-/// Makes a clone of the specified FrozenKeyIdxBiMap (the clone inherites the
+/// Makes a clone of the specified FrozenKeyIdxBiMap (the clone inherits the
 /// original BiMapMode).
 ///
-/// > [!note]
-/// > If this function returns `bimap`, then the caller should invoke
-/// > `FrozenKeyIdxBiMap_is_ok(&bimap)` to test whether there was an error.
-/// > This is pretty ugly/clunky, but its the only practical way to achieve
-/// > comparable behavior to other internal datatypes (ideally, we would make
-/// > this a simple C++ class instead)
-FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
+/// @note
+/// If this function returns \c bimap, then the caller should invoke
+/// \c FrozenKeyIdxBiMap_is_ok(&bimap) to test whether there was an error.
+/// This is pretty ugly/clunky, but it's the only practical way to achieve
+/// comparable behavior to other internal datatypes (ideally, we would make
+/// this a simple C++ class instead)
+inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
   return new_FrozenKeyIdxBiMap(ptr->keys, ptr->length, ptr->mode);
 };
 
@@ -246,4 +246,4 @@ inline const char* FrozenKeyIdxBiMap_key_from_idx(const FrozenKeyIdxBiMap* map,
 
 }  // namespace grackle::impl
 
-#endif  // UTILS_FROZENKEYIDXBIMAP_HPP
+#endif  // SUPPORT_FROZENKEYIDXBIMAP_HPP

From 0a8c9db350050ff50614c9b44cf8ff0b52c40fa2 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Mon, 5 Jan 2026 16:22:23 -0500
Subject: [PATCH 15/25] added a nice example illustrating how FrozenKeyIdxBiMap
 works

---
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index a928829ab..60af40833 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -17,6 +17,66 @@
 #include "support/FrozenKeyIdxBiMap.hpp"
 #include "grackle.h"
 
+// this top test was introduced to provide a more concrete example
+// of how we might use FrozenKeyIdxBiMap
+
+TEST(FrozenKeyIdxBiMap, FullExample) {
+  // THE SCENARIO: we have a list of unique ordered strings
+  //
+  // We are going build a FrozenKeyIdxBiMap instance from the following list.
+  // The resulting object is a bidirectional map that can both:
+  // 1. map a string to its index (at the time of construction) in the list.
+  //    - example: "HII" is mapped to 2
+  //    - example: "O2II" is mapped to 33
+  // 2. perform the reverse mapping (i.e. index -> string)
+  //    - example: 2 is mapped to "HII"
+  //    - example: 33 is mapped to "O2II"
+  //
+  // It's worth emphasizing that the mapping is frozen when its constructed &
+  // contents can't be changed (even if you reorder the original)
+  const char* keys[34] = {
+      "e",   "HI",   "HII", "HeI",  "HeII",  "HeIII", "HM",   "H2I",   "H2II",
+      "DI",  "DII",  "HDI", "DM",   "HDII",  "HeHII", "CI",   "CII",   "CO",
+      "CO2", "OI",   "OH",  "H2O",  "O2",    "SiI",   "SiOI", "SiO2I", "CH",
+      "CH2", "COII", "OII", "OHII", "H2OII", "H3OII", "O2II"};
+
+  namespace grimpl = grackle::impl;
+
+  // PART 1: build a FrozenKeyIdxBiMap from this list
+  // the 3rd argument tells the string to make copies of each string
+  grimpl::FrozenKeyIdxBiMap m = grimpl::new_FrozenKeyIdxBiMap(
+      keys, 34, grimpl::BiMapMode::COPIES_KEYDATA);
+
+  // before we use it, we should confirm the constructor succeeded
+  if (!grimpl::FrozenKeyIdxBiMap_is_ok(&m)) {
+    FAIL() << "creation of the m failed unexpectedly";
+  }
+
+  // PART 2: let's show some examples of lookups from names
+
+  // Equivalent Python/idiomatic C++:  `2 == m["HII"]`
+  EXPECT_EQ(2, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "HII"));
+  // Equivalent Python/idiomatic C++:  `33 == m["O2II"]`
+  EXPECT_EQ(33, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "O2II"));
+
+  // Behavior is well-defined when a key isn't known
+  int invalid = grimpl::bimap::invalid_val;
+  EXPECT_EQ(invalid, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "NotAKey"));
+
+  // PART 3: let's show the reverse of the previous lookups
+  EXPECT_STREQ("HII", grimpl::FrozenKeyIdxBiMap_key_from_idx(&m, 2));
+  EXPECT_STREQ("O2II", grimpl::FrozenKeyIdxBiMap_key_from_idx(&m, 33));
+
+  // Behavior is again well-defined when passing an invalid index
+  EXPECT_EQ(nullptr, grimpl::FrozenKeyIdxBiMap_key_from_idx(&m, 131));
+
+  // PART 4: We can also query the length
+  EXPECT_EQ(34, grimpl::FrozenKeyIdxBiMap_size(&m));
+
+  // Finally, to cleanup we will deallocate data tracked internally by `m`
+  grimpl::drop_FrozenKeyIdxBiMap(&m);
+}
+
 class FrozenKeyIdxBiMapConstructorSuite
     : public testing::TestWithParam<grackle::impl::BiMapMode> {
   // You can implement all the usual fixture class members here.

From 967a3e6abc4011e698f05422eb2aea0b2e5ab284 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Mon, 5 Jan 2026 16:53:20 -0500
Subject: [PATCH 16/25] more rigorously tested empty bimaps and fixed bugs

---
 src/clib/support/FrozenKeyIdxBiMap.hpp     |  4 ++
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 54 +++++++++++++++++++++-
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index e9ddebada..9798d66bb 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -122,6 +122,10 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
   // this will be returned if there is an error
   FrozenKeyIdxBiMap erroneous_obj{-1, nullptr, BiMapMode::REFS_KEYDATA};
 
+  if (keys == nullptr && key_count == 0) {
+    return FrozenKeyIdxBiMap{0, nullptr, mode};
+  }
+
   // check the specified keys
   long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
   if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 60af40833..07345e38c 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -77,6 +77,49 @@ TEST(FrozenKeyIdxBiMap, FullExample) {
   grimpl::drop_FrozenKeyIdxBiMap(&m);
 }
 
+// validate basic operations for an empty bimap
+TEST(FrozenKeyIdxBiMap, EmptyBasicOps) {
+  grackle::impl::FrozenKeyIdxBiMap m = grackle::impl::new_FrozenKeyIdxBiMap(
+      nullptr, 0, grackle::impl::BiMapMode::COPIES_KEYDATA);
+  ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&m))
+      << "construction of a FrozenKeyIdxBiMap unexpectedly failed";
+
+  EXPECT_EQ(0, grackle::impl::FrozenKeyIdxBiMap_size(&m))
+      << "an empty mapping should have a size of 0";
+
+  EXPECT_EQ(grackle::impl::bimap::invalid_val,
+            grackle::impl::FrozenKeyIdxBiMap_idx_from_key(&m, "NotAKey"))
+      << "key lookup should always fail for an empty mapping";
+
+  EXPECT_EQ(nullptr, grackle::impl::FrozenKeyIdxBiMap_key_from_idx(&m, 0))
+      << "index lookup should always fail for an empty mapping";
+
+  grackle::impl::drop_FrozenKeyIdxBiMap(&m);
+}
+
+// validate behavior of clone for an empty bimap
+TEST(FrozenKeyIdxBiMap, EmptyClone) {
+  // make the original
+  grackle::impl::FrozenKeyIdxBiMap m = grackle::impl::new_FrozenKeyIdxBiMap(
+      nullptr, 0, grackle::impl::BiMapMode::COPIES_KEYDATA);
+  ASSERT_TRUE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&m))
+      << "construction of a FrozenKeyIdxBiMap unexpectedly failed";
+
+  // make the clone
+  grackle::impl::FrozenKeyIdxBiMap m_clone =
+      grackle::impl::FrozenKeyIdxBiMap_clone(&m);
+
+  bool success = grackle::impl::FrozenKeyIdxBiMap_is_ok(&m_clone);
+
+  grackle::impl::drop_FrozenKeyIdxBiMap(&m);  // drop the original
+
+  if (success) {
+    grackle::impl::drop_FrozenKeyIdxBiMap(&m_clone);
+  } else {
+    FAIL() << "cloning an empty mapping failed!";
+  }
+}
+
 class FrozenKeyIdxBiMapConstructorSuite
     : public testing::TestWithParam<grackle::impl::BiMapMode> {
   // You can implement all the usual fixture class members here.
@@ -123,9 +166,16 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, 0LenKey) {
   ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 
-TEST_P(FrozenKeyIdxBiMapConstructorSuite, NoKeys) {
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, NullptrWithPosCount) {
+  grackle::impl::FrozenKeyIdxBiMap tmp =
+      grackle::impl::new_FrozenKeyIdxBiMap(nullptr, 1, GetParam());
+  ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
+}
+
+TEST_P(FrozenKeyIdxBiMapConstructorSuite, NotNull0KeyCount) {
+  const char* keys[] = {"denisty", "internal_energy"};
   grackle::impl::FrozenKeyIdxBiMap tmp =
-      grackle::impl::new_FrozenKeyIdxBiMap(nullptr, 0, GetParam());
+      grackle::impl::new_FrozenKeyIdxBiMap(keys, 0, GetParam());
   ASSERT_FALSE(grackle::impl::FrozenKeyIdxBiMap_is_ok(&tmp));
 }
 

From f515c268628b524a61d06c29e1e44becfbe29312 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Tue, 6 Jan 2026 15:39:23 -0500
Subject: [PATCH 17/25] a bunch of assorted changes

---
 src/clib/support/FrozenKeyIdxBiMap.hpp | 199 +++++++++++++++++--------
 1 file changed, 138 insertions(+), 61 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index 9798d66bb..b00c0d0e0 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -12,14 +12,8 @@
 /// bookkeeping.
 ///
 /// The underlying implementation of this type is *highly* suboptimal (it's
-/// simplistic at the cost of speed). PR #270 introduce a replacement that
-/// maintains almost the exact same API (the new_FrozenKeyIdxBiMap,
-/// FrozenKeyIdxBiMap_clone, and drop_FrozenKeyIdxBiMap functions will need to
-/// be tweaked)
-///
-/// If we decide to more fully embrace C++, it would make a LOT of sense to
-/// convert this to a full-blown class (we would probably delete the copy
-/// constructor And copy assignement methods OR adopt reference counting).
+/// simplistic at the cost of speed). PR #484 introduce a drop-in replacement
+/// the API won't change that is much faster
 ///
 //===----------------------------------------------------------------------===//
 #ifndef SUPPORT_FROZENKEYIDXBIMAP_HPP
@@ -33,7 +27,7 @@
 #include "grackle.h"
 #include "status_reporting.h"
 
-// the motivation for these constants are provided in PR #270 (they are related
+// the motivation for these constants are provided in PR #484 (they are related
 // to some optimizations in the FrozenKeyIdxBiMap implementation)
 namespace grackle::impl::bimap {
 /// specifies an invalid value of the map (we state that you can't store the
@@ -45,42 +39,61 @@ inline constexpr std::uint16_t invalid_val =
 inline constexpr std::uint16_t keylen_max = 29;
 }  // namespace grackle::impl::bimap
 
-// these are just here for to make it easier for us to adopt changes from PR
-// #270 (then we can delete these macros)
-#define STRU16MAP_INVALID_VAL grackle::impl::bimap::invalid_val
-#define STRU16MAP_KEYLEN_MAX grackle::impl::bimap::keylen_max
-
 namespace grackle::impl {
 
-enum class BiMapMode { REFS_KEYDATA = 0, COPIES_KEYDATA = 1 };
-
-/// This is a bidirectional map (bimap). It is specialized to map @c n
-/// unique string keys to unique indexes with values of @c 0` through @c (n-1)
-/// and  vice versa. The ordering of keys is set at initialization and frozen.
-///
-/// This is primarily intended to be used in the implementation of Maps of
-/// arrays (where the values could be part of a single contiguous array or are
-/// individual arrays), but this but may be broadly useful for other
-/// applications.
-///
-/// This operates in 2 modes:
-/// 1. @ref BiMapMode::REFS_KEYDATA This is the default, where we operate
-///    under the assumption that the allocations holding the string characters
-///    outlive  the bimap. In this mode the bimap is intended to hold
-///    string-literals. (which are live for the entirety of a program). This
-///    minimizes memory usage.
-/// 2. @ref BiMapMode::COPIES_KEYDATA Under this mode, the bimap copies the
-///    data of all keys. This is useful for testing purposes. In the long-term,
-///    if we allow dynamic extension of chemistry networks, it will also be
-///    useful. If we are implement the optimizations described down below
-///    (where we directly embed the string in the hash-table-rows), this will
-///    probably be a quite a bit faster
-///
-/// @par Replacement in PR #270
+// the following doxygen comment block logically groups every all parts of
+// the (internal) API for Grackle's (internal) FrozenKeyIdxBiMap. It's useful
+// when generating a doxygen webpage
+
+/// @defgroup bimap-grp FrozenKeyIdxBiMap Data Type
+///
+/// FrozenKeyIdxBiMap provides specialized mapping functionality for internal
+/// use within Grackle. The functionality is useful as a building-block for
+/// runtime lookup-tables and other data types with map-like interface.
+///
+/// The data type was implemented in a C-style. The FrozenKeyIdxBiMap struct
+/// should be treated as an opaque type that is operated upon by a set of
+/// associated functions. More idiomatic C++ (or languages like Rust & Swift),
+/// the associated functions would be attached to the struct as methods
+/** @{*/
+
+/// describes the operating modes of @ref FrozenKeyIdxBiMap
+enum class BiMapMode {
+  /// The preferred default mode, where the creation of a BiMap involves making
+  /// copies of each key (cleaning up a BiMap will deallocate the copies)
+  ///
+  /// In general, this is much safer, and it will be @b very useful in the
+  /// longer-term if we allow dynamic extension of chemistry networks. If we
+  /// adopt the embedded-key optimization (discussed in the FrozenKeyIdxBiMap),
+  /// this mode will probably be significantly faster.
+  COPIES_KEYDATA = 1,
+  /// This mode aims to reduce memory usage by having the BiMap reference
+  /// external keys. In other words, the BiMap won't attempt to manage
+  /// allocations holding each character in a string.
+  ///
+  /// @warning
+  /// For safety this should @b ONLY be used when all keys are immutable
+  /// string-literals (i.e. when the strings are valid for program's duration)
+  REFS_KEYDATA = 0,
+};
+
+/// @brief A bidirectional map (bimap), specialized to map @c n unique string
+/// keys to unique indexes with values of @c 0 through @c (n-1) and
+/// vice versa. The ordering & values of keys are set at creation and frozen.
+///
+/// This type is useful in a number of scenarios. For example, it can be used
+/// to implement a type representing a Map of arrays (where the values could
+/// be part of a single contiguous array or are individual arrays).
+///
+/// This type operates in 2 modes: @ref BiMapMode::COPIES_KEYDATA and
+/// @ref BiMapMode::REFS_KEYDATA. Their docstrings provide more context. When
+/// in doubt, prefer the former mode.
+///
+/// @par Replacement in PR #484
 /// The current implementation is extremely oversimplified and inefficient! It
 /// doesn't even use a hash table. The purpose is to create a simple abstract
 /// data structure for which the implementation will be dramatically improved
-/// by PR #270 (but the interface won't be touched at all).
+/// by PR #484 (but the interface won't be touched at all).
 ///
 /// @par
 /// The PR with the improved version, also updates this docstring with a
@@ -98,8 +111,7 @@ struct FrozenKeyIdxBiMap {
   int length;
   /// array of keys
   const char** keys;
-  /// indicates whether the map "owns" the memory holding the characters in
-  /// each key or just references it
+  /// specifies ownership of keys, @see BiMapMode
   BiMapMode mode;
 };
 
@@ -112,11 +124,11 @@ struct FrozenKeyIdxBiMap {
 ///     clones that are made.
 ///
 /// @note
-/// If this function returns \c bimap, then the caller should invoke
-/// \c FrozenKeyIdxBiMap_is_ok(&bimap) to test whether there was an error.
-/// This is pretty ugly/clunky, but it's the only practical way to achieve
-/// comparable behavior to other internal datatypes (ideally, we would make
-/// this a simple C++ class instead)
+/// Callers should pass the returned value to @ref FrozenKeyIdxBiMap_is_ok
+/// to check whether there was an error during creation. This is pretty
+/// ugly/clunky, but it's the only practical way to achieve comparable behavior
+/// to other internal data types. The best alternatives involve things like
+/// std::optional or converting this type to a simple C++ class.
 inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
                                                int key_count, BiMapMode mode) {
   // this will be returned if there is an error
@@ -186,12 +198,41 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
                            /* mode = */ mode};
 }
 
-/// returns whether new_FrozenKeyIdxBiMap constructed a valid object
+/// checks whether a creational function produced a valid bimap
+///
+/// @param[in] ptr Points to the object being checked
+/// @return true if the value is ok or false if the value is invalid
+///
+/// @important
+/// The interface of @ref FrozenKeyIdxBiMap sets values in a very particular
+/// way to signal that FrozenKeyIdxBiMap is in an invalid state. This function
+/// @b ONLY checks for that particular signature.
 inline bool FrozenKeyIdxBiMap_is_ok(const FrozenKeyIdxBiMap* ptr) {
   return (ptr->length != -1);
 }
 
-/// Destroys the specified FrozenKeyIdxBiMap
+/// Destroys the internal data tracked by an instance
+///
+/// @param[in] ptr A non-null pointer to a valid bimap instance
+///
+/// @warning
+/// As with any C datatype, care is required to avoid issues with internal
+/// dangling pointers. YOU SHOULD ONLY CALL THIS ONCE for a given instance
+/// (and only if the instance was properly by the interface)
+/// - while some efforts are made to reduce the possiblity of issues, some
+///   things just can't be avoided (especially when it comes to shallow copies)
+/// - here's a problematic example:
+///   @code{.cpp}
+///   FrozenKeyIdxBiMap bimap = new_FrozenKeyIdxBiMap( /*<args...>*/ );
+///   // (the FrozenKeyIdxBiMap_is_ok check is elided for brevity)
+///
+///   // you should generally avoid shallow copies (if possible)
+///   FrozenKeyIdxBiMap shallow_cpy = bimap;
+///
+///   // problems arise below (if we swap order, the 2nd call is still bad)
+///   drop_FrozenKeyIdxBiMap(&shallow_cpy);  // <- this is OK
+///   drop_FrozenKeyIdxBiMap(&bimap);        // <- this is BAD
+///   @endcode
 inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
   if (ptr->mode == BiMapMode::COPIES_KEYDATA) {
     for (int i = 0; i < ptr->length; i++) {
@@ -203,21 +244,31 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
   }
 }
 
-/// Makes a clone of the specified FrozenKeyIdxBiMap (the clone inherits the
-/// original BiMapMode).
+/// Makes a clone of the specified FrozenKeyIdxBiMap
+///
+/// The clone inherits the original's BiMapMode value. If it held
+/// BiMapMode::COPIES_KEYDATA, then fresh copies of the strings are made
 ///
 /// @note
-/// If this function returns \c bimap, then the caller should invoke
-/// \c FrozenKeyIdxBiMap_is_ok(&bimap) to test whether there was an error.
-/// This is pretty ugly/clunky, but it's the only practical way to achieve
-/// comparable behavior to other internal datatypes (ideally, we would make
-/// this a simple C++ class instead)
+/// Callers should pass the returned value to @ref FrozenKeyIdxBiMap_is_ok
+/// to check whether there was an error during creation. This is pretty
+/// ugly/clunky, but it's the only practical way to achieve comparable behavior
+/// to other internal data types. The best alternatives involve things like
+/// std::optional or converting this type to a simple C++ class.
 inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
   return new_FrozenKeyIdxBiMap(ptr->keys, ptr->length, ptr->mode);
 };
 
-/// returns the value associated with the key or (if the key can't be found)
-/// @ref grackle::impl::bimap::invalid_val
+/// lookup the value associated with the key
+///
+/// This is the analog to calling `map[key]` in python. In practice, the
+/// semantics are more similar to calling `map.get(key,invalid_val)`
+///
+/// @param[in] map A pointer to a valid bimap
+/// @param[in] key A null-terminated string
+///
+/// @return the key's associated value or, if the key can't be found,
+///     @ref grackle::impl::bimap::invalid_val
 inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
     const FrozenKeyIdxBiMap* map, const char* key) {
   GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
@@ -229,17 +280,41 @@ inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
   return bimap::invalid_val;
 }
 
-/// checks if the map contains a key
-inline int FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
-                                      const char* key) {
+/// returns whether the map contains the key
+///
+/// @param[in] map A pointer to a valid bimap
+/// @param[in] key A null-terminated string
+inline bool FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
+                                       const char* key) {
   return FrozenKeyIdxBiMap_idx_from_key(map, key) != bimap::invalid_val;
 }
 
+/// return the number of keys in the map
+///
+/// @param[in] map A pointer to a valid bimap
 inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map) {
   return map->length;
 }
 
 /// Return the ith key (this is effectively a reverse lookup)
+///
+/// For some context, if this function returns a string `s` for some index `i`,
+/// then a call to @ref FrozenKeyIdxBiMap_idx_from_key that passes `s` will
+/// return `i`
+///
+/// This is intended for use in situations where you briefly need the string
+/// (i.e. and you plan to stop using the pointer before or at the same time as
+/// the @p map is destroyed). In more detail:
+/// - If the @p map was constructed in @ref BiMapMode::COPIES_KEYDATA mode,
+///   returned strings have the same lifetime as @p map (i.e. they are
+///   deallocated when the contents of @p map are deallocated).
+/// - Otherwise, the returned string's allocation is externally managed. But,
+///   any scenario where the allocation doesn't live at least as long as @p map,
+///   is ill-formed
+///
+/// @param[in] map A pointer to a valid bimap
+/// @param[in] i The returned index
+/// @return The pointer to the appropriate key
 inline const char* FrozenKeyIdxBiMap_key_from_idx(const FrozenKeyIdxBiMap* map,
                                                   std::uint16_t i) {
   if (i >= map->length) {
@@ -248,6 +323,8 @@ inline const char* FrozenKeyIdxBiMap_key_from_idx(const FrozenKeyIdxBiMap* map,
   return map->keys[i];  // this can't be a nullptr
 }
 
+/** @}*/  // end of group
+
 }  // namespace grackle::impl
 
 #endif  // SUPPORT_FROZENKEYIDXBIMAP_HPP

From 547251da6b5ee6bf0dc707d044e964dbf9e6fde1 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Mon, 5 Jan 2026 19:05:14 -0500
Subject: [PATCH 18/25] introduce FNV1a hash function

---
 src/clib/CMakeLists.txt         |  1 +
 src/clib/support/fnv1a_hash.hpp | 63 +++++++++++++++++++++++++++++++++
 tests/unit/CMakeLists.txt       |  3 +-
 tests/unit/test_unit_hash.cpp   | 61 +++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 src/clib/support/fnv1a_hash.hpp
 create mode 100644 tests/unit/test_unit_hash.cpp

diff --git a/src/clib/CMakeLists.txt b/src/clib/CMakeLists.txt
index afd214763..4cd193789 100644
--- a/src/clib/CMakeLists.txt
+++ b/src/clib/CMakeLists.txt
@@ -127,6 +127,7 @@ add_library(Grackle_Grackle
   step_rate_gauss_seidel.hpp
   step_rate_newton_raphson.hpp
   support/FrozenKeyIdxBiMap.hpp
+  support/fnv1a_hash.hpp
   time_deriv_0d.hpp
   utils-cpp.cpp utils-cpp.hpp
   utils-field.hpp
diff --git a/src/clib/support/fnv1a_hash.hpp b/src/clib/support/fnv1a_hash.hpp
new file mode 100644
index 000000000..6aafca022
--- /dev/null
+++ b/src/clib/support/fnv1a_hash.hpp
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// See the LICENSE file for license and copyright information
+// SPDX-License-Identifier: NCSA AND BSD-3-Clause
+//
+//===----------------------------------------------------------------------===//
+///
+/// @file
+/// implements the 32-bit fnv-1a hash function
+///
+//===----------------------------------------------------------------------===//
+#ifndef SUPPORT_FNV1A_HASH_HPP
+#define SUPPORT_FNV1A_HASH_HPP
+
+#include <cstdint>
+#include <limits>
+
+namespace grackle::impl {
+
+/// Holds the result of a call to fnv1a_hash
+struct HashRsltPack {
+  bool success;
+  std::uint16_t keylen;
+  std::uint32_t hash;
+};
+
+/// calculate 32-bit FNV-1a hash of key and measures the key's length.
+///
+/// @tparam MaxKeyLen the max number of characters in key (excluding '\0'). By
+///     default, it's the largest value HashRsltPack::keylen holds. A smaller
+///     value can be specified as an optimization.
+/// @param key the null-terminated string. Behavior is deliberately undefined
+///     when passed a `nullptr`
+///
+/// @note
+/// The current implementation prioritizes convenience. We may want to evaluate
+/// whether alternatives (e.g. fxhash) are faster or have fewer collisions with
+/// our typical keys.
+///
+/// @warning
+/// Obviously this is @b NOT cryptographically secure
+template <int MaxKeyLen = std::numeric_limits<std::uint16_t>::max()>
+HashRsltPack fnv1a_hash(const char* key) {
+  static_assert(
+      0 <= MaxKeyLen && MaxKeyLen <= std::numeric_limits<std::uint16_t>::max(),
+      "MaxKeyLen can't be encoded by HashRsltPack");
+
+  constexpr std::uint32_t prime = 16777619;
+  constexpr std::uint32_t offset = 2166136261;
+
+  std::uint32_t hash = offset;
+  for (int i = 0; i <= MaxKeyLen; i++) {  // the `<=` is intentional
+    if (key[i] == '\0') {
+      return HashRsltPack{true, static_cast<std::uint16_t>(i), hash};
+    }
+    hash = (hash ^ key[i]) * prime;
+  }
+  return HashRsltPack{false, 0, 0};
+}
+
+}  // namespace grackle::impl
+
+#endif  // SUPPORT_FNV1A_HASH_HPP
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
index d09e1c7c4..60d67a737 100644
--- a/tests/unit/CMakeLists.txt
+++ b/tests/unit/CMakeLists.txt
@@ -48,7 +48,8 @@ target_compile_definitions(grtest_utils
 
 # start declaring targets for tests
 # ---------------------------------
-add_executable(testFrozenKeyIdxBiMap test_unit_FrozenKeyIdxBiMap.cpp)
+add_executable(testFrozenKeyIdxBiMap
+  test_unit_hash.cpp test_unit_FrozenKeyIdxBiMap.cpp)
 target_link_libraries(testFrozenKeyIdxBiMap testdeps)
 gtest_discover_tests(testFrozenKeyIdxBiMap)
 
diff --git a/tests/unit/test_unit_hash.cpp b/tests/unit/test_unit_hash.cpp
new file mode 100644
index 000000000..101347ad4
--- /dev/null
+++ b/tests/unit/test_unit_hash.cpp
@@ -0,0 +1,61 @@
+//===----------------------------------------------------------------------===//
+//
+// See the LICENSE file for license and copyright information
+// SPDX-License-Identifier: NCSA AND BSD-3-Clause
+//
+//===----------------------------------------------------------------------===//
+///
+/// @file
+/// Check correctness of the hash function
+///
+//===----------------------------------------------------------------------===//
+
+#include <gtest/gtest.h>
+#include "support/fnv1a_hash.hpp"
+#include <iomanip>
+#include <ostream>
+
+namespace grackle::impl {
+/// Teach GTest how to print HashRsltPack
+/// @note it's important this is in the same namespace as HashRsltPack
+void PrintTo(const HashRsltPack& pack, std::ostream* os) {
+  *os << "{success: " << pack.success << ", keylen: " << pack.keylen
+      << ", hash: 0x" << std::setfill('0')
+      << std::setw(8)  // u32 has 8 hex digits
+      << std::hex << pack.hash << "}";
+}
+
+bool operator==(const HashRsltPack& a, const HashRsltPack& b) {
+  return a.success == b.success && a.keylen == b.keylen && a.hash == b.hash;
+}
+
+}  // namespace grackle::impl
+
+// the test answers primarily came from Appendix C of
+// https://datatracker.ietf.org/doc/html/draft-eastlake-fnv-17
+
+TEST(FNV1a, EmptyString) {
+  grackle::impl::HashRsltPack expected{true, 0, 0x811c9dc5ULL};
+  ASSERT_EQ(grackle::impl::fnv1a_hash(""), expected);
+}
+
+TEST(FNV1a, aString) {
+  grackle::impl::HashRsltPack expected{true, 1, 0xe40c292cULL};
+  ASSERT_EQ(grackle::impl::fnv1a_hash("a"), expected);
+}
+
+TEST(FNV1a, foobarString) {
+  grackle::impl::HashRsltPack expected{true, 6, 0xbf9cf968ULL};
+  ASSERT_EQ(grackle::impl::fnv1a_hash("foobar"), expected);
+}
+
+TEST(FNV1a, MaxSizeString) {
+  constexpr int MaxKeyLen = 6;  // <- exactly matches the key's length
+  grackle::impl::HashRsltPack expected{true, 6, 0xbf9cf968ULL};
+  ASSERT_EQ(grackle::impl::fnv1a_hash("foobar"), expected);
+}
+
+TEST(FNV1a, TooLongString) {
+  constexpr int MaxKeyLen = 5;  // <- shorter than the queried key
+  ASSERT_FALSE(grackle::impl::fnv1a_hash<MaxKeyLen>("foobar").success);
+}

From 1a3fb006a094801e228bde07e6c1272b61760d6b Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Tue, 6 Jan 2026 15:50:01 -0500
Subject: [PATCH 19/25] Overhaul the FrozenKeyIdxBiMap

---
 src/clib/CMakeLists.txt                       |   1 +
 src/clib/support/FrozenKeyIdxBiMap.hpp        | 359 ++++++++++++------
 src/clib/support/FrozenKeyIdxBiMap_detail.hpp | 198 ++++++++++
 3 files changed, 439 insertions(+), 119 deletions(-)
 create mode 100644 src/clib/support/FrozenKeyIdxBiMap_detail.hpp

diff --git a/src/clib/CMakeLists.txt b/src/clib/CMakeLists.txt
index 4cd193789..7d43b9997 100644
--- a/src/clib/CMakeLists.txt
+++ b/src/clib/CMakeLists.txt
@@ -127,6 +127,7 @@ add_library(Grackle_Grackle
   step_rate_gauss_seidel.hpp
   step_rate_newton_raphson.hpp
   support/FrozenKeyIdxBiMap.hpp
+  support/FrozenKeyIdxBiMap_detail.hpp
   support/fnv1a_hash.hpp
   time_deriv_0d.hpp
   utils-cpp.cpp utils-cpp.hpp
diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index 7cfef83ec..b627c1a13 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -8,13 +8,6 @@
 /// @file
 /// Declares the internal FrozenKeyIdxBiMap type
 ///
-/// All insertions occur during initialization. This simplifies a lot of
-/// bookkeeping.
-///
-/// The underlying implementation of this type is *highly* suboptimal (it's
-/// simplistic at the cost of speed). PR #484 introduce a drop-in replacement
-/// the API won't change that is much faster
-///
 //===----------------------------------------------------------------------===//
 #ifndef SUPPORT_FROZENKEYIDXBIMAP_HPP
 #define SUPPORT_FROZENKEYIDXBIMAP_HPP
@@ -22,22 +15,9 @@
 #include <cstdint>
 #include <cstdlib>
 #include <cstring>
-#include <limits>
 
-#include "grackle.h"
 #include "status_reporting.h"
-
-// the motivation for these constants are provided in PR #484 (they are related
-// to some optimizations in the FrozenKeyIdxBiMap implementation)
-namespace grackle::impl::bimap {
-/// specifies an invalid value of the map (we state that you can't store the
-/// maximum u16 value)
-inline constexpr std::uint16_t invalid_val =
-    std::numeric_limits<std::uint16_t>::max();
-
-/// specifies maximum allowed length of a key (excluding the null character).
-inline constexpr std::uint16_t keylen_max = 29;
-}  // namespace grackle::impl::bimap
+#include "FrozenKeyIdxBiMap_detail.hpp"
 
 namespace grackle::impl {
 
@@ -89,16 +69,81 @@ enum class BiMapMode {
 /// @ref BiMapMode::REFS_KEYDATA. Their docstrings provide more context. When
 /// in doubt, prefer the former mode.
 ///
-/// @par Replacement in PR #484
-/// The current implementation is extremely oversimplified and inefficient! It
-/// doesn't even use a hash table. The purpose is to create a simple abstract
-/// data structure for which the implementation will be dramatically improved
-/// by PR #484 (but the interface won't be touched at all).
+/// @par Implementation Notes
+/// At the time of writing, the type is primarily implemented in terms of
+/// a hash table that uses open-addressing with linear probing to resolve
+/// collisions. The implementation heavily draws from logic I wrote for Enzo-E:
+///   https://github.com/enzo-project/enzo-e/blob/main/src/Cello/view_StringIndRdOnlyMap.hpp
+/// (More details are provided below under C++ considerations)
+///
+/// @par Why Frozen?
+/// The contents are "frozen" for 3 primary reasons:
+/// 1. It drastically simplifies the implementation (we don't have to worry
+///    about deletion -- which can be quite messy)
+/// 2. Linear-probing generally provides better data locality than other hash
+///    collision resolution techniques, but generally has other drawbacks.
+///    Freezing the contents lets us mitigate many drawbacks (mostly related to
+///    the deletion operation)
+/// 3. It could let us make copy operations cheaper. If we know the map won't
+///    change, we could just use reference counting.
+///
+/// @par Consideration: Reference Counting
+/// The original C++ leverages @c std::shared_ptr to achieve reference counting
+/// (and reduce the cost of copying). Theoretically, I would like to see us use
+/// some kind of reference-counting too. But this is tricky in library code,
+/// given the diversity of threading libraries that are not formally
+/// interoperable. I think the only way to properly do this would be to come up
+/// with a system for allowing registration of locks/atomics with Grackle as a
+/// whole.
+///
+/// @par C++ Considerations
+/// It would definitely be worth evaluating whether we should embrace C++
+/// in order to convert this to a full-blown class and adopt characteristics
+/// present in the original Enzo-E version:
+/// - most importantly, it would greatly reduce the chance of memory leaks
+/// - (much less importantly) it would be a lot more ergonomic (& less clunky)
+/// - But, for reasons expressed above, I am concerned about using
+///   @c std::shared_ptr for reference counting.
 ///
 /// @par
-/// The PR with the improved version, also updates this docstring with a
-/// detailed explanation of design decisions (like why the contents
-/// are "frozen") and highlights a number of potential improvements.
+/// I would be stunned if <tt> std::map<std::string, std::uint16_t> </tt> or
+/// <tt> std::map<const char*, std::uint16_t> </tt> is faster than the internal
+/// hash table since @c std::map is usually implemented as a tree.
+///
+/// @par Potential Improvements
+/// Simple Ideas:
+/// - We could be smarter about the order that we insert keys into the table
+///   (in the constructor) to minimize the search time.
+/// - We might be able to come up with a better hash function
+///
+/// @par
+/// A more ambitious idea is to embed string allocations within the rows for
+/// @ref BiMapMode::COPIES_KEYDATA mode. This is possible thanks to the fact
+/// that we use @ref bimap::keylen_max to limit the size of keys.
+/// - Essentially, we would replace @ref bimap_StrU16_detail::Row with
+///   something like the following:
+///   @code{.cpp}
+///   struct alignas(32) PackedRow { char data[32]; };
+///
+///   bool is_empty(PackedRow& r) { return data[0] == '\0' }
+///   const char* get_key(PackedRow r) { return r.data; }
+///   std::uint16_t get_val(Packed r) {
+///     stdd::uint16_t o;
+///     std::memcpy(&o, r.data+30, 2);
+///     return o;
+///   }
+///   @endcode
+/// - additional context about the preceding snippet:
+///   - when empty, a @c PackedRow::data is filled with '\0'
+///   - otherwise, @c PackedRow::data encodes the key-value pair:
+///     - data[0:30] is the null-terminated key string ('\0' fills unused space)
+///     - data[30:32] encodes the 16-bit value
+///   - @c alignas(32) primarily ensures better cacheline alignment.
+/// - Benefits of this change:
+///   1. better locality (if @c PackedRow is in the cache, so is the key-string)
+///   2. probing can use memcmp without a checking whether a row is empty
+///   3. with a little extra care, we could use the forced alignment of
+///      @c PackedRow::data to compare strings with SIMD instructions
 ///
 /// @note
 /// The contents of this struct should be considered an implementation
@@ -108,17 +153,33 @@ struct FrozenKeyIdxBiMap {
   // don't forget to update FrozenKeyIdxBiMap_clone when changing members
 
   /// the number of contained strings
-  int length;
-  /// array of keys
-  const char** keys;
+  bimap::rowidx_type length;
+  /// the number of elements in table_rows
+  bimap::rowidx_type capacity;
+  /// max number of rows that must be probed to determine if a key is contained
+  bimap::rowidx_type max_probe;
   /// specifies ownership of keys, @see BiMapMode
   BiMapMode mode;
+
+  /// actual hash table data
+  bimap_StrU16_detail::Row* table_rows;
+  /// tracks the row indices to make iteration faster
+  bimap::rowidx_type* ordered_row_indices;
 };
 
-// ugh, it's unfortunate that we need to make this... but for now its useful.
-// Ideally, we would refactor so that we can get rid of this function.
+/// Create an invalid FrozenKeyIdxBiMap
+///
+/// @note
+/// ugh, it's unfortunate that we need to make this... but for now it's useful.
+/// Ideally, we would refactor so that we can get rid of this function. A
+/// useful compromise might simply put it within the bimap_detail namespace
 inline FrozenKeyIdxBiMap mk_invalid_FrozenKeyIdxBiMap() {
-  return FrozenKeyIdxBiMap{-1, nullptr, BiMapMode::REFS_KEYDATA};
+  return FrozenKeyIdxBiMap{bimap::invalid_val,
+                           bimap::invalid_val,
+                           0,
+                           BiMapMode::REFS_KEYDATA,
+                           nullptr,
+                           nullptr};
 }
 
 /// Constructs a new FrozenKeyIdxBiMap
@@ -135,71 +196,8 @@ inline FrozenKeyIdxBiMap mk_invalid_FrozenKeyIdxBiMap() {
 /// ugly/clunky, but it's the only practical way to achieve comparable behavior
 /// to other internal data types. The best alternatives involve things like
 /// std::optional or converting this type to a simple C++ class.
-inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
-                                               int key_count, BiMapMode mode) {
-  if (keys == nullptr && key_count == 0) {
-    return FrozenKeyIdxBiMap{0, nullptr, mode};
-  }
-
-  // check the specified keys
-  long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
-  if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
-    GrPrintErrMsg("key_count must be positive and cannot exceed %lld",
-                  max_keys);
-    return mk_invalid_FrozenKeyIdxBiMap();
-  } else if (keys == nullptr) {
-    GrPrintErrMsg("keys must not be a nullptr");
-    return mk_invalid_FrozenKeyIdxBiMap();
-  }
-  for (int i = 0; i < key_count; i++) {
-    GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
-    std::size_t n_chrs_without_nul = std::strlen(keys[i]);
-    if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap::keylen_max) {
-      GrPrintErrMsg(
-          "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
-          "exceeding %d",
-          keys[i], i, bimap::keylen_max);
-      return mk_invalid_FrozenKeyIdxBiMap();
-    }
-    // check uniqueness
-    for (int j = 0; j < i; j++) {
-      if (strcmp(keys[i], keys[j]) == 0) {
-        GrPrintErrMsg("\"%s\" key repeats", keys[i]);
-        return mk_invalid_FrozenKeyIdxBiMap();
-      }
-    }
-  }
-
-  // now, actually construct the result
-  const char** out_keys = nullptr;
-  switch (mode) {
-    case BiMapMode::REFS_KEYDATA: {
-      out_keys = new const char*[key_count];
-      for (int i = 0; i < key_count; i++) {
-        out_keys[i] = keys[i];
-      }
-      break;
-    }
-    case BiMapMode::COPIES_KEYDATA: {
-      char** tmp_keys = new char*[key_count];
-      for (int i = 0; i < key_count; i++) {
-        std::size_t n_chrs_without_nul = std::strlen(keys[i]);
-        tmp_keys[i] = new char[n_chrs_without_nul + 1];
-        std::memcpy(tmp_keys[i], keys[i], n_chrs_without_nul + 1);
-      }
-      out_keys = (const char**)tmp_keys;
-      break;
-    }
-    default: {
-      GrPrintErrMsg("unknown mode");
-      return mk_invalid_FrozenKeyIdxBiMap();
-    }
-  }
-
-  return FrozenKeyIdxBiMap{/* length = */ key_count,
-                           /* keys = */ out_keys,
-                           /* mode = */ mode};
-}
+FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[], int key_count,
+                                        BiMapMode mode);
 
 /// checks whether a creational function produced a valid bimap
 ///
@@ -211,7 +209,7 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
 /// way to signal that FrozenKeyIdxBiMap is in an invalid state. This function
 /// @b ONLY checks for that particular signature.
 inline bool FrozenKeyIdxBiMap_is_ok(const FrozenKeyIdxBiMap* ptr) {
-  return (ptr->length != -1);
+  return ptr->length != bimap::invalid_val;
 }
 
 /// Destroys the internal data tracked by an instance
@@ -237,13 +235,22 @@ inline bool FrozenKeyIdxBiMap_is_ok(const FrozenKeyIdxBiMap* ptr) {
 ///   drop_FrozenKeyIdxBiMap(&bimap);        // <- this is BAD
 ///   @endcode
 inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
-  if (ptr->mode == BiMapMode::COPIES_KEYDATA) {
-    for (int i = 0; i < ptr->length; i++) {
-      delete[] ptr->keys[i];
-    }
-  }
-  if (ptr->keys != nullptr) {
-    delete[] ptr->keys;
+  if (FrozenKeyIdxBiMap_is_ok(ptr)) {
+    if (ptr->length > 0) {
+      if (ptr->mode == BiMapMode::COPIES_KEYDATA) {
+        for (bimap::rowidx_type i = 0; i < ptr->capacity; i++) {
+          bimap_StrU16_detail::Row* row = ptr->table_rows + i;
+          // casting from (const char*) to (char*) should be legal (as long as
+          // there were no bugs modifying the value of ptr->mode)
+          if (row->keylen > 0) {
+            delete[] row->key;
+          }
+        }
+      }
+      delete[] ptr->table_rows;
+      delete[] ptr->ordered_row_indices;
+    }  // ptr->length > 0
+    (*ptr) = mk_invalid_FrozenKeyIdxBiMap();
   }
 }
 
@@ -258,9 +265,7 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
 /// ugly/clunky, but it's the only practical way to achieve comparable behavior
 /// to other internal data types. The best alternatives involve things like
 /// std::optional or converting this type to a simple C++ class.
-inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
-  return new_FrozenKeyIdxBiMap(ptr->keys, ptr->length, ptr->mode);
-};
+FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr);
 
 /// lookup the value associated with the key
 ///
@@ -274,13 +279,9 @@ inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
 ///     @ref grackle::impl::bimap::invalid_val
 inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
     const FrozenKeyIdxBiMap* map, const char* key) {
-  GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
-  for (int i = 0; i < map->length; i++) {
-    if (std::strcmp(map->keys[i], key) == 0) {
-      return static_cast<std::uint16_t>(i);
-    }
-  }
-  return bimap::invalid_val;
+  return bimap_StrU16_detail::search(map->table_rows, key, map->capacity,
+                                     map->max_probe)
+      .val;
 }
 
 /// returns whether the map contains the key
@@ -323,11 +324,131 @@ inline const char* FrozenKeyIdxBiMap_key_from_idx(const FrozenKeyIdxBiMap* map,
   if (i >= map->length) {
     return nullptr;
   }
-  return map->keys[i];  // this can't be a nullptr
+  const char* out = map->table_rows[map->ordered_row_indices[i]].key;
+  GR_INTERNAL_REQUIRE(out != nullptr, "logical error: string can't be nullptr");
+  return out;
 }
 
 /** @}*/  // end of group
 
+namespace bimap_detail {
+
+/// a helper function used to actually allocate memory for FrozenKeyIdxBiMap
+inline FrozenKeyIdxBiMap alloc(std::uint16_t length, std::uint16_t capacity,
+                               BiMapMode mode) {
+  // it would be nice to handle allocate all pointers as a single block of
+  // memory, but that gets tricky. Essentially, we would allocate uninitialized
+  // memory and manually use placement-new (and the corresponding `delete`)
+  using bimap::rowidx_type;
+  using bimap_StrU16_detail::Row;
+  FrozenKeyIdxBiMap out = {
+      /*length=*/length,
+      /*capacity=*/capacity,
+      /*max_probe=*/capacity,
+      /*mode=*/mode,
+      /*table_rows=*/(capacity > 0) ? new Row[capacity] : nullptr,
+      /*ordered_row_indices=*/(length > 0) ? new rowidx_type[length] : nullptr};
+  for (std::uint16_t i = 0; i < capacity; i++) {
+    out.table_rows[i].keylen = 0;
+  }
+  return out;
+}
+
+}  // namespace bimap_detail
+
+inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
+                                               int key_count, BiMapMode mode) {
+  long long max_len = static_cast<long long>(bimap_cap_detail::max_key_count());
+  if (keys == nullptr && key_count == 0) {
+    return bimap_detail::alloc(0, 0, mode);
+  } else if (keys == nullptr) {
+    GrPrintErrMsg("keys must not be a nullptr");
+    return mk_invalid_FrozenKeyIdxBiMap();
+  } else if (key_count < 1 || static_cast<long long>(key_count) > max_len) {
+    GrPrintErrMsg("key_count must be positive & can't exceed %lld", max_len);
+    return mk_invalid_FrozenKeyIdxBiMap();
+  }
+
+  // based on the preceding check, this shouldn't be able to fail
+  bimap::rowidx_type capacity = bimap_cap_detail::calc_map_capacity(key_count);
+  GR_INTERNAL_REQUIRE(capacity > 0, "something went wrong");
+
+  // let's validate the keys
+  for (int i = 0; i < key_count; i++) {
+    GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
+    std::size_t n_chrs_without_nul = std::strlen(keys[i]);
+    if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap::keylen_max) {
+      GrPrintErrMsg(
+          "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
+          "exceeding %d",
+          keys[i], i, bimap::keylen_max);
+      return mk_invalid_FrozenKeyIdxBiMap();
+    }
+    // check uniqueness
+    for (int j = 0; j < i; j++) {
+      if (strcmp(keys[i], keys[j]) == 0) {
+        GrPrintErrMsg("\"%s\" key repeats", keys[i]);
+        return mk_invalid_FrozenKeyIdxBiMap();
+      }
+    }
+  }
+
+  // now, that we know we will succeed, lets construct the bimap
+  FrozenKeyIdxBiMap out = bimap_detail::alloc(key_count, capacity, mode);
+
+  // now it's time to fill in the array
+  int max_probe_count = 1;
+  for (int i = 0; i < key_count; i++) {
+    // search for the first empty row
+    bimap_StrU16_detail::SearchRslt search_rslt = bimap_StrU16_detail::search(
+        out.table_rows, keys[i], capacity, capacity);
+    // this should be infallible (especially after we already did some checks)
+    GR_INTERNAL_REQUIRE(search_rslt.probe_count != 0, "sanity check failed");
+
+    // now we overwrite the row
+    bimap_StrU16_detail::overwrite_row(out.table_rows + search_rslt.rowidx,
+                                       keys[i], std::strlen(keys[i]), i,
+                                       mode == BiMapMode::COPIES_KEYDATA);
+    out.ordered_row_indices[i] = search_rslt.rowidx;
+
+    max_probe_count = std::max(max_probe_count, search_rslt.probe_count);
+  }
+  out.max_probe = max_probe_count;
+
+  return out;
+}
+
+inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
+  FrozenKeyIdxBiMap out =
+      bimap_detail::alloc(ptr->length, ptr->capacity, ptr->mode);
+  out.max_probe = ptr->max_probe;
+
+  if (ptr->length == 0 || !FrozenKeyIdxBiMap_is_ok(ptr)) {
+    return out;
+  }
+
+  // give the compiler/linter a hint that out.table_rows is not a nullptr
+  // (this is guaranteed by the preceding early exit)
+  GR_INTERNAL_REQUIRE(
+      (out.table_rows != nullptr) && (out.ordered_row_indices != nullptr),
+      "something is very wrong!");
+
+  bool copy_key_data = out.mode == BiMapMode::COPIES_KEYDATA;
+  for (bimap::rowidx_type i = 0; i < ptr->capacity; i++) {
+    const bimap_StrU16_detail::Row& ref_row = ptr->table_rows[i];
+    if (ref_row.keylen > 0) {
+      bimap_StrU16_detail::overwrite_row(out.table_rows + i, ref_row.key,
+                                         ref_row.keylen, ref_row.value,
+                                         copy_key_data);
+    }
+  }
+
+  for (bimap::rowidx_type i = 0; i < ptr->length; i++) {
+    out.ordered_row_indices[i] = ptr->ordered_row_indices[i];
+  }
+  return out;
+};
+
 }  // namespace grackle::impl
 
 #endif  // SUPPORT_FROZENKEYIDXBIMAP_HPP
diff --git a/src/clib/support/FrozenKeyIdxBiMap_detail.hpp b/src/clib/support/FrozenKeyIdxBiMap_detail.hpp
new file mode 100644
index 000000000..7d005d668
--- /dev/null
+++ b/src/clib/support/FrozenKeyIdxBiMap_detail.hpp
@@ -0,0 +1,198 @@
+//===----------------------------------------------------------------------===//
+//
+// See the LICENSE file for license and copyright information
+// SPDX-License-Identifier: NCSA AND BSD-3-Clause
+//
+//===----------------------------------------------------------------------===//
+///
+/// @file
+/// Defines the hash table machinery used to implement the internals of the
+/// FrozenKeyIdxBiMap type
+///
+//===----------------------------------------------------------------------===//
+#ifndef SUPPORT_FROZENKEYIDXBIMAP_DETAIL_HPP
+#define SUPPORT_FROZENKEYIDXBIMAP_DETAIL_HPP
+
+#include <algorithm>  // std:::min
+#include <cstdint>
+#include <cstring>  // std::memcmp
+#include <limits>
+#include "status_reporting.h"
+
+#include "fnv1a_hash.hpp"
+
+namespace grackle::impl {
+
+/// This namespace holds a few constants that are used by consumers of
+/// FrozenKeyIdxBiMap
+namespace bimap {
+/// specifies an invalid value of the map
+///
+/// In other words, a map must have fewer entries than the maximum possible u16
+/// value
+inline constexpr std::uint16_t invalid_val =
+    std::numeric_limits<std::uint16_t>::max();
+
+/// specifies maximum allowed length of a key (excluding the null terminator).
+///
+/// @note
+/// While the value may seem low, it's probably large enough. Restricting
+/// strings to 30 elements (including the terminator) allows for a hypothetical
+/// optimization (see the @ref FrozenKeyIdxBiMap docstring for info).
+inline constexpr std::uint16_t keylen_max = 29;
+
+/// the type used for indexing the rows of the BiMap's internal hash table
+///
+/// @note
+/// This does not necessarily need to be a uint16_t (even if the internal
+/// values of the hash table are uint16_t).
+typedef std::uint16_t rowidx_type;
+
+}  // namespace bimap
+
+// -----------------------------------------------------------------
+
+/// Encloses code and logic pertaining to the capacity of a FrozenKeyIdxBiMap
+namespace bimap_cap_detail {
+
+/// the load factor specifies the fraction of the capacity of the Hash
+/// table that is filled. This should be an integer.
+///
+/// Generally, the larger this is, the fewer collisions there are, but the more
+/// memory is required. Lookups probably get slower if it's too big
+inline constexpr int INVERSE_LOAD_FACTOR = 2;
+static_assert(INVERSE_LOAD_FACTOR > 1);
+
+/// list of allowed capacities. These are all prime numbers that
+/// have nearly constant linear spacing (it may make more sense to have
+/// logarithmic spacing)
+inline constexpr std::uint32_t CAPACITIES_LIST[] = {
+    7,   19,  31,  41,  53,  61,  71,  83,  89,  101, 113, 127,
+    139, 149, 163, 173, 181, 191, 199, 211, 223, 233, 241, 251};
+
+inline constexpr int N_CAPACITIES =
+    (sizeof(CAPACITIES_LIST) / sizeof(std::uint32_t));
+
+/// compute the maximum number of keys
+inline bimap::rowidx_type max_key_count() {
+  return std::min<bimap::rowidx_type>(
+      std::numeric_limits<bimap::rowidx_type>::max(),
+      CAPACITIES_LIST[N_CAPACITIES - 1] / INVERSE_LOAD_FACTOR);
+}
+
+/// compute the capacity of the map (a value of 0 indicates that a large enough
+/// capacity can't be found)
+///
+/// @param key_count the desired number of keys (should be positive)
+inline std::uint16_t calc_map_capacity(int key_count) {
+  std::uint64_t c = INVERSE_LOAD_FACTOR * static_cast<std::uint64_t>(key_count);
+  for (int i = 0; i < N_CAPACITIES; i++) {  // binary search may be faster
+    if (c < CAPACITIES_LIST[i]) {
+      return static_cast<std::uint16_t>(CAPACITIES_LIST[i]);
+    }
+  }
+  return 0;
+}
+
+}  // namespace bimap_cap_detail
+
+// -----------------------------------------------------------------
+
+/// Holds machinery for hash tablea that implement FrozenKeyIdxBiMap
+///
+/// This machinery is specialized for (string, u16) key-value pairs
+namespace bimap_StrU16_detail {
+
+/// entry in a hash table
+///
+/// This acts as a (key,value) pair with a little extra metadata. A hash table
+/// is fundamentally an array of these instances
+///
+/// @note members are ordered to minimize the struct size (i.e. smallest members
+/// listed first) to pack as many entries into a cacheline as possible
+struct Row {
+  /// specifies the value associated with the current key
+  std::uint16_t value;
+
+  /// specifies the length of the key (not including the '\0')
+  ///
+  /// @note Tracked for short-circuiting comparisons (while probing collisions)
+  std::uint16_t keylen;
+  /// identifies the address of this entry's key
+  const char* key;
+};
+
+static void overwrite_row(Row* row, const char* key, std::uint16_t keylen,
+                          std::uint16_t value, bool copy_key_data) {
+  GR_INTERNAL_REQUIRE(row->keylen == 0, "Sanity check failed!");
+  row->value = value;
+  row->keylen = keylen;
+  const char* key_ptr = key;
+  if (copy_key_data) {
+    std::size_t total_len = keylen + 1;  // <- add 1 to account for '\0'
+    char* ptr = new char[total_len];
+    std::memcpy(ptr, key, total_len);
+    key_ptr = ptr;
+  }
+  *row = Row{value, keylen, key_ptr};
+}
+
+/// represents the result of an internal search for a key
+///
+/// @note
+/// As a rule of thumb, it's generally better (for compiler optimization) to
+/// return a struct of integers than rely on modifying pointer arguments
+struct SearchRslt {
+  /// specifies the value found by the search (or @ref invalid_val)
+  std::uint16_t val;
+  /// specified the number of probes before the search returned
+  int probe_count;
+  /// specify the index of the "row" corresponding to the search result
+  int rowidx;
+};
+
+/// Search for the row matching key. The search ends when a match is found, an
+/// an empty row is found, or the function has probed `max_probe` entries
+///
+/// @param rows an array of rows to search to be compared
+/// @param key the key to be compared
+/// @param capacity the length of the rows array
+/// @param max_probe the maximum number of rows to check before giving up
+///
+/// @important
+/// The behavior is undefined if @p key is a @c nullptr, @p keylen is 0, or
+/// @p keylen exceeds @p bimap::keylen
+///
+/// @note
+/// This is declared as `static inline` to facilitate inlining within
+/// FrozenKeyIdxBiMap's interface API.
+inline SearchRslt search(const Row* rows, const char* key, int capacity,
+                         int max_probe) {
+  GR_INTERNAL_REQUIRE(key != nullptr, "Major programming oversight");
+  max_probe = (max_probe <= 0 || max_probe > capacity) ? capacity : max_probe;
+
+  HashRsltPack h = fnv1a_hash<bimap::keylen_max>(key);
+  int i = -1;  // <- set to a dummy value
+  int launched_probes = 0;
+  if (h.keylen > 0 && h.success && max_probe > 0) {
+    int guess_i = static_cast<int>(h.hash % capacity);  // <- initial guess
+
+    do {  // circularly loop over rows to search for key (start at guess_i)
+      i = (guess_i + launched_probes) % capacity;
+      launched_probes++;  // <- about to perform a new probe
+      const Row& r = rows[i];
+
+      if (r.keylen == h.keylen && std::memcmp(r.key, key, h.keylen) == 0) {
+        return SearchRslt{r.value, launched_probes, i};  // match found!
+      }
+
+      // check if rows[i] is empty or if we have hit the limit on searches
+    } while (rows[i].keylen != 0 && launched_probes < max_probe);
+  }
+
+  return SearchRslt{bimap::invalid_val, launched_probes, i};
+}
+
+}  // namespace bimap_StrU16_detail
+}  // namespace grackle::impl
+#endif  // SUPPORT_FROZENKEYIDXBIMAP_DETAIL_HPP

From 8f66736e47f0eb306fd7f270fc3d507067d02ad1 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Tue, 6 Jan 2026 22:18:01 -0500
Subject: [PATCH 20/25] rename grackle::impl::{bimap->bimap_detail} &
 capitalize constant names

Both changes essentially affected 2 constants, which prior to this
commit were known as `keylen_max` & `invalid_val`.

The 2 contained constants are really just implementation details that
users of the inferface for `FrozenKeyIdxBiMap` shouldn't need to know
anything about
---
 src/clib/support/FrozenKeyIdxBiMap.hpp     | 28 +++++++++++-----------
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 20 ++++++++--------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index b00c0d0e0..cb07d752c 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -27,19 +27,18 @@
 #include "grackle.h"
 #include "status_reporting.h"
 
+namespace grackle::impl {
+
 // the motivation for these constants are provided in PR #484 (they are related
 // to some optimizations in the FrozenKeyIdxBiMap implementation)
-namespace grackle::impl::bimap {
+namespace bimap_detail {
 /// specifies an invalid value of the map (we state that you can't store the
 /// maximum u16 value)
-inline constexpr std::uint16_t invalid_val =
-    std::numeric_limits<std::uint16_t>::max();
+inline constexpr uint16_t INVALID_VAL = std::numeric_limits<uint16_t>::max();
 
 /// specifies maximum allowed length of a key (excluding the null character).
-inline constexpr std::uint16_t keylen_max = 29;
-}  // namespace grackle::impl::bimap
-
-namespace grackle::impl {
+inline constexpr uint16_t KEYLEN_MAX = 29;
+}  // namespace bimap_detail
 
 // the following doxygen comment block logically groups every all parts of
 // the (internal) API for Grackle's (internal) FrozenKeyIdxBiMap. It's useful
@@ -139,7 +138,7 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
   }
 
   // check the specified keys
-  long long max_keys = static_cast<long long>(bimap::invalid_val) - 1LL;
+  long long max_keys = static_cast<long long>(bimap_detail::INVALID_VAL) - 1LL;
   if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
     GrPrintErrMsg("key_count must be positive and cannot exceed %lld",
                   max_keys);
@@ -151,11 +150,12 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
   for (int i = 0; i < key_count; i++) {
     GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
     std::size_t n_chrs_without_nul = std::strlen(keys[i]);
-    if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap::keylen_max) {
+    if (n_chrs_without_nul == 0 ||
+        n_chrs_without_nul > bimap_detail::KEYLEN_MAX) {
       GrPrintErrMsg(
           "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
           "exceeding %d",
-          keys[i], i, bimap::keylen_max);
+          keys[i], i, bimap_detail::KEYLEN_MAX);
       return erroneous_obj;
     }
     // check uniqueness
@@ -262,13 +262,13 @@ inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
 /// lookup the value associated with the key
 ///
 /// This is the analog to calling `map[key]` in python. In practice, the
-/// semantics are more similar to calling `map.get(key,invalid_val)`
+/// semantics are more similar to calling `map.get(key,INVALID_VAL)`
 ///
 /// @param[in] map A pointer to a valid bimap
 /// @param[in] key A null-terminated string
 ///
 /// @return the key's associated value or, if the key can't be found,
-///     @ref grackle::impl::bimap::invalid_val
+///     @ref grackle::impl::bimap_detail::INVALID_VAL
 inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
     const FrozenKeyIdxBiMap* map, const char* key) {
   GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
@@ -277,7 +277,7 @@ inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
       return static_cast<std::uint16_t>(i);
     }
   }
-  return bimap::invalid_val;
+  return bimap_detail::INVALID_VAL;
 }
 
 /// returns whether the map contains the key
@@ -286,7 +286,7 @@ inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
 /// @param[in] key A null-terminated string
 inline bool FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
                                        const char* key) {
-  return FrozenKeyIdxBiMap_idx_from_key(map, key) != bimap::invalid_val;
+  return FrozenKeyIdxBiMap_idx_from_key(map, key) != bimap_detail::INVALID_VAL;
 }
 
 /// return the number of keys in the map
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 07345e38c..87ee9d8a6 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -60,7 +60,7 @@ TEST(FrozenKeyIdxBiMap, FullExample) {
   EXPECT_EQ(33, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "O2II"));
 
   // Behavior is well-defined when a key isn't known
-  int invalid = grimpl::bimap::invalid_val;
+  int invalid = grimpl::bimap_detail::INVALID_VAL;
   EXPECT_EQ(invalid, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "NotAKey"));
 
   // PART 3: let's show the reverse of the previous lookups
@@ -87,7 +87,7 @@ TEST(FrozenKeyIdxBiMap, EmptyBasicOps) {
   EXPECT_EQ(0, grackle::impl::FrozenKeyIdxBiMap_size(&m))
       << "an empty mapping should have a size of 0";
 
-  EXPECT_EQ(grackle::impl::bimap::invalid_val,
+  EXPECT_EQ(grackle::impl::bimap_detail::INVALID_VAL,
             grackle::impl::FrozenKeyIdxBiMap_idx_from_key(&m, "NotAKey"))
       << "key lookup should always fail for an empty mapping";
 
@@ -139,7 +139,7 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, Simple) {
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
   const char* first_key = "density";
-  std::string long_key(grackle::impl::bimap::keylen_max, 'A');
+  std::string long_key(grackle::impl::bimap_detail::KEYLEN_MAX, 'A');
   const char* keys[2] = {first_key, long_key.data()};
 
   grackle::impl::FrozenKeyIdxBiMap tmp =
@@ -151,7 +151,7 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, LongKey) {
 
 TEST_P(FrozenKeyIdxBiMapConstructorSuite, TooLongKey) {
   const char* first_key = "density";
-  std::string long_key(grackle::impl::bimap::keylen_max + 1, 'A');
+  std::string long_key(grackle::impl::bimap_detail::KEYLEN_MAX + 1, 'A');
   const char* keys[2] = {first_key, long_key.data()};
 
   grackle::impl::FrozenKeyIdxBiMap tmp =
@@ -259,22 +259,22 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
-            grackle::impl::bimap::invalid_val);
+            grackle::impl::bimap_detail::INVALID_VAL);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
-            grackle::impl::bimap::invalid_val);
+            grackle::impl::bimap_detail::INVALID_VAL);
 
-  std::string key(grackle::impl::bimap::keylen_max + 1, 'A');
+  std::string key(grackle::impl::bimap_detail::KEYLEN_MAX + 1, 'A');
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, key.data()),
-            grackle::impl::bimap::invalid_val);
+            grackle::impl::bimap_detail::INVALID_VAL);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 3), nullptr);
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
-                bimap_p, grackle::impl::bimap::invalid_val),
+                bimap_p, grackle::impl::bimap_detail::INVALID_VAL),
             nullptr);
 }
 
@@ -311,7 +311,7 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
       grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "internal_energy"),
       0);
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "notAKey"),
-            grackle::impl::bimap::invalid_val);
+            grackle::impl::bimap_detail::INVALID_VAL);
 
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 3), nullptr);
   EXPECT_EQ(

From c30a4d376963c495f5dd3c35f25be54e1a14f11b Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 7 Jan 2026 09:31:45 -0500
Subject: [PATCH 21/25] intermediate commit

---
 src/clib/support/FrozenKeyIdxBiMap.hpp     | 43 +++++++----
 tests/unit/CMakeLists.txt                  |  2 +-
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 84 ++++++++++++++--------
 3 files changed, 84 insertions(+), 45 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index cb07d752c..60a3f52ab 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -259,25 +259,38 @@ inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
   return new_FrozenKeyIdxBiMap(ptr->keys, ptr->length, ptr->mode);
 };
 
+namespace bimap {
+
+/// holds the result of a call to @ref FrozenKeyIdxBiMap_get
+///
+/// @note This C-style approximation of std::optional<uint16_t>
+struct AccessRslt {
+  /// Indicates whether the value member is valid
+  bool has_value;
+  /// the loaded value (if has_value is false then this holds garbage)
+  uint16_t value;
+};
+
+}
+
 /// lookup the value associated with the key
 ///
-/// This is the analog to calling `map[key]` in python. In practice, the
-/// semantics are more similar to calling `map.get(key,INVALID_VAL)`
+/// This is the analog to calling `map[key]` in python.
 ///
 /// @param[in] map A pointer to a valid bimap
 /// @param[in] key A null-terminated string
 ///
-/// @return the key's associated value or, if the key can't be found,
-///     @ref grackle::impl::bimap_detail::INVALID_VAL
-inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
+/// @return An instance of @ref bimap::AccessRslt that encodes the value (if
+///     the key is present)
+inline bimap::AccessRslt FrozenKeyIdxBiMap_get(
     const FrozenKeyIdxBiMap* map, const char* key) {
   GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
   for (int i = 0; i < map->length; i++) {
     if (std::strcmp(map->keys[i], key) == 0) {
-      return static_cast<std::uint16_t>(i);
+      return bimap::AccessRslt{true, static_cast<uint16_t>(i)};
     }
   }
-  return bimap_detail::INVALID_VAL;
+  return bimap::AccessRslt{false, 0};
 }
 
 /// returns whether the map contains the key
@@ -286,7 +299,7 @@ inline std::uint16_t FrozenKeyIdxBiMap_idx_from_key(
 /// @param[in] key A null-terminated string
 inline bool FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
                                        const char* key) {
-  return FrozenKeyIdxBiMap_idx_from_key(map, key) != bimap_detail::INVALID_VAL;
+  return FrozenKeyIdxBiMap_get(map, key).has_value;
 }
 
 /// return the number of keys in the map
@@ -296,10 +309,10 @@ inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map) {
   return map->length;
 }
 
-/// Return the ith key (this is effectively a reverse lookup)
+/// Return the key associated with the specified value
 ///
 /// For some context, if this function returns a string `s` for some index `i`,
-/// then a call to @ref FrozenKeyIdxBiMap_idx_from_key that passes `s` will
+/// then a call to @ref FrozenKeyIdxBiMap_get that passes `s` will
 /// return `i`
 ///
 /// This is intended for use in situations where you briefly need the string
@@ -313,14 +326,14 @@ inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map) {
 ///   is ill-formed
 ///
 /// @param[in] map A pointer to a valid bimap
-/// @param[in] i The returned index
+/// @param[in] idx The index to check
 /// @return The pointer to the appropriate key
-inline const char* FrozenKeyIdxBiMap_key_from_idx(const FrozenKeyIdxBiMap* map,
-                                                  std::uint16_t i) {
-  if (i >= map->length) {
+inline const char* FrozenKeyIdxBiMap_inverse_get(const FrozenKeyIdxBiMap* map,
+                                                 uint16_t idx) {
+  if (idx >= map->length) {
     return nullptr;
   }
-  return map->keys[i];  // this can't be a nullptr
+  return map->keys[idx];  // this can't be a nullptr
 }
 
 /** @}*/  // end of group
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
index d09e1c7c4..8c2e2bcf6 100644
--- a/tests/unit/CMakeLists.txt
+++ b/tests/unit/CMakeLists.txt
@@ -49,7 +49,7 @@ target_compile_definitions(grtest_utils
 # start declaring targets for tests
 # ---------------------------------
 add_executable(testFrozenKeyIdxBiMap test_unit_FrozenKeyIdxBiMap.cpp)
-target_link_libraries(testFrozenKeyIdxBiMap testdeps)
+target_link_libraries(testFrozenKeyIdxBiMap testdeps GTest::gmock)
 gtest_discover_tests(testFrozenKeyIdxBiMap)
 
 add_executable(runInterpolationTests test_unit_interpolators_g.cpp)
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 87ee9d8a6..00ff7615e 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -10,13 +10,32 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include <iostream>
 #include <string>
 #include <vector>
 
 #include <gtest/gtest.h>
+#include <gmock/gmock.h>
 #include "support/FrozenKeyIdxBiMap.hpp"
 #include "grackle.h"
 
+using ::testing::Field;
+using ::testing::FieldsAre;
+
+namespace grimpl = grackle::impl;
+namespace bimap = grackle::impl::bimap;
+
+// teach GoogleTest how to print grackle::impl::bimap::AccessRslt
+namespace grackle::impl::bimap {
+void PrintTo(const AccessRslt& access_rslt, std::ostream* os) {
+  if (!access_rslt.has_value) {
+    *os << "AccessRslt{.has_value=false}";
+  } else {
+    *os << "AccessRslt{.has_value=true, .value=" << access_rslt.value << "}";
+  }
+}
+} // namespace grackle::impl::bimap
+
 // this top test was introduced to provide a more concrete example
 // of how we might use FrozenKeyIdxBiMap
 
@@ -53,22 +72,26 @@ TEST(FrozenKeyIdxBiMap, FullExample) {
   }
 
   // PART 2: let's show some examples of lookups from names
+  // -> for context, grimpl::FrozenKeyIdxBiMap_get returns a bimap::AccessRslt,
+  //    which holds {bool has_value; uint16_t value;}
 
   // Equivalent Python/idiomatic C++:  `2 == m["HII"]`
-  EXPECT_EQ(2, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "HII"));
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "HII"),
+              ::FieldsAre(/*has_value=*/true, /*value=*/2));
   // Equivalent Python/idiomatic C++:  `33 == m["O2II"]`
-  EXPECT_EQ(33, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "O2II"));
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "O2II"),
+              ::FieldsAre(/*has_value=*/true, /*value=*/33));
 
   // Behavior is well-defined when a key isn't known
-  int invalid = grimpl::bimap_detail::INVALID_VAL;
-  EXPECT_EQ(invalid, grimpl::FrozenKeyIdxBiMap_idx_from_key(&m, "NotAKey"));
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "NotAKey"),
+              ::Field("has_value", &bimap::AccessRslt::has_value, false));
 
   // PART 3: let's show the reverse of the previous lookups
-  EXPECT_STREQ("HII", grimpl::FrozenKeyIdxBiMap_key_from_idx(&m, 2));
-  EXPECT_STREQ("O2II", grimpl::FrozenKeyIdxBiMap_key_from_idx(&m, 33));
+  EXPECT_STREQ("HII", grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 2));
+  EXPECT_STREQ("O2II", grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 33));
 
   // Behavior is again well-defined when passing an invalid index
-  EXPECT_EQ(nullptr, grimpl::FrozenKeyIdxBiMap_key_from_idx(&m, 131));
+  EXPECT_EQ(nullptr, grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 131));
 
   // PART 4: We can also query the length
   EXPECT_EQ(34, grimpl::FrozenKeyIdxBiMap_size(&m));
@@ -87,11 +110,11 @@ TEST(FrozenKeyIdxBiMap, EmptyBasicOps) {
   EXPECT_EQ(0, grackle::impl::FrozenKeyIdxBiMap_size(&m))
       << "an empty mapping should have a size of 0";
 
-  EXPECT_EQ(grackle::impl::bimap_detail::INVALID_VAL,
-            grackle::impl::FrozenKeyIdxBiMap_idx_from_key(&m, "NotAKey"))
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "NotAKey"),
+              ::Field("has_value", &bimap::AccessRslt::has_value, false))
       << "key lookup should always fail for an empty mapping";
 
-  EXPECT_EQ(nullptr, grackle::impl::FrozenKeyIdxBiMap_key_from_idx(&m, 0))
+  EXPECT_EQ(nullptr, grackle::impl::FrozenKeyIdxBiMap_inverse_get(&m, 0))
       << "index lookup should always fail for an empty mapping";
 
   grackle::impl::drop_FrozenKeyIdxBiMap(&m);
@@ -180,7 +203,7 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, NotNull0KeyCount) {
 }
 
 INSTANTIATE_TEST_SUITE_P(
-    , /* <- leaving Instantiation name empty */
+    , // <- leaving Instantiation name empty
     FrozenKeyIdxBiMapConstructorSuite,
     testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
                     grackle::impl::BiMapMode::COPIES_KEYDATA),
@@ -238,7 +261,7 @@ class FrozenKeyIdxBiMapGeneralSuite
   bool ReusesOriginalKeyPtrs(const grackle::impl::FrozenKeyIdxBiMap* p) const {
     for (int i = 0; i < 3; i++) {
       const char* orig_key_ptr = ordered_keys[i].c_str();
-      if (grackle::impl::FrozenKeyIdxBiMap_key_from_idx(p, i) != orig_key_ptr) {
+      if (grackle::impl::FrozenKeyIdxBiMap_inverse_get(p, i) != orig_key_ptr) {
         return false;
       }
     }
@@ -246,47 +269,48 @@ class FrozenKeyIdxBiMapGeneralSuite
   }
 };
 
+/*
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "density"),
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "density"),
             1);
   EXPECT_EQ(
-      grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "internal_energy"),
+      grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "internal_energy"),
       0);
   EXPECT_EQ(
-      grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "metal_density"),
+      grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "metal_density"),
       2);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, "notAKey"),
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "notAKey"),
             grackle::impl::bimap_detail::INVALID_VAL);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, ""),
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, ""),
             grackle::impl::bimap_detail::INVALID_VAL);
 
   std::string key(grackle::impl::bimap_detail::KEYLEN_MAX + 1, 'A');
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(bimap_p, key.data()),
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, key.data()),
             grackle::impl::bimap_detail::INVALID_VAL);
 }
-
+*/
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 3), nullptr);
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 3), nullptr);
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(
                 bimap_p, grackle::impl::bimap_detail::INVALID_VAL),
             nullptr);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 2)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 2)),
       std::string("metal_density"));
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 1)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 1)),
       std::string("density"));
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(bimap_p, 0)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 0)),
       std::string("internal_energy"));
 
   // check whether the bimap is using pointers to the keys used during init
@@ -297,6 +321,7 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
   }
 }
 
+/*
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   grackle::impl::FrozenKeyIdxBiMap clone =
       grackle::impl::FrozenKeyIdxBiMap_clone(bimap_p);
@@ -308,14 +333,14 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   bimap_p = nullptr;
 
   EXPECT_EQ(
-      grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "internal_energy"),
+      grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "internal_energy"),
       0);
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_idx_from_key(clone_p, "notAKey"),
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "notAKey"),
             grackle::impl::bimap_detail::INVALID_VAL);
 
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 3), nullptr);
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 3), nullptr);
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_key_from_idx(clone_p, 1)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 1)),
       std::string("density"));
 
   // check whether the clone is using pointers to the keys used during init
@@ -328,9 +353,10 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   // finally, cleanup the clone
   grackle::impl::drop_FrozenKeyIdxBiMap(clone_p);
 }
+*/
 
 INSTANTIATE_TEST_SUITE_P(
-    , /* <- leaving Instantiation name empty */
+    , // <- leaving Instantiation name empty
     FrozenKeyIdxBiMapGeneralSuite,
     testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
                     grackle::impl::BiMapMode::COPIES_KEYDATA),

From 13b956f418c8e0607dc95f83605073a1ff827241 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 7 Jan 2026 13:03:42 -0500
Subject: [PATCH 22/25] shift to using AccessRslt

---
 src/clib/support/FrozenKeyIdxBiMap.hpp     |   6 +-
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 117 +++++++++++----------
 2 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index 60a3f52ab..37dbbb8d4 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -271,7 +271,7 @@ struct AccessRslt {
   uint16_t value;
 };
 
-}
+}  // namespace bimap
 
 /// lookup the value associated with the key
 ///
@@ -282,8 +282,8 @@ struct AccessRslt {
 ///
 /// @return An instance of @ref bimap::AccessRslt that encodes the value (if
 ///     the key is present)
-inline bimap::AccessRslt FrozenKeyIdxBiMap_get(
-    const FrozenKeyIdxBiMap* map, const char* key) {
+inline bimap::AccessRslt FrozenKeyIdxBiMap_get(const FrozenKeyIdxBiMap* map,
+                                               const char* key) {
   GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
   for (int i = 0; i < map->length; i++) {
     if (std::strcmp(map->keys[i], key) == 0) {
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 00ff7615e..240a7fdf2 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -10,7 +10,7 @@
 ///
 //===----------------------------------------------------------------------===//
 
-#include <iostream>
+#include <iostream>  // needed to teach googletest how to print
 #include <string>
 #include <vector>
 
@@ -19,22 +19,42 @@
 #include "support/FrozenKeyIdxBiMap.hpp"
 #include "grackle.h"
 
-using ::testing::Field;
-using ::testing::FieldsAre;
+// teach GoogleTest how to print grackle::impl::bimap::AccessRslt for more
+// informative errors (otherwise it just shows the memory's raw byte values)
+namespace grackle::impl::bimap {
+void PrintTo(const AccessRslt& ar, std::ostream* os) {
+  std::string tmp = (ar.has_value) ? std::to_string(ar.value) : "<garbage>";
+  *os << "{has_value=" << ar.has_value << ", value=" << tmp << '}';
+}
+}  // namespace grackle::impl::bimap
 
-namespace grimpl = grackle::impl;
-namespace bimap = grackle::impl::bimap;
+std::string prep_descr(std::string descr, bool negation) {
+  return ((negation) ? "isn't " : "is ") + descr;
+}
 
-// teach GoogleTest how to print grackle::impl::bimap::AccessRslt
-namespace grackle::impl::bimap {
-void PrintTo(const AccessRslt& access_rslt, std::ostream* os) {
-  if (!access_rslt.has_value) {
-    *os << "AccessRslt{.has_value=false}";
-  } else {
-    *os << "AccessRslt{.has_value=true, .value=" << access_rslt.value << "}";
+// the following defines a custom matcher for checking whether the has_value
+// member of an AccessRslt instance is false. Use via
+//   EXPECT_THAT(arg, EmptyAccessRslt)
+MATCHER(EmptyAccessRslt, prep_descr("an empty AccessRslt", negation)) {
+  if (!arg.has_value) {
+    return true;
+  }
+  *result_listener << "holds the " << arg.value << " value";
+  return false;
+}
+
+// the following defines a custom matcher for checking whether the has_value
+// member of an AccessRslt instance is true
+//   EXPECT_THAT(arg, AccessRsltHolding(value))
+MATCHER_P(AccessRsltHolds, v,
+          prep_descr("an AccessRslt holding " + std::to_string(v), negation)) {
+  if (!arg.has_value) {
+    *result_listener << "is empty";
+  } else if (arg.value != v) {
+    *result_listener << " holds the value, " << v;
   }
+  return arg.has_value && arg.value == v;
 }
-} // namespace grackle::impl::bimap
 
 // this top test was introduced to provide a more concrete example
 // of how we might use FrozenKeyIdxBiMap
@@ -72,19 +92,16 @@ TEST(FrozenKeyIdxBiMap, FullExample) {
   }
 
   // PART 2: let's show some examples of lookups from names
-  // -> for context, grimpl::FrozenKeyIdxBiMap_get returns a bimap::AccessRslt,
-  //    which holds {bool has_value; uint16_t value;}
 
-  // Equivalent Python/idiomatic C++:  `2 == m["HII"]`
+  // Equivalent Python:  `2 == m["HII"]`
   EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "HII"),
-              ::FieldsAre(/*has_value=*/true, /*value=*/2));
+              AccessRsltHolds(2));  // aka AccessRslt{has_value=true, value=2}
+
   // Equivalent Python/idiomatic C++:  `33 == m["O2II"]`
-  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "O2II"),
-              ::FieldsAre(/*has_value=*/true, /*value=*/33));
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "O2II"), AccessRsltHolds(33));
 
-  // Behavior is well-defined when a key isn't known
-  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "NotAKey"),
-              ::Field("has_value", &bimap::AccessRslt::has_value, false));
+  // for unknown key, returns AccessRslt{has_value=false, value=<garbage>}
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "Dummy"), EmptyAccessRslt());
 
   // PART 3: let's show the reverse of the previous lookups
   EXPECT_STREQ("HII", grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 2));
@@ -110,8 +127,8 @@ TEST(FrozenKeyIdxBiMap, EmptyBasicOps) {
   EXPECT_EQ(0, grackle::impl::FrozenKeyIdxBiMap_size(&m))
       << "an empty mapping should have a size of 0";
 
-  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "NotAKey"),
-              ::Field("has_value", &bimap::AccessRslt::has_value, false))
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(&m, "key"),
+              EmptyAccessRslt())
       << "key lookup should always fail for an empty mapping";
 
   EXPECT_EQ(nullptr, grackle::impl::FrozenKeyIdxBiMap_inverse_get(&m, 0))
@@ -203,7 +220,7 @@ TEST_P(FrozenKeyIdxBiMapConstructorSuite, NotNull0KeyCount) {
 }
 
 INSTANTIATE_TEST_SUITE_P(
-    , // <- leaving Instantiation name empty
+    ,  // <- leaving Instantiation name empty
     FrozenKeyIdxBiMapConstructorSuite,
     testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
                     grackle::impl::BiMapMode::COPIES_KEYDATA),
@@ -269,37 +286,31 @@ class FrozenKeyIdxBiMapGeneralSuite
   }
 };
 
-/*
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "density"),
-            1);
-  EXPECT_EQ(
-      grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "internal_energy"),
-      0);
-  EXPECT_EQ(
-      grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "metal_density"),
-      2);
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "density"),
+              AccessRsltHolds(1));
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "internal_energy"),
+              AccessRsltHolds(0));
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "metal_density"),
+              AccessRsltHolds(2));
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "notAKey"),
-            grackle::impl::bimap_detail::INVALID_VAL);
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "notAKey"),
+              EmptyAccessRslt());
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, ""),
-            grackle::impl::bimap_detail::INVALID_VAL);
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, ""),
+              EmptyAccessRslt());
 
   std::string key(grackle::impl::bimap_detail::KEYLEN_MAX + 1, 'A');
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, key.data()),
-            grackle::impl::bimap_detail::INVALID_VAL);
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, key.data()),
+              EmptyAccessRslt());
 }
-*/
+
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 3), nullptr);
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(
-                bimap_p, grackle::impl::bimap_detail::INVALID_VAL),
-            nullptr);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
@@ -321,7 +332,6 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
   }
 }
 
-/*
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   grackle::impl::FrozenKeyIdxBiMap clone =
       grackle::impl::FrozenKeyIdxBiMap_clone(bimap_p);
@@ -332,16 +342,14 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p);
   bimap_p = nullptr;
 
-  EXPECT_EQ(
-      grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "internal_energy"),
-      0);
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "notAKey"),
-            grackle::impl::bimap_detail::INVALID_VAL);
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "internal_energy"),
+              AccessRsltHolds(0));
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "notAKey"),
+              EmptyAccessRslt());
 
   EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 3), nullptr);
-  EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 1)),
-      std::string("density"));
+  EXPECT_STREQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 1),
+               "density");
 
   // check whether the clone is using pointers to the keys used during init
   if (GetParam() == grackle::impl::BiMapMode::REFS_KEYDATA) {
@@ -353,10 +361,9 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   // finally, cleanup the clone
   grackle::impl::drop_FrozenKeyIdxBiMap(clone_p);
 }
-*/
 
 INSTANTIATE_TEST_SUITE_P(
-    , // <- leaving Instantiation name empty
+    ,  // <- leaving Instantiation name empty
     FrozenKeyIdxBiMapGeneralSuite,
     testing::Values(grackle::impl::BiMapMode::REFS_KEYDATA,
                     grackle::impl::BiMapMode::COPIES_KEYDATA),

From 76b91eb3fe9a65fe1e15e791e81b5a85668f3b64 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 7 Jan 2026 13:06:27 -0500
Subject: [PATCH 23/25] some slight renaming

---
 src/clib/support/FrozenKeyIdxBiMap.hpp     | 14 +++----
 tests/unit/test_unit_FrozenKeyIdxBiMap.cpp | 46 +++++++++++-----------
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index 37dbbb8d4..40ebcfdc8 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -261,7 +261,7 @@ inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
 
 namespace bimap {
 
-/// holds the result of a call to @ref FrozenKeyIdxBiMap_get
+/// holds the result of a call to @ref FrozenKeyIdxBiMap_find
 ///
 /// @note This C-style approximation of std::optional<uint16_t>
 struct AccessRslt {
@@ -282,8 +282,8 @@ struct AccessRslt {
 ///
 /// @return An instance of @ref bimap::AccessRslt that encodes the value (if
 ///     the key is present)
-inline bimap::AccessRslt FrozenKeyIdxBiMap_get(const FrozenKeyIdxBiMap* map,
-                                               const char* key) {
+inline bimap::AccessRslt FrozenKeyIdxBiMap_find(const FrozenKeyIdxBiMap* map,
+                                                const char* key) {
   GR_INTERNAL_REQUIRE(key != nullptr, "A nullptr key is forbidden");
   for (int i = 0; i < map->length; i++) {
     if (std::strcmp(map->keys[i], key) == 0) {
@@ -299,7 +299,7 @@ inline bimap::AccessRslt FrozenKeyIdxBiMap_get(const FrozenKeyIdxBiMap* map,
 /// @param[in] key A null-terminated string
 inline bool FrozenKeyIdxBiMap_contains(const FrozenKeyIdxBiMap* map,
                                        const char* key) {
-  return FrozenKeyIdxBiMap_get(map, key).has_value;
+  return FrozenKeyIdxBiMap_find(map, key).has_value;
 }
 
 /// return the number of keys in the map
@@ -312,7 +312,7 @@ inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map) {
 /// Return the key associated with the specified value
 ///
 /// For some context, if this function returns a string `s` for some index `i`,
-/// then a call to @ref FrozenKeyIdxBiMap_get that passes `s` will
+/// then a call to @ref FrozenKeyIdxBiMap_find that passes `s` will
 /// return `i`
 ///
 /// This is intended for use in situations where you briefly need the string
@@ -328,8 +328,8 @@ inline int FrozenKeyIdxBiMap_size(const FrozenKeyIdxBiMap* map) {
 /// @param[in] map A pointer to a valid bimap
 /// @param[in] idx The index to check
 /// @return The pointer to the appropriate key
-inline const char* FrozenKeyIdxBiMap_inverse_get(const FrozenKeyIdxBiMap* map,
-                                                 uint16_t idx) {
+inline const char* FrozenKeyIdxBiMap_inverse_find(const FrozenKeyIdxBiMap* map,
+                                                  uint16_t idx) {
   if (idx >= map->length) {
     return nullptr;
   }
diff --git a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
index 240a7fdf2..148c3270e 100644
--- a/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
+++ b/tests/unit/test_unit_FrozenKeyIdxBiMap.cpp
@@ -94,21 +94,21 @@ TEST(FrozenKeyIdxBiMap, FullExample) {
   // PART 2: let's show some examples of lookups from names
 
   // Equivalent Python:  `2 == m["HII"]`
-  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "HII"),
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_find(&m, "HII"),
               AccessRsltHolds(2));  // aka AccessRslt{has_value=true, value=2}
 
   // Equivalent Python/idiomatic C++:  `33 == m["O2II"]`
-  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "O2II"), AccessRsltHolds(33));
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_find(&m, "O2II"), AccessRsltHolds(33));
 
   // for unknown key, returns AccessRslt{has_value=false, value=<garbage>}
-  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_get(&m, "Dummy"), EmptyAccessRslt());
+  EXPECT_THAT(grimpl::FrozenKeyIdxBiMap_find(&m, "Dummy"), EmptyAccessRslt());
 
   // PART 3: let's show the reverse of the previous lookups
-  EXPECT_STREQ("HII", grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 2));
-  EXPECT_STREQ("O2II", grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 33));
+  EXPECT_STREQ("HII", grimpl::FrozenKeyIdxBiMap_inverse_find(&m, 2));
+  EXPECT_STREQ("O2II", grimpl::FrozenKeyIdxBiMap_inverse_find(&m, 33));
 
   // Behavior is again well-defined when passing an invalid index
-  EXPECT_EQ(nullptr, grimpl::FrozenKeyIdxBiMap_inverse_get(&m, 131));
+  EXPECT_EQ(nullptr, grimpl::FrozenKeyIdxBiMap_inverse_find(&m, 131));
 
   // PART 4: We can also query the length
   EXPECT_EQ(34, grimpl::FrozenKeyIdxBiMap_size(&m));
@@ -127,11 +127,11 @@ TEST(FrozenKeyIdxBiMap, EmptyBasicOps) {
   EXPECT_EQ(0, grackle::impl::FrozenKeyIdxBiMap_size(&m))
       << "an empty mapping should have a size of 0";
 
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(&m, "key"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(&m, "key"),
               EmptyAccessRslt())
       << "key lookup should always fail for an empty mapping";
 
-  EXPECT_EQ(nullptr, grackle::impl::FrozenKeyIdxBiMap_inverse_get(&m, 0))
+  EXPECT_EQ(nullptr, grackle::impl::FrozenKeyIdxBiMap_inverse_find(&m, 0))
       << "index lookup should always fail for an empty mapping";
 
   grackle::impl::drop_FrozenKeyIdxBiMap(&m);
@@ -278,7 +278,7 @@ class FrozenKeyIdxBiMapGeneralSuite
   bool ReusesOriginalKeyPtrs(const grackle::impl::FrozenKeyIdxBiMap* p) const {
     for (int i = 0; i < 3; i++) {
       const char* orig_key_ptr = ordered_keys[i].c_str();
-      if (grackle::impl::FrozenKeyIdxBiMap_inverse_get(p, i) != orig_key_ptr) {
+      if (grackle::impl::FrozenKeyIdxBiMap_inverse_find(p, i) != orig_key_ptr) {
         return false;
       }
     }
@@ -287,41 +287,41 @@ class FrozenKeyIdxBiMapGeneralSuite
 };
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyContainedKey) {
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "density"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(bimap_p, "density"),
               AccessRsltHolds(1));
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "internal_energy"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(bimap_p, "internal_energy"),
               AccessRsltHolds(0));
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "metal_density"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(bimap_p, "metal_density"),
               AccessRsltHolds(2));
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentKey) {
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, "notAKey"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(bimap_p, "notAKey"),
               EmptyAccessRslt());
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, IdxFromKeyAbsentIrregularKeys) {
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, ""),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(bimap_p, ""),
               EmptyAccessRslt());
 
   std::string key(grackle::impl::bimap_detail::KEYLEN_MAX + 1, 'A');
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(bimap_p, key.data()),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(bimap_p, key.data()),
               EmptyAccessRslt());
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxInvalidIdx) {
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 3), nullptr);
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_find(bimap_p, 3), nullptr);
 }
 
 TEST_P(FrozenKeyIdxBiMapGeneralSuite, KeyFromIdxValidIdx) {
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 2)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_find(bimap_p, 2)),
       std::string("metal_density"));
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 1)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_find(bimap_p, 1)),
       std::string("density"));
   EXPECT_EQ(
-      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_get(bimap_p, 0)),
+      std::string(grackle::impl::FrozenKeyIdxBiMap_inverse_find(bimap_p, 0)),
       std::string("internal_energy"));
 
   // check whether the bimap is using pointers to the keys used during init
@@ -342,13 +342,13 @@ TEST_P(FrozenKeyIdxBiMapGeneralSuite, Clone) {
   grackle::impl::drop_FrozenKeyIdxBiMap(bimap_p);
   bimap_p = nullptr;
 
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "internal_energy"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(clone_p, "internal_energy"),
               AccessRsltHolds(0));
-  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_get(clone_p, "notAKey"),
+  EXPECT_THAT(grackle::impl::FrozenKeyIdxBiMap_find(clone_p, "notAKey"),
               EmptyAccessRslt());
 
-  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 3), nullptr);
-  EXPECT_STREQ(grackle::impl::FrozenKeyIdxBiMap_inverse_get(clone_p, 1),
+  EXPECT_EQ(grackle::impl::FrozenKeyIdxBiMap_inverse_find(clone_p, 3), nullptr);
+  EXPECT_STREQ(grackle::impl::FrozenKeyIdxBiMap_inverse_find(clone_p, 1),
                "density");
 
   // check whether the clone is using pointers to the keys used during init

From 37915539c82787db981f3954faf3467a8088546b Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 7 Jan 2026 13:13:22 -0500
Subject: [PATCH 24/25] some final cleanup

---
 src/clib/support/FrozenKeyIdxBiMap.hpp | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index 40ebcfdc8..97f2e0c94 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -29,13 +29,9 @@
 
 namespace grackle::impl {
 
-// the motivation for these constants are provided in PR #484 (they are related
+// the motivation for this constant is provided in PR #484 (its related
 // to some optimizations in the FrozenKeyIdxBiMap implementation)
 namespace bimap_detail {
-/// specifies an invalid value of the map (we state that you can't store the
-/// maximum u16 value)
-inline constexpr uint16_t INVALID_VAL = std::numeric_limits<uint16_t>::max();
-
 /// specifies maximum allowed length of a key (excluding the null character).
 inline constexpr uint16_t KEYLEN_MAX = 29;
 }  // namespace bimap_detail
@@ -128,7 +124,7 @@ struct FrozenKeyIdxBiMap {
 /// ugly/clunky, but it's the only practical way to achieve comparable behavior
 /// to other internal data types. The best alternatives involve things like
 /// std::optional or converting this type to a simple C++ class.
-inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
+inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* const keys[],
                                                int key_count, BiMapMode mode) {
   // this will be returned if there is an error
   FrozenKeyIdxBiMap erroneous_obj{-1, nullptr, BiMapMode::REFS_KEYDATA};
@@ -138,10 +134,9 @@ inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* keys[],
   }
 
   // check the specified keys
-  long long max_keys = static_cast<long long>(bimap_detail::INVALID_VAL) - 1LL;
-  if (key_count < 1 || static_cast<long long>(key_count) > max_keys) {
-    GrPrintErrMsg("key_count must be positive and cannot exceed %lld",
-                  max_keys);
+  int max_keys = static_cast<int>(std::numeric_limits<uint16_t>::max());
+  if (key_count < 1 || key_count > max_keys) {
+    GrPrintErrMsg("key_count must be positive & can't exceed %d", max_keys);
     return erroneous_obj;
   } else if (keys == nullptr) {
     GrPrintErrMsg("keys must not be a nullptr");
@@ -263,7 +258,9 @@ namespace bimap {
 
 /// holds the result of a call to @ref FrozenKeyIdxBiMap_find
 ///
-/// @note This C-style approximation of std::optional<uint16_t>
+/// @note
+/// This is a C-style approximation of std::optional<uint16_t>. Additionally,
+/// the choice to make value a uint16_t is motivated by PR #484
 struct AccessRslt {
   /// Indicates whether the value member is valid
   bool has_value;

From 1d645e2ffc2262597e351ec937a3b94be37bd844 Mon Sep 17 00:00:00 2001
From: Matthew Abruzzo <matthewabruzzo@gmail.com>
Date: Wed, 7 Jan 2026 14:50:31 -0500
Subject: [PATCH 25/25] another few tweaks

---
 src/clib/support/FrozenKeyIdxBiMap.hpp        | 36 ++++++++--------
 src/clib/support/FrozenKeyIdxBiMap_detail.hpp | 43 ++++++++++---------
 2 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/src/clib/support/FrozenKeyIdxBiMap.hpp b/src/clib/support/FrozenKeyIdxBiMap.hpp
index 14dc0b165..3057d6336 100644
--- a/src/clib/support/FrozenKeyIdxBiMap.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap.hpp
@@ -106,8 +106,8 @@ enum class BiMapMode {
 ///   @c std::shared_ptr for reference counting.
 ///
 /// @par
-/// I would be stunned if <tt> std::map<std::string, std::uint16_t> </tt> or
-/// <tt> std::map<const char*, std::uint16_t> </tt> is faster than the internal
+/// I would be stunned if <tt> std::map<std::string, uint16_t> </tt> or
+/// <tt> std::map<const char*, uint16_t> </tt> is faster than the internal
 /// hash table since @c std::map is usually implemented as a tree.
 ///
 /// @par Potential Improvements
@@ -127,7 +127,7 @@ enum class BiMapMode {
 ///
 ///   bool is_empty(PackedRow& r) { return data[0] == '\0' }
 ///   const char* get_key(PackedRow r) { return r.data; }
-///   std::uint16_t get_val(Packed r) {
+///   uint16_t get_val(Packed r) {
 ///     stdd::uint16_t o;
 ///     std::memcpy(&o, r.data+30, 2);
 ///     return o;
@@ -153,18 +153,18 @@ struct FrozenKeyIdxBiMap {
   // don't forget to update FrozenKeyIdxBiMap_clone when changing members
 
   /// the number of contained strings
-  bimap::rowidx_type length;
+  bimap_detail::rowidx_type length;
   /// the number of elements in table_rows
-  bimap::rowidx_type capacity;
+  bimap_detail::rowidx_type capacity;
   /// max number of rows that must be probed to determine if a key is contained
-  bimap::rowidx_type max_probe;
+  bimap_detail::rowidx_type max_probe;
   /// specifies ownership of keys, @see BiMapMode
   BiMapMode mode;
 
   /// actual hash table data
   bimap_StrU16_detail::Row* table_rows;
   /// tracks the row indices to make iteration faster
-  bimap::rowidx_type* ordered_row_indices;
+  bimap_detail::rowidx_type* ordered_row_indices;
 };
 
 /// Create an invalid FrozenKeyIdxBiMap
@@ -238,7 +238,7 @@ inline void drop_FrozenKeyIdxBiMap(FrozenKeyIdxBiMap* ptr) {
   if (FrozenKeyIdxBiMap_is_ok(ptr)) {
     if (ptr->length > 0) {
       if (ptr->mode == BiMapMode::COPIES_KEYDATA) {
-        for (bimap::rowidx_type i = 0; i < ptr->capacity; i++) {
+        for (bimap_detail::rowidx_type i = 0; i < ptr->capacity; i++) {
           bimap_StrU16_detail::Row* row = ptr->table_rows + i;
           // casting from (const char*) to (char*) should be legal (as long as
           // there were no bugs modifying the value of ptr->mode)
@@ -350,12 +350,12 @@ inline const char* FrozenKeyIdxBiMap_inverse_find(const FrozenKeyIdxBiMap* map,
 namespace bimap_detail {
 
 /// a helper function used to actually allocate memory for FrozenKeyIdxBiMap
-inline FrozenKeyIdxBiMap alloc(std::uint16_t length, std::uint16_t capacity,
+inline FrozenKeyIdxBiMap alloc(uint16_t length, uint16_t capacity,
                                BiMapMode mode) {
   // it would be nice to handle allocate all pointers as a single block of
   // memory, but that gets tricky. Essentially, we would allocate uninitialized
   // memory and manually use placement-new (and the corresponding `delete`)
-  using bimap::rowidx_type;
+  using bimap_detail::rowidx_type;
   using bimap_StrU16_detail::Row;
   FrozenKeyIdxBiMap out = {
       /*length=*/length,
@@ -364,7 +364,7 @@ inline FrozenKeyIdxBiMap alloc(std::uint16_t length, std::uint16_t capacity,
       /*mode=*/mode,
       /*table_rows=*/(capacity > 0) ? new Row[capacity] : nullptr,
       /*ordered_row_indices=*/(length > 0) ? new rowidx_type[length] : nullptr};
-  for (std::uint16_t i = 0; i < capacity; i++) {
+  for (uint16_t i = 0; i < capacity; i++) {
     out.table_rows[i].keylen = 0;
   }
   return out;
@@ -374,26 +374,28 @@ inline FrozenKeyIdxBiMap alloc(std::uint16_t length, std::uint16_t capacity,
 
 inline FrozenKeyIdxBiMap new_FrozenKeyIdxBiMap(const char* const keys[],
                                                int key_count, BiMapMode mode) {
-  long long max_len = static_cast<long long>(bimap_cap_detail::max_key_count());
+  int64_t max_len = static_cast<int64_t>(bimap_cap_detail::max_key_count());
   if (keys == nullptr && key_count == 0) {
     return bimap_detail::alloc(0, 0, mode);
   } else if (keys == nullptr) {
     GrPrintErrMsg("keys must not be a nullptr");
     return mk_invalid_FrozenKeyIdxBiMap();
-  } else if (key_count < 1 || static_cast<long long>(key_count) > max_len) {
+  } else if (key_count < 1 || static_cast<int64_t>(key_count) > max_len) {
     GrPrintErrMsg("key_count must be positive & can't exceed %lld", max_len);
     return mk_invalid_FrozenKeyIdxBiMap();
   }
 
   // based on the preceding check, this shouldn't be able to fail
-  bimap::rowidx_type capacity = bimap_cap_detail::calc_map_capacity(key_count);
+  bimap_detail::rowidx_type capacity =
+      bimap_cap_detail::calc_map_capacity(key_count);
   GR_INTERNAL_REQUIRE(capacity > 0, "something went wrong");
 
   // let's validate the keys
   for (int i = 0; i < key_count; i++) {
     GR_INTERNAL_REQUIRE(keys[i] != nullptr, "Can't specify a nullptr key");
     std::size_t n_chrs_without_nul = std::strlen(keys[i]);
-    if (n_chrs_without_nul == 0 || n_chrs_without_nul > bimap_detail::KEYLEN_MAX) {
+    if (n_chrs_without_nul == 0 ||
+        n_chrs_without_nul > bimap_detail::KEYLEN_MAX) {
       GrPrintErrMsg(
           "calling strlen on \"%s\", the key @ index %d, yields 0 or a length "
           "exceeding %d",
@@ -450,7 +452,7 @@ inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
       "something is very wrong!");
 
   bool copy_key_data = out.mode == BiMapMode::COPIES_KEYDATA;
-  for (bimap::rowidx_type i = 0; i < ptr->capacity; i++) {
+  for (bimap_detail::rowidx_type i = 0; i < ptr->capacity; i++) {
     const bimap_StrU16_detail::Row& ref_row = ptr->table_rows[i];
     if (ref_row.keylen > 0) {
       bimap_StrU16_detail::overwrite_row(out.table_rows + i, ref_row.key,
@@ -459,7 +461,7 @@ inline FrozenKeyIdxBiMap FrozenKeyIdxBiMap_clone(const FrozenKeyIdxBiMap* ptr) {
     }
   }
 
-  for (bimap::rowidx_type i = 0; i < ptr->length; i++) {
+  for (bimap_detail::rowidx_type i = 0; i < ptr->length; i++) {
     out.ordered_row_indices[i] = ptr->ordered_row_indices[i];
   }
   return out;
diff --git a/src/clib/support/FrozenKeyIdxBiMap_detail.hpp b/src/clib/support/FrozenKeyIdxBiMap_detail.hpp
index 2b82af660..24cacd4b9 100644
--- a/src/clib/support/FrozenKeyIdxBiMap_detail.hpp
+++ b/src/clib/support/FrozenKeyIdxBiMap_detail.hpp
@@ -38,17 +38,16 @@ inline constexpr uint16_t INVALID_VAL = std::numeric_limits<uint16_t>::max();
 /// strings to 30 elements (including the terminator) allows for a hypothetical
 /// optimization (see the @ref FrozenKeyIdxBiMap docstring for info).
 inline constexpr uint16_t KEYLEN_MAX = 29;
-}  // namespace bimap_detail
 
-namespace bimap {
 /// the type used for indexing the rows of the BiMap's internal hash table
 ///
 /// @note
 /// This does not necessarily need to be a uint16_t (even if the internal
-/// values of the hash table are uint16_t).
-typedef std::uint16_t rowidx_type;
+/// values of the hash table are uint16_t). It just needs to be able to hold
+/// the maximum possible value
+typedef uint16_t rowidx_type;
 
-}  // namespace bimap
+}  // namespace bimap_detail
 
 // -----------------------------------------------------------------
 
@@ -66,17 +65,19 @@ static_assert(INVERSE_LOAD_FACTOR > 1);
 /// list of allowed capacities. These are all prime numbers that
 /// have nearly constant linear spacing (it may make more sense to have
 /// logarithmic spacing)
-inline constexpr std::uint32_t CAPACITIES_LIST[] = {
-    7,   19,  31,  41,  53,  61,  71,  83,  89,  101, 113, 127,
-    139, 149, 163, 173, 181, 191, 199, 211, 223, 233, 241, 251};
+inline constexpr uint32_t CAPACITIES_LIST[] = {
+    // each number increases by ~10 in this first batch
+    7, 19, 31, 41, 53, 61, 71, 83, 89, 101, 113, 127, 139, 149, 163, 173, 181,
+    191, 199, 211, 223, 233, 241, 251,
+    // probably won't use these (so we increase spacing:
+    293, 401, 503, 601, 701, 797, 907, 997};
 
-inline constexpr int N_CAPACITIES =
-    (sizeof(CAPACITIES_LIST) / sizeof(std::uint32_t));
+inline constexpr int N_CAPACITIES = sizeof(CAPACITIES_LIST) / sizeof(uint32_t);
 
 /// compute the maximum number of keys
-inline bimap::rowidx_type max_key_count() {
-  return std::min<bimap::rowidx_type>(
-      std::numeric_limits<bimap::rowidx_type>::max(),
+inline bimap_detail::rowidx_type max_key_count() {
+  return std::min<bimap_detail::rowidx_type>(
+      std::numeric_limits<bimap_detail::rowidx_type>::max(),
       CAPACITIES_LIST[N_CAPACITIES - 1] / INVERSE_LOAD_FACTOR);
 }
 
@@ -84,11 +85,11 @@ inline bimap::rowidx_type max_key_count() {
 /// capacity can't be found)
 ///
 /// @param key_count the desired number of keys (should be positive)
-inline std::uint16_t calc_map_capacity(int key_count) {
-  std::uint64_t c = INVERSE_LOAD_FACTOR * static_cast<std::uint64_t>(key_count);
+inline uint16_t calc_map_capacity(int key_count) {
+  uint64_t c = INVERSE_LOAD_FACTOR * static_cast<uint64_t>(key_count);
   for (int i = 0; i < N_CAPACITIES; i++) {  // binary search may be faster
     if (c < CAPACITIES_LIST[i]) {
-      return static_cast<std::uint16_t>(CAPACITIES_LIST[i]);
+      return static_cast<uint16_t>(CAPACITIES_LIST[i]);
     }
   }
   return 0;
@@ -112,18 +113,18 @@ namespace bimap_StrU16_detail {
 /// listed first) to pack as many entries into a cacheline as possible
 struct Row {
   /// specifies the value associated with the current key
-  std::uint16_t value;
+  uint16_t value;
 
   /// specifies the length of the key (not including the '\0')
   ///
   /// @note Tracked for short-circuiting comparisons (while probing collisions)
-  std::uint16_t keylen;
+  uint16_t keylen;
   /// identifies the address of this entry's key
   const char* key;
 };
 
-static void overwrite_row(Row* row, const char* key, std::uint16_t keylen,
-                          std::uint16_t value, bool copy_key_data) {
+static void overwrite_row(Row* row, const char* key, uint16_t keylen,
+                          uint16_t value, bool copy_key_data) {
   GR_INTERNAL_REQUIRE(row->keylen == 0, "Sanity check failed!");
   row->value = value;
   row->keylen = keylen;
@@ -144,7 +145,7 @@ static void overwrite_row(Row* row, const char* key, std::uint16_t keylen,
 /// return a struct of integers than rely on modifying pointer arguments
 struct SearchRslt {
   /// specifies value found by the search (or @ref bimap_detail::INVALID_VAL)
-  std::uint16_t val;
+  uint16_t val;
   /// specified the number of probes before the search returned
   int probe_count;
   /// specify the index of the "row" corresponding to the search result