[https://nvbugs/6215107][fix] Namespace cnn_dailymail repo id in load_calib_dataset

[Tabrizian]

Summary by CodeRabbit

• Bug Fixes
  • Improved dataset loading for CNN/DailyMail by handling the dataset name format expected by the latest Hugging Face Hub rules.
  • Prevents failures when loading the dataset using the short "cnn_dailymail" identifier.

Description

Newer huggingface_hub rejects the legacy bare cnn_dailymail dataset id and raises HfUriError ("Repository id must be 'namespace/name'") while resolving the dataset. load_calib_dataset() in tensorrt_llm/models/convert_utils.py still forwarded the bare id to datasets.load_dataset(), breaking the default calibration path of the TensorRT convert scripts.

This is the same root cause already fixed for the ModelOpt path in quantize_by_modelopt.py. This PR mirrors that fix by rewriting the bare id to the namespaced abisee/cnn_dailymail repo before loading.

Test Coverage

Existing quantization integration tests that exercise the default calibration path (e.g. SmoothQuant / INT8 KV-cache convert flows in tests/integration/defs) cover load_calib_dataset() with the default cnn_dailymail dataset.

PR Checklist

Please review the following before submitting your PR:

• PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

• PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

• Test cases are provided for new code paths (see test instructions)

• If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

• Any new dependencies have been scanned for license and vulnerabilities

• CODEOWNERS updated if ownership changes

• Documentation updated as needed

• Update tava architecture diagram if there is a significant design change in PR.

• The reviewers assigned automatically/manually are appropriate for the PR.

• Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

[Tabrizian]

/bot run --disable-fail-fast

[coderabbitai]

📝 Walkthrough

Walkthrough

The load_calib_dataset function now remaps the dataset identifier "cnn_dailymail" to "abisee/cnn_dailymail" before invoking load_dataset, addressing HuggingFace Hub's rejection of bare repo ids.

Changes

Dataset Identifier Normalization

Layer / File(s)	Summary
Remap cnn_dailymail dataset id tensorrt_llm/models/convert_utils.py	Adds a special-case check that maps dataset_name_or_dir == "cnn_dailymail" to "abisee/cnn_dailymail" before calling load_dataset.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the PR format and clearly summarizes the fix to the cnn_dailymail dataset id.
Description check	✅ Passed	The description explains the bug, the fix, test coverage, and includes the checklist.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

tensorrt_llm/models/convert_utils.py (1)

320-323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider centralizing the dataset-id remap instead of duplicating it per call site.

Per the PR objectives, this same "cnn_dailymail" → "abisee/cnn_dailymail" normalization was already applied separately in the ModelOpt quantization path. Hardcoding an identical special-case string comparison in two places risks drift if the canonical id changes again (e.g., another HF namespace migration). A small shared mapping/helper (e.g., a module-level _LEGACY_DATASET_ID_REMAP dict or a normalize_hf_dataset_id() utility) used by both call sites would keep this consistent going forward.

♻️ Suggested consolidation

+# Bare legacy dataset ids rejected by newer huggingface_hub (which requires
+# a "namespace/name" repo id); map them to their namespaced equivalents.
+_LEGACY_DATASET_ID_REMAP = {
+    "cnn_dailymail": "abisee/cnn_dailymail",
+}
+
+
 def load_calib_dataset(dataset_name_or_dir: str,
                        config_name: Optional[str] = None,
                        split: Optional[str] = None,
                        key: Optional[str] = None,
                        trust_remote_code=True,
                        **kwargs):
     if config_name is None:
         for name, meta in DEFAULT_HF_DATASET_META.items():
             if name in dataset_name_or_dir:
                 if config_name is None:
                     config_name = meta[0]
                 if split is None:
                     split = meta[1]
                 if key is None:
                     key = meta[2]
                 break

-    # Bare "cnn_dailymail" id is rejected by newer huggingface_hub, which
-    # requires a "namespace/name" repo id; use the namespaced repo instead.
-    if dataset_name_or_dir == "cnn_dailymail":
-        dataset_name_or_dir = "abisee/cnn_dailymail"
+    dataset_name_or_dir = _LEGACY_DATASET_ID_REMAP.get(
+        dataset_name_or_dir, dataset_name_or_dir)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/models/convert_utils.py` around lines 320 - 323, Centralize the
legacy dataset-id normalization instead of keeping the hardcoded "cnn_dailymail"
to "abisee/cnn_dailymail" check inside convert_utils.py. Move this remap into a
shared helper or module-level mapping (for example, a normalize_hf_dataset_id
utility) and have both the convert_utils path and the ModelOpt quantization path
call it so the canonical Hugging Face dataset id is maintained in one place. Use
the existing dataset_name_or_dir handling in convert_utils as the integration
point and update the other call site to reuse the same normalization logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tensorrt_llm/models/convert_utils.py`:
- Around line 320-323: Centralize the legacy dataset-id normalization instead of
keeping the hardcoded "cnn_dailymail" to "abisee/cnn_dailymail" check inside
convert_utils.py. Move this remap into a shared helper or module-level mapping
(for example, a normalize_hf_dataset_id utility) and have both the convert_utils
path and the ModelOpt quantization path call it so the canonical Hugging Face
dataset id is maintained in one place. Use the existing dataset_name_or_dir
handling in convert_utils as the integration point and update the other call
site to reuse the same normalization logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fbdf47d1-2afc-4651-b967-49f928dde853

📥 Commits

Reviewing files that changed from the base of the PR and between 06df8da and 85881a7.

📒 Files selected for processing (1)

• tensorrt_llm/models/convert_utils.py

[tensorrt-cicd]

PR_Github #58261 [ ] completed with state FAILURE. Commit: 85881a7
Not allowed on merged PR

Link to invocation