-
Notifications
You must be signed in to change notification settings - Fork 535
Preserve HF PTQ checkpoint sidecar files [NV BUG 6491822] #2060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
da68df5
cec2df9
e18d5e4
874a57c
d669f9c
7e701d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,6 @@ | |
| import json | ||
| import logging | ||
| import os | ||
| import shutil | ||
| import warnings | ||
| from collections.abc import Callable, Iterable | ||
| from dataclasses import dataclass | ||
|
|
@@ -44,6 +43,7 @@ | |
| ) | ||
|
|
||
| from modelopt.torch.export.model_utils import is_multimodal_model | ||
| from modelopt.torch.export.plugins.hf_checkpoint_utils import copy_non_safetensor_files_from_ckpt | ||
|
|
||
| try: | ||
| from huggingface_hub import snapshot_download | ||
|
|
@@ -56,6 +56,52 @@ | |
|
|
||
| SPECULATIVE_MODEL_LIST = ["Eagle", "Medusa"] | ||
|
|
||
| _HF_SIDECAR_DOWNLOAD_ALLOW_PATTERNS = [ | ||
| "*.jinja", | ||
| "*.json", | ||
| "*.md", | ||
| "*.model", | ||
| "*.py", | ||
| "*.tiktoken", | ||
| "*.txt", | ||
| "LICENSE*", | ||
| "NOTICE*", | ||
| ] | ||
|
jenchen13 marked this conversation as resolved.
|
||
| _HF_PTQ_WEIGHT_FILE_PATTERNS = ( | ||
| "*.safetensors", | ||
|
jenchen13 marked this conversation as resolved.
|
||
| "*.safetensors.index.json", | ||
| "*.bin", | ||
| "*.bin.index.json", | ||
| "*.ckpt", | ||
| "*.gguf", | ||
| "*.h5", | ||
| "*.msgpack", | ||
| "*.npy", | ||
| "*.npz", | ||
| "*.onnx", | ||
| "*.pb", | ||
| "*.pickle", | ||
| "*.pkl", | ||
| "*.pt", | ||
| "*.pth", | ||
| "*.tar", | ||
| "*.tar.bz2", | ||
| "*.tar.gz", | ||
| "*.tar.xz", | ||
| "*.tflite", | ||
| "*.tgz", | ||
| "*.zip", | ||
| ) | ||
| _HF_PTQ_EXPORT_OWNED_FILES = { | ||
| "config.json", | ||
|
jenchen13 marked this conversation as resolved.
|
||
| "hf_quant_config.json", | ||
| "quant_config.json", | ||
| "quantization_config.json", | ||
| "quantize_config.json", | ||
| "recipe.yaml", | ||
| "recipe.yml", | ||
| } | ||
|
jenchen13 marked this conversation as resolved.
Comment on lines
+104
to
+112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SUGGESTION] The name
Worth splitting so the intent is self-documenting, and so the # Written by the export itself; the source copy must not clobber them.
_HF_PTQ_EXPORT_OWNED_FILES = {
"config.json",
"hf_quant_config.json",
}
# Quantization metadata from an already-quantized source (compressed-tensors,
# llm-compressor, MXFP4). Dropped, not overwritten: carrying it next to a freshly
# written hf_quant_config.json makes deployment stacks pick the wrong algorithm.
_HF_PTQ_STALE_QUANT_METADATA_FILES = {
"quant_config.json",
"quantization_config.json",
"quantize_config.json",
"recipe.yaml",
"recipe.yml",
}and at the call site: |
||
|
|
||
|
|
||
| @dataclass | ||
| class DistributedState: | ||
|
|
@@ -895,11 +941,13 @@ def _resolve_model_path(model_name_or_path: str, trust_remote_code: bool = False | |
| try: | ||
| local_path = snapshot_download( | ||
| repo_id=model_name_or_path, | ||
| allow_patterns=["*.py", "*.json"], # Only download Python files and config | ||
| allow_patterns=_HF_SIDECAR_DOWNLOAD_ALLOW_PATTERNS, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [IMPORTANT Compatibility] For HF-Hub-ID sources, the widening this PR is about is capped by a second, independent whitelist — so sidecars are still silently dropped. What happens. When Concrete misses (all copied fine for a local source dir, dropped for a Hub ID):
This isn't a regression (the old list was just Suggested fix. local_path = snapshot_download(
repo_id=model_name_or_path,
ignore_patterns=_HF_PTQ_WEIGHT_FILE_PATTERNS,
)
|
||
| ) | ||
|
jenchen13 marked this conversation as resolved.
|
||
| return local_path | ||
| except Exception as e: | ||
| print(f"Warning: Could not download model files using snapshot_download: {e}") | ||
| print( | ||
| f"Warning: Could not download checkpoint sidecars using snapshot_download: {e}" | ||
| ) | ||
|
|
||
| # Fallback: try to find in HuggingFace cache | ||
| from transformers.utils import TRANSFORMERS_CACHE | ||
|
|
@@ -934,49 +982,31 @@ def _resolve_model_path(model_name_or_path: str, trust_remote_code: bool = False | |
| return model_name_or_path | ||
|
|
||
|
|
||
| def copy_custom_model_files(source_path: str, export_path: str, trust_remote_code: bool = False): | ||
| """Copy processor/tokenizer artifacts (and, with trust_remote_code, custom code) to export. | ||
|
|
||
| Processor and tokenizer *data* artifacts -- e.g. a VLM's ``preprocessor_config.json``, | ||
| ``merges.txt``/``vocab.json``, and the processor helper modules -- are needed by the | ||
| deployment stack (vLLM/SGLang) even when the model itself runs on native (non-remote) | ||
| transformers code. transformers 5.x restructured many VLM configs and no longer | ||
| re-saves these on ``save_pretrained`` for models loaded natively, so without copying | ||
| them a native-path export is missing e.g. ``preprocessor_config.json`` and fails to | ||
| load (``Can't load image processor``). These are copied regardless of | ||
| ``trust_remote_code``. Executable model/config code (``modeling*.py``, | ||
| ``configuration_*.py``, ``tokenization_*.py``, and other custom JSON) is only meaningful | ||
| with ``trust_remote_code`` and is copied only then. ``config.json`` and | ||
| ``model.safetensors.index.json`` are always skipped (handled by the export itself). | ||
| def copy_custom_model_files( | ||
| source_path: str, | ||
| export_path: str, | ||
| trust_remote_code: bool = False, | ||
| exclude_files: Iterable[str] | None = None, | ||
| ): | ||
| """Copy source checkpoint sidecar files to an HF PTQ export. | ||
|
|
||
| The HF PTQ script writes ModelOpt-owned metadata and quantized weights first, then | ||
| copies source checkpoint sidecars so tokenizer/processor files, remote-code modules, | ||
| README assets, parser plugins, and similar deployment files are preserved for both | ||
| native and ``trust_remote_code`` loads. Weight and weight-index files are skipped | ||
| to avoid copying the unquantized source weights. Export-owned metadata (``config.json``, | ||
| ``hf_quant_config.json``) and stale source quantization metadata are also skipped. | ||
| Source tokenizer and processor files intentionally still win because Transformers may | ||
| not regenerate all metadata in the source format. Callers that write a generation config | ||
| can exclude it; the TensorRT-LLM export retains the source generation config. | ||
|
|
||
| Args: | ||
| source_path: Path to the original model directory or HuggingFace model ID | ||
| export_path: Path to the exported model directory | ||
| trust_remote_code: Whether trust_remote_code was used (gates the executable code files) | ||
| trust_remote_code: Whether trust_remote_code was used when resolving HuggingFace model | ||
| IDs | ||
| exclude_files: Additional source file names to skip. | ||
|
Comment on lines
1040
to
+1044
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SUGGESTION] The "Whether Same for the sentence "Weight and weight-index files are skipped" — worth noting the skip is top-level only (the helper does not recurse), so a source with weights in a subdirectory is not covered. |
||
| """ | ||
| # Deployment-critical processor/tokenizer artifacts: safe to copy regardless of | ||
| # trust_remote_code (data + processor helpers, not model code). | ||
| always_copy_patterns = [ | ||
| "preprocessor_config.json", | ||
| "processor_config.json", | ||
| "image_processing*.py", | ||
| "processing_*.py", | ||
| "video_processing*.py", | ||
| "feature_extraction_*.py", | ||
| "added_tokens.json", | ||
| "special_tokens_map.json", | ||
| "vocab.json", | ||
| "merges.txt", | ||
| "tokenizer.model", | ||
| ] | ||
| # Executable custom model/config code + other custom JSON: only used with trust_remote_code. | ||
| code_patterns = [ | ||
| "configuration_*.py", | ||
| "modeling*.py", | ||
| "tokenization_*.py", | ||
| "*.json", | ||
| ] | ||
|
|
||
| # Resolve the source path (handles both local paths and HF model IDs) | ||
| resolved_source_path = _resolve_model_path(source_path, trust_remote_code) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The old docstring deliberately gated executable code ( |
||
|
|
||
|
|
@@ -997,29 +1027,19 @@ def copy_custom_model_files(source_path: str, export_path: str, trust_remote_cod | |
| print(f"Warning: Export directory {export_path} does not exist") | ||
| return | ||
|
|
||
| patterns = [*always_copy_patterns, *(code_patterns if trust_remote_code else [])] | ||
|
|
||
| copied_files: list[str] = [] | ||
| for pattern in patterns: | ||
| for file_path in source_dir.glob(pattern): | ||
| if file_path.is_file(): | ||
| # Skip config.json and model.safetensors.index.json as they're handled separately | ||
| if file_path.name in ["config.json", "model.safetensors.index.json"]: | ||
| continue | ||
| if file_path.name in copied_files: # e.g. matched by both pattern lists | ||
| continue | ||
| dest_path = export_dir / file_path.name | ||
| try: | ||
| shutil.copy2(file_path, dest_path) | ||
| copied_files.append(file_path.name) | ||
| print(f"Copied custom model file: {file_path.name}") | ||
| except Exception as e: | ||
| print(f"Warning: Failed to copy {file_path.name}: {e}") | ||
| copied_files = copy_non_safetensor_files_from_ckpt( | ||
| source_dir, | ||
| export_dir, | ||
| exclude_files=_HF_PTQ_EXPORT_OWNED_FILES | set(exclude_files or ()), | ||
| exclude_patterns=_HF_PTQ_WEIGHT_FILE_PATTERNS, | ||
| ) | ||
|
Comment on lines
+1070
to
+1075
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [IMPORTANT Compatibility] Delegating to the shared helper drops the per-file What changed. The old loop isolated failures per file: try:
shutil.copy2(file_path, dest_path)
copied_files.append(file_path.name)
print(f"Copied custom model file: {file_path.name}")
except Exception as e:
print(f"Warning: Failed to copy {file_path.name}: {e}")
Why it matters more now than before. Two effects compound:
Concrete triggers, all reachable in practice:
Suggested fix. Keep the shared helper as-is (the Megatron caller wants strictness) and restore per-file tolerance in this example wrapper, since a missing sidecar is not worth discarding a completed quantization run. Either wrap the call: try:
copied_files = copy_non_safetensor_files_from_ckpt(...)
except Exception as e:
print(f"Warning: Failed to copy checkpoint sidecar files: {e}")
copied_files = []or, better, add an opt-in |
||
|
|
||
| if copied_files: | ||
| print(f"Successfully copied {len(copied_files)} custom model files to {export_path}") | ||
| for file_name in copied_files: | ||
| print(f"Copied checkpoint sidecar file: {file_name}") | ||
| print(f"Successfully copied {len(copied_files)} checkpoint sidecar files to {export_path}") | ||
| else: | ||
| print("No custom model files found to copy") | ||
| print("No checkpoint sidecar files found to copy") | ||
|
|
||
|
|
||
| def _layerwise_checkpoint_dir_location(algorithm) -> tuple[str, str] | None: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -856,11 +856,12 @@ def export_quantized( | |
| print("This is normal for some VLM architectures that don't use AutoProcessor") | ||
|
|
||
| start_time = time.time() | ||
| if ( | ||
| is_tensorrt_llm_export = ( | ||
| model_type in ["t5", "bart", "whisper"] | ||
| or args.sparsity_fmt != "dense" | ||
| or "int8_sq" in args.qformat | ||
| ): | ||
| ) | ||
| if is_tensorrt_llm_export: | ||
| if ( | ||
| args.inference_tensor_parallel != 1 or args.inference_pipeline_parallel != 1 | ||
| ) and args.qformat == "nvfp4_svdquant": | ||
|
|
@@ -932,7 +933,13 @@ def export_quantized( | |
| # from the source checkpoint take precedence over regenerated ones (which may | ||
| # differ in format due to newer transformers versions). | ||
| if args.dist_state.is_main: | ||
| copy_custom_model_files(args.pyt_ckpt_path, export_path, args.trust_remote_code) | ||
| exclude_files = None if is_tensorrt_llm_export else {"generation_config.json"} | ||
| copy_custom_model_files( | ||
| args.pyt_ckpt_path, | ||
| export_path, | ||
| args.trust_remote_code, | ||
| exclude_files=exclude_files, | ||
| ) | ||
|
Comment on lines
+941
to
+947
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [IMPORTANT Export] The widened copy now lets the source Why this is new. Before this PR, Failure scenario (very common repo layout — Llama/Qwen/Mistral native, no
Transformers treats a template defined in both places as a conflict rather than a benign duplicate — please verify against the transformers version this example targets, since the outcome is either a hard Suggested fix. Make the exclusion decision reflect what the export actually wrote, rather than hardcoding a single filename. if args.dist_state.is_main:
exclude_files = set()
if not is_tensorrt_llm_export:
# export_hf_checkpoint() writes a sanitized generation_config.json
# (see _sanitize_generation_config_for_save); don't let the source clobber it.
exclude_files.add("generation_config.json")
if (Path(export_path) / "chat_template.jinja").exists():
# tokenizer.save_pretrained() relocated the chat template out of
# tokenizer_config.json; copying the source config back would restore a
# duplicate template in the legacy embedded format.
exclude_files.add("tokenizer_config.json")
copy_custom_model_files(
args.pyt_ckpt_path,
export_path,
args.trust_remote_code,
exclude_files=exclude_files or None,
)If you'd rather keep source-wins for |
||
|
|
||
| end_time = time.time() | ||
| print_rank_0( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,10 +15,12 @@ | |
|
|
||
| """Hugging Face checkpoint utility.""" | ||
|
|
||
| import fnmatch | ||
| import json | ||
| import os | ||
| import shutil | ||
| import warnings | ||
| from collections.abc import Iterable | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
|
|
@@ -253,25 +255,46 @@ def load_multimodal_components( | |
| return multimodal_state_dict | ||
|
|
||
|
|
||
| def copy_non_safetensor_files_from_ckpt(src: str | os.PathLike, dst: str | os.PathLike): | ||
| def _matches_any_pattern(file_name: str, patterns: tuple[str, ...]) -> bool: | ||
| return any(fnmatch.fnmatchcase(file_name, pattern) for pattern in patterns) | ||
|
|
||
|
|
||
| def copy_non_safetensor_files_from_ckpt( | ||
| src: str | os.PathLike, | ||
| dst: str | os.PathLike, | ||
| *, | ||
| exclude_files: Iterable[str] | None = None, | ||
| exclude_patterns: Iterable[str] | None = None, | ||
| ) -> list[str]: | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| """Copy every non-safetensors file from a local HF checkpoint dir verbatim. | ||
|
|
||
| Use as a baseline so tokenizer files, remote_code ``*.py``, README, LICENSE, etc. | ||
| are preserved from the source. The caller is expected to overwrite the files | ||
| modelopt owns (``config.json``, ``generation_config.json``, ``hf_quant_config.json``, | ||
| ``preprocessor_config.json``) after this step. | ||
| are preserved from the source. Callers can exclude additional files or patterns when | ||
|
Comment on lines
+265
to
+272
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SUGGESTION] The removed sentence documented a contract the Megatron caller still depends on. The old docstring said "The caller is expected to overwrite the files modelopt owns ( Both are valid, but a future caller reading this can't tell which one to follow. Suggest documenting both: The "top-level only" note is worth adding regardless — it's the pre-existing behavior but it's not stated anywhere, and it's load-bearing now that the exclusion patterns are the only thing keeping source weights out. |
||
| copying after export-owned metadata has already been written. | ||
|
|
||
| Args: | ||
| src: Source HF checkpoint directory. Must be a local path. | ||
| dst: Destination directory; created if missing. | ||
| exclude_files: Exact file names to skip. | ||
| exclude_patterns: Glob patterns for additional files to skip. | ||
|
|
||
| Returns: | ||
| File names copied into ``dst``. | ||
| """ | ||
| if not os.path.isdir(src): | ||
| raise ValueError(f"Invalid source path: {src}. It should be a directory.") | ||
| exclude_files = set(exclude_files or ()) | ||
| exclude_patterns = tuple(exclude_patterns or ()) | ||
| copied_files = [] | ||
| os.makedirs(dst, exist_ok=True) | ||
| for entry in os.listdir(src): | ||
| for entry in sorted(os.listdir(src)): | ||
| if entry in exclude_files or _matches_any_pattern(entry, exclude_patterns): | ||
| continue | ||
| sp = os.path.join(src, entry) | ||
| if not os.path.isfile(sp): | ||
| continue | ||
| if entry.endswith(".safetensors") or entry == "model.safetensors.index.json": | ||
| continue | ||
| shutil.copy2(sp, dst) | ||
|
jenchen13 marked this conversation as resolved.
Outdated
|
||
| copied_files.append(entry) | ||
|
jenchen13 marked this conversation as resolved.
|
||
| return copied_files | ||
Uh oh!
There was an error while loading. Please reload this page.