Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions modelopt/torch/export/unified_export_hf.py
Original file line number Diff line number Diff line change
Expand Up @@ -826,11 +826,15 @@ def _dispatch_export_handler(name: str, sub_module: nn.Module, ctx: ExportContex

def _resolve_export_dtype(model: nn.Module, dtype: torch.dtype | None) -> torch.dtype:
"""Return the export dtype, defaulting to the model's own and warning on a mismatch."""
configured_dtype = getattr(model.config, "torch_dtype", None)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

How does a Diffusers export reach this function? export_hf_checkpoint() dispatches is_diffusers_object(model) to _export_diffusers_checkpoint(), which resolves dtype through infer_dtype_from_model() and never reads model.config.torch_dtype; _resolve_export_dtype is only reachable from the transformers (resident + streaming) paths. If a diffusers pipeline/component is landing here, that means is_diffusers_object() returned False (most likely HAS_DIFFUSERS/_HAS_DIFFUSERS being False because the from diffusers import ... guard failed under the minimum-transformers combo), and the root cause is the detection, not the missing torch_dtype — this patch would only push the failure to the next transformers-only step (TiedWeightMap(model), requantize_resmooth_fused_llm_layers(model), model.named_modules() on a pipeline). Please include the traceback in the PR body so the actual failing dispatch is on record.

Also, getattr(model.config, ...) still raises if the object has no .config at all; getattr(getattr(model, "config", None), "torch_dtype", None) would be the fully defensive form if that's the intent.

if dtype is None:
return model.config.torch_dtype
if dtype != model.config.torch_dtype:
if configured_dtype is not None:
return configured_dtype
first_parameter = next(model.parameters(), None)
return first_parameter.dtype if first_parameter is not None else torch.float16

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

This is a verbatim re-implementation of infer_dtype_from_model() in modelopt/torch/export/diffusers_utils.py (first parameter dtype, torch.float16 fallback), which the diffusers export path already uses via _export_diffusers_checkpoint. Please call that helper here rather than adding a second copy — otherwise the two defaults can drift apart. If importing it from diffusers_utils is awkward because that module is diffusers-optional, note that the function itself has no diffusers dependency and could be moved to a neutral module (e.g. model_utils.py).

if configured_dtype is not None and dtype != configured_dtype:
warnings.warn(
f"Model's original dtype ({model.config.torch_dtype}) differs from target dtype "
f"Model's original dtype ({configured_dtype}) differs from target dtype "
f"({dtype}), which may lead to numerical errors."
)
return dtype
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/torch/export/test_unified_export_hf.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

"""Tests for tied-weight helpers in unified_export_hf."""

from types import SimpleNamespace

import pytest
import torch
from _test_utils.torch.quantization.tied_modules import (
Expand All @@ -29,9 +31,44 @@
postprocess_state_dict,
sync_tied_input_amax,
)
from modelopt.torch.export.unified_export_hf import _resolve_export_dtype
from modelopt.torch.quantization.nn import TensorQuantizer


@pytest.mark.parametrize(
("configured_dtype", "dtype", "expected_dtype", "warning_count"),
[
(None, None, torch.float32, 0),
(None, torch.float16, torch.float16, 0),
(torch.bfloat16, None, torch.bfloat16, 0),
(torch.bfloat16, torch.bfloat16, torch.bfloat16, 0),
(torch.bfloat16, torch.float16, torch.float16, 1),
],
)
def test_resolve_export_dtype(configured_dtype, dtype, expected_dtype, warning_count, recwarn):
model = torch.nn.Linear(1, 1)
model.config = (
SimpleNamespace(torch_dtype=configured_dtype) if configured_dtype is not None else object()
)

assert _resolve_export_dtype(model, dtype) == expected_dtype
assert len(recwarn) == warning_count
if warning_count:
assert str(recwarn[0].message) == (
"Model's original dtype (torch.bfloat16) differs from target dtype "
"(torch.float16), which may lead to numerical errors."
)


def test_resolve_export_dtype_with_empty_diffusers_config():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

This case is behaviorally identical to the already-parameterized (None, None, torch.float32, 0) row: an empty FrozenDict has no torch_dtype attribute, so both take the next(model.parameters()) branch. What isn't covered is (a) a config that does carry torch_dtype as a string ("bfloat16") — which is how diffusers FrozenDicts deserialize it from JSON, and which would be returned as-is here and then fed to weight.to(...), and (b) the parameterless torch.float16 fallback. Both are worth a row.

# Import locally so Diffusers stays optional during torch-only test collection.
frozen_dict = pytest.importorskip("diffusers.configuration_utils").FrozenDict()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
model = torch.nn.Linear(1, 1)
model.config = frozen_dict

assert _resolve_export_dtype(model, None) == torch.float32


def test_hf_all_tied_weights_keys_contract():
"""Pin the transformers API we build tied_map from, so a version bump fails loud here.

Expand Down
Loading