Serialization: EnumValid uses switch, not sentinel; add framing

Calling visitor() on a sub-struct adds size field, whereas the direct call to VisitFields did not. This should be a no-op for existing models.

PiperOrigin-RevId: 955295371

Author: pculliton

gemma/configs.h

@@ -153,24 +153,34 @@ AttentionImpl GetAttentionImpl(const std::string& impl);
 enum class PostNormType {
   None,
   Scale,
-  kSentinel  // must be last
 };
 
 static inline bool EnumValid(PostNormType type) {
-  return static_cast<size_t>(type) <
-         static_cast<size_t>(PostNormType::kSentinel);
+  switch (type) {
+    case PostNormType::None:
+    case PostNormType::Scale:
+      return true;
+    default:
+      return false;
+  }
 }
 
 // Post qk projection operation type.
 enum class PostQKType {
   Rope,
   HalfRope,
   NormLocalRope = 8,  // Norm without scale, and rope for local attention layers
-  kSentinel           // must be last
 };
 
 static inline bool EnumValid(PostQKType type) {
-  return static_cast<size_t>(type) < static_cast<size_t>(PostQKType::kSentinel);
+  switch (type) {
+    case PostQKType::Rope:
+    case PostQKType::HalfRope:
+    case PostQKType::NormLocalRope:
+      return true;
+    default:
+      return false;
+  }
 }
 
 // FFW activation function.
@@ -215,12 +225,15 @@ static inline bool EnumValid(QueryScaleType type) {
 // Residual connection type.
 enum class ResidualType {
   Add,
-  kSentinel  // must be last
 };
 
 static inline bool EnumValid(ResidualType type) {
-  return static_cast<size_t>(type) <
-         static_cast<size_t>(ResidualType::kSentinel);
+  switch (type) {
+    case ResidualType::Add:
+      return true;
+    default:
+      return false;
+  }
 }
 
 template <size_t kNum>
@@ -314,6 +327,8 @@ void ForEachModel(const Func& func) {
   }
 }
 
+static inline bool IsInternal(Model model) { return false; }
+
 static inline bool EnumValid(Model model) {
   // Valid for purposes of serialization, even if unknown.
   if (model == Model::UNKNOWN) return true;
@@ -360,7 +375,7 @@ struct LayerConfig : public IFields {
     visitor(activation);
     visitor(post_qk);
     visitor(use_qk_norm);
-    internal.VisitFields(visitor);
+    // Visiting includes size prefix, whereas calling VisitFields would inline.
     visitor(norm_v);
     visitor(num_experts);
     visitor(num_experts_per_datapoint);
@@ -611,7 +626,7 @@ struct ModelConfig : public IFields {
 
     visitor(scale_base_names);
 
-    internal.VisitFields(visitor);
+    // Visiting includes size prefix, whereas calling VisitFields would inline.
 
     visitor(use_global_timescale);
     visitor(partial_rotary_factor);

python/convert_from_safetensors.py

@@ -1181,7 +1181,15 @@ def add_cross_projection(prefix, hf_name, sbs_name, i):
     raise ValueError(
         f"{sbs_model_specifier!r} is not an encoder-decoder config."
     )
-  writer.write(sbs_config, tokenizer_file)
+  if tokenizer_file.endswith(".json"):
+    sbs_config.tokenizer_kind = configs.TokenizerKind.kHfBpe
+    tokenizer_blob = pack_bpe_tokenizer(tokenizer_file)
+  else:
+    sbs_config.tokenizer_kind = configs.TokenizerKind.kSentencePiece
+    with open(tokenizer_file, "rb") as f:
+      tokenizer_blob = f.read()
+  writer.write(sbs_config, tokenizer_blob)
+
 
   with open(csv_file, "w") as csv_handle:
     csv.writer(csv_handle).writerows(metadata)