From 90279d769a5dca6f0663306e3053d35017ed7fc2 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Sun, 29 Mar 2026 19:21:08 -0400 Subject: [PATCH] fix: VLMModelWrapper PEFT isinstance compatibility for TRL validation TRL's validate_quantization_for_training() uses isinstance(model, PeftModel) to check for adapters. The wrapper hid the PeftModel, causing: "You cannot perform fine-tuning on purely quantized models." Fix: dynamically create a combined class inheriting from both VLMModelWrapper and the wrapped model's type. This makes isinstance() pass while keeping our forward/generate/cache methods via MRO. Tests added: - test_peft_attributes_delegated: peft_config accessible through wrapper - test_hasattr_peft_config: hasattr() works for TRL's checks - test_isinstance_peft_model: isinstance(wrapper, PeftModel) == True - test_wrapper_passes_peft_validation (e2e): full TRL validation sim - test_wrapper_preserves_trainable_parameters (e2e): optimizer setup Co-Authored-By: Claude Opus 4.6 (1M context) --- openadapt_evals/training/vlm_wrapper.py | 41 +++++++++++++++++++ tests/test_trl_e2e.py | 53 +++++++++++++++++++++++++ tests/test_vlm_wrapper.py | 46 +++++++++++++++++++++ 3 files changed, 140 insertions(+) diff --git a/openadapt_evals/training/vlm_wrapper.py b/openadapt_evals/training/vlm_wrapper.py index 60c4db1..ae980ec 100644 --- a/openadapt_evals/training/vlm_wrapper.py +++ b/openadapt_evals/training/vlm_wrapper.py @@ -50,6 +50,47 @@ def __init__(self, model: Any): object.__setattr__(self, "_cache_hits", 0) object.__setattr__(self, "_cache_misses", 0) + # --- PEFT / quantization compatibility --- + # TRL's validate_quantization_for_training() checks for PEFT via: + # 1. isinstance(model, PeftModel) — fails because wrapper isn't PeftModel + # 2. hasattr(model, "peft_config") — works via our __getattr__ + # 3. Checking model.is_quantized / model.quantization_method + # + # The isinstance check is the blocker. We solve it by making the + # wrapper's __class__ inherit from the wrapped model's type, so + # isinstance(wrapper, PeftModel) returns True. + try: + from peft import PeftModel + if isinstance(model, PeftModel): + # Create a new class that inherits from BOTH our wrapper + # and the actual model class. This makes isinstance work + # while keeping our forward/generate/cache methods. + combined = type( + "VLMPeftModelWrapper", + (VLMModelWrapper, type(model)), + { + # Ensure our methods take priority (MRO) + "forward": VLMModelWrapper.forward, + "generate": VLMModelWrapper.generate, + "__call__": VLMModelWrapper.__call__, + "cache_vision_inputs": VLMModelWrapper.cache_vision_inputs, + "__getattr__": VLMModelWrapper.__getattr__, + }, + ) + object.__setattr__(self, "__class__", combined) + logger.info( + "VLMModelWrapper: PEFT isinstance compatibility enabled " + "(wrapped model is %s)", type(model).__name__, + ) + except ImportError: + pass + except Exception as exc: + # If dynamic class fails, fall back to attribute-level compat + logger.warning( + "VLMModelWrapper: PEFT isinstance setup failed: %s. " + "Falling back to attribute-level compatibility.", exc, + ) + def cache_vision_inputs(self, inputs: dict[str, Any]) -> None: """Cache vision tensors from a processor output dict. diff --git a/tests/test_trl_e2e.py b/tests/test_trl_e2e.py index 999653f..f553a31 100644 --- a/tests/test_trl_e2e.py +++ b/tests/test_trl_e2e.py @@ -328,3 +328,56 @@ def test_vision_changes_logits(self): "Either the model ignores vision inputs or the wrapper " "isn't injecting them. Training will produce zero gradient." ) + + def test_wrapper_passes_peft_validation(self): + """VLMModelWrapper passes TRL's PEFT/quantization validation. + + TRL checks isinstance(model, PeftModel) to verify adapters are + attached to quantized models. The wrapper must pass this check. + Without it: ValueError: "You cannot perform fine-tuning on purely + quantized models. Please attach trainable adapters." + """ + torch = pytest.importorskip("torch") + + try: + from peft import PeftModel + except ImportError: + pytest.skip("peft not installed") + + from openadapt_evals.training.vlm_wrapper import VLMModelWrapper + + # Create a mock PeftModel (has peft_config, active_adapter, etc.) + model = MagicMock(spec=PeftModel) + model.peft_config = {"default": MagicMock()} + model.active_adapter = "default" + model.parameters.return_value = iter([torch.zeros(1, requires_grad=True)]) + + wrapper = VLMModelWrapper(model) + + # The critical check TRL performs + assert isinstance(wrapper, PeftModel), ( + "isinstance(wrapper, PeftModel) must be True. " + "TRL rejects quantized models without PEFT adapters." + ) + assert hasattr(wrapper, "peft_config"), ( + "wrapper must expose peft_config for TRL validation." + ) + + def test_wrapper_preserves_trainable_parameters(self): + """VLMModelWrapper exposes trainable parameters for the optimizer. + + TRL needs model.parameters() to set up the optimizer. The wrapper + must delegate this to the wrapped model. + """ + torch = pytest.importorskip("torch") + from openadapt_evals.training.vlm_wrapper import VLMModelWrapper + + model, _ = self._make_tiny_vlm_and_processor() + wrapper = VLMModelWrapper(model) + + # Verify parameters are accessible and trainable + params = list(wrapper.parameters()) + assert len(params) > 0, "Wrapper must expose model parameters" + assert any(p.requires_grad for p in params), ( + "At least some parameters must require grad for training" + ) diff --git a/tests/test_vlm_wrapper.py b/tests/test_vlm_wrapper.py index 50fafd1..a1084c4 100644 --- a/tests/test_vlm_wrapper.py +++ b/tests/test_vlm_wrapper.py @@ -145,6 +145,52 @@ def test_empty_cache_from_text_only_inputs(self): wrapper.forward(input_ids="ids") assert "pixel_values" not in model.last_forward_kwargs + def test_peft_attributes_delegated(self): + """PEFT attributes are accessible through the wrapper.""" + model = _FakeModel() + model.peft_config = {"default": "lora_config"} + model.active_adapter = "default" + wrapper = VLMModelWrapper(model) + + assert wrapper.peft_config == {"default": "lora_config"} + assert wrapper.active_adapter == "default" + + def test_hasattr_peft_config(self): + """hasattr(wrapper, 'peft_config') returns True when model has it.""" + model = _FakeModel() + model.peft_config = {"default": "config"} + wrapper = VLMModelWrapper(model) + + assert hasattr(wrapper, "peft_config"), ( + "hasattr(wrapper, 'peft_config') must return True for TRL's " + "validate_quantization_for_training() to pass." + ) + + def test_hasattr_peft_config_false_when_missing(self): + """hasattr(wrapper, 'peft_config') returns False when model lacks it.""" + model = _FakeModel() + wrapper = VLMModelWrapper(model) + + assert not hasattr(wrapper, "peft_config") + + def test_isinstance_peft_model(self): + """isinstance(wrapper, PeftModel) works when PEFT is available.""" + try: + from peft import PeftModel + except ImportError: + pytest.skip("peft not installed") + + # Create a mock that isinstance recognizes as PeftModel + model = MagicMock(spec=PeftModel) + model.peft_config = {"default": "config"} + wrapper = VLMModelWrapper(model) + + assert isinstance(wrapper, PeftModel), ( + "isinstance(wrapper, PeftModel) must return True. " + "TRL's validation uses isinstance to detect PEFT adapters. " + "Without this, TRL rejects quantized models." + ) + # --------------------------------------------------------------------------- # Real e2e test with a tiny torch model (requires torch — skipped in CI)