Ensure we still accept None as value

Author: thvasilo

tests/workers/config/test_model_config_on_cpu.py

@@ -52,6 +52,21 @@ def test_target_modules_accepts_list_via_omegaconf(self):
         # Verify the list was merged correctly
         assert list(merged.target_modules) == ["k_proj", "o_proj", "q_proj", "v_proj"]
 
+    def test_target_modules_accepts_none_via_omegaconf(self):
+        """Test that target_modules still accepts None values."""
+
+        cfg_from_dataclass = OmegaConf.structured(HFModelConfig)
+
+        cli_config = OmegaConf.create(
+            {
+                "path": self.model_path,
+                "target_modules": None,
+            }
+        )
+
+        merged = OmegaConf.merge(cfg_from_dataclass, cli_config)
+        assert merged.target_modules is None
+
     def test_target_modules_accepts_string_via_omegaconf(self):
         """Test that target_modules still accepts string values."""
 

verl/workers/config/model.py

@@ -204,15 +204,19 @@ def __post_init__(self):
         if getattr(self.hf_config, "model_type", None) == "kimi_vl":
             self.hf_config.text_config.topk_method = "greedy"
 
-        # Ensure target_modules is a str or list[str]
-        if self.target_modules is None:
-            self.target_modules = "all-linear"
-        if not isinstance(self.target_modules, (str, list)):
-            raise TypeError(f"target_modules must be a string or a list of strings, but got {type(self.target_modules).__name__}")
-        if isinstance(self.target_modules, list):
-            for x in self.target_modules:
-                if not isinstance(x, str):
-                    raise TypeError(f"All elements in target_modules list must be strings, but found {type(x).__name__}")
+        # Ensure target_modules is a str or list[str] (only if not None)
+        if self.target_modules is not None:
+            if not isinstance(self.target_modules, (str | list)):
+                raise TypeError(
+                    "target_modules must be a string or a list of strings, "
+                    f"but got {type(self.target_modules).__name__}"
+                )
+            if isinstance(self.target_modules, list):
+                for x in self.target_modules:
+                    if not isinstance(x, str):
+                        raise TypeError(
+                            f"All elements in target_modules list must be strings, but found {type(x).__name__}"
+                        )
 
     def get_processor(self):
         return self.processor if self.processor is not None else self.tokenizer