Add 7 nvidia/Nemotron-* calibration datasets to SUPPORTED_DATASET_CONFIG - #1498
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a helper to join chat message contents, registers multiple Nemotron dataset variants in SUPPORTED_DATASET_CONFIG using that preprocess and chat_key="messages", and adds parametrized tests that validate the new registry entries and optionally fetch two samples when a Hugging Face token is available. ChangesNemotron Dataset Registration and Testing
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
cjluo-nv
reviewed
May 15, 2026
cjluo-nv
left a comment
cjluo-nv
left a comment
Collaborator
There was a problem hiding this comment.
Bot review — DM the bot to share feedback.
Small, focused registry addition that follows the existing pattern for nemotron-post-training-dataset-v{1,2} exactly — same messages chat key, same \n.join preprocess, same shape. No correctness issues spotted; the inline comments explaining which splits are excluded due to heterogeneous parquet schemas are helpful.
The reason for nudging rather than approving: the only test added is gated on HF_TOKEN being present in the environment, and these nvidia/Nemotron-* datasets are gated on HF Hub. If CI runners don't have HF_TOKEN set, the test will silently skip and the registry entries (split names, dataset paths) won't actually be exercised before merge — a typo in any of the 30+ split names wouldn't be caught. Worth a human confirming whether the CI environment has HF_TOKEN configured for these tests, or whether a structural-only test (asserting each entry has expected keys / a callable preprocess) would be a useful complement.
Minor nit (non-blocking): the preprocess lambda lambda sample: "\n".join(turn["content"] for turn in sample["messages"]) is now duplicated across 9 entries. Could be hoisted to a module-level helper, but matches existing style in this file.
Contributor
Author
|
/claude review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/torch/utils/test_dataset_utils.py (1)
695-723:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark this smoke test as integration-scoped.
This test hits live gated HF datasets; without an integration marker it can introduce flaky/slower unit runs whenever
HF_TOKENis present.Suggested fix
+@pytest.mark.integration `@pytest.mark.parametrize`( "dataset_key", [ "nemotron-sft-instruction-following-chat-v2", "nemotron-science-v1", "nemotron-competitive-programming-v1", "nemotron-sft-agentic-v2", "nemotron-math-v2", "nemotron-sft-swe-v2", "nemotron-sft-multilingual-v1", ], ) def test_get_dataset_samples_new_nemotron(dataset_key):As per coding guidelines:
tests/**/*.py: tests intests/unitare expected to stay fast CPU-based.🤖 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 `@tests/unit/torch/utils/test_dataset_utils.py` around lines 695 - 723, The smoke test test_get_dataset_samples_new_nemotron currently runs as a unit test but hits gated HF Hub datasets; mark it as integration-scoped by adding the pytest marker (e.g., `@pytest.mark.integration`) to the test_get_dataset_samples_new_nemotron function so CI and local test runners can opt out; locate the test function definition and apply the marker decorator (or include the marker in the existing parametrize decorators) to ensure it is treated as an integration test.
🤖 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.
Outside diff comments:
In `@tests/unit/torch/utils/test_dataset_utils.py`:
- Around line 695-723: The smoke test test_get_dataset_samples_new_nemotron
currently runs as a unit test but hits gated HF Hub datasets; mark it as
integration-scoped by adding the pytest marker (e.g., `@pytest.mark.integration`)
to the test_get_dataset_samples_new_nemotron function so CI and local test
runners can opt out; locate the test function definition and apply the marker
decorator (or include the marker in the existing parametrize decorators) to
ensure it is treated as an integration test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 18285ab5-cf8a-4ae4-a407-570652ad2b0b
📒 Files selected for processing (2)
modelopt/torch/utils/dataset_utils.pytests/unit/torch/utils/test_dataset_utils.py
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude review passed — no significant issues found. LGTM
Additive registration of 7 nvidia/Nemotron-* dataset configs that follow the existing nemotron-post-training-dataset-v1/v2 pattern exactly (same messages chat_key, same content-join preprocess lambda). The split exclusions are well-documented with inline comments explaining the streaming schema-cast failures.
The smoke test correctly gates on HF_TOKEN since these are gated repos, matching the network-test convention already established by test_get_dataset_samples_with_unsupported_minipile_dataset. No algorithm correctness, mode/state composability, export, or backward-compat concerns apply to a config-only addition.
Collaborator
There was a problem hiding this comment.
Have you tried directly trying these datasets (e.g. get_dataset_dataloader("nvidia/Nemotron-SFT-Instruction-Following-Chat-v2", ...)) without adding to this dictionary? We have a fallback logic that auto-detects dataset type and parses it if not present in the dictionary
Contributor
Author
There was a problem hiding this comment.
Tried it on main for nvidia/Nemotron-SFT-Instruction-Following-Chat-v2 — the fallback hits three issues:
get_dataset_samples(path, num_samples=2) → ValueError: Bad split: train. Available splits: ['reasoning_off', 'reasoning_on'].
Auto-detect defaults tosplit=["train"] (dataset_utils.py:415);none of the 7 Nemotron datasets has atrainsplit.- With the correct split but no tokenizer →
ValueError: Dataset '…' has a 'messages' column but no tokenizer with apply_chat_template was provided (dataset_utils.py:256-260). - With a tokenizer but
apply_chat_template=False→ the fallback still callsapply_chat_templateunconditionally, which fails on tokenizers without a chat template (e.g. gpt2).
We can think about more of improving the fallback pass.
As a background, we are evaluating if these dataset should be the default calibration dataset instead. So adding them in this PR
| """ | ||
| pytest.importorskip("datasets") | ||
| pytest.importorskip("huggingface_hub") | ||
| if not os.environ.get("HF_TOKEN"): |
Collaborator
There was a problem hiding this comment.
@claude is this correct check? Does this als work if hf auth login is done instead of setting HF_TOKEN Env var?
Contributor
Author
There was a problem hiding this comment.
Does this als work if hf auth login is done instead of setting HF_TOKEN Env var?
I did a test locally and it worked
Collaborator
|
Commits are not signed hence CI wont run. Please take a look at https://github.com/NVIDIA/TensorRT-Model-Optimizer/blob/main/CONTRIBUTING.md#%EF%B8%8F-signing-your-work |
Fridah-nv
force-pushed
the
fridah/nemotron-datasets
branch
from
May 15, 2026 21:34
54f7ab7 to
6b65412
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1498 +/- ##
==========================================
- Coverage 77.43% 76.82% -0.62%
==========================================
Files 473 473
Lines 51432 51532 +100
==========================================
- Hits 39824 39587 -237
- Misses 11608 11945 +337
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fridah-nv
force-pushed
the
fridah/nemotron-datasets
branch
from
May 15, 2026 21:58
6b65412 to
068ef00
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/unit/torch/utils/test_dataset_utils.py`:
- Line 20: The module-level import "from huggingface_hub import get_token"
causes test collection failures if huggingface_hub is not installed; move this
import into the test function test_get_dataset_samples_new_nemotron() (or
replace it with pytest.importorskip("huggingface_hub") at the top of that test)
so the optional dependency is only required when that test runs and unrelated
tests can still be collected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6b82ec4d-298b-4da0-98f0-524adfcd8770
📒 Files selected for processing (2)
modelopt/torch/utils/dataset_utils.pytests/unit/torch/utils/test_dataset_utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- modelopt/torch/utils/dataset_utils.py
|
|
||
| import pytest | ||
| import torch | ||
| from huggingface_hub import get_token |
Contributor
There was a problem hiding this comment.
Avoid module-level import of optional test dependency.
Importing huggingface_hub at file import time can fail test collection in environments missing that package. Move get_token import inside test_get_dataset_samples_new_nemotron() (or use pytest.importorskip("huggingface_hub")) so unrelated unit tests still collect/run.
🤖 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 `@tests/unit/torch/utils/test_dataset_utils.py` at line 20, The module-level
import "from huggingface_hub import get_token" causes test collection failures
if huggingface_hub is not installed; move this import into the test function
test_get_dataset_samples_new_nemotron() (or replace it with
pytest.importorskip("huggingface_hub") at the top of that test) so the optional
dependency is only required when that test runs and unrelated tests can still be
collected.
Fridah-nv
force-pushed
the
fridah/nemotron-datasets
branch
from
May 15, 2026 22:11
068ef00 to
9bbcda2
Compare
Registers nemotron-{sft-instruction-following-chat-v2, science-v1,
competitive-programming-v1, sft-agentic-v2, math-v2, sft-swe-v2,
sft-multilingual-v1} so hf_ptq.py's --dataset flag (which enumerates
get_supported_datasets() automatically) can select these for PTQ
calibration. Splits with heterogeneous parquet schemas that crash
streaming CastError mid-iteration are excluded per inline comments.
Adds a parametrized smoke test that skips when HF_TOKEN is unset since
the nvidia/Nemotron-* datasets are gated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Fridah-nv <201670829+Fridah-nv@users.noreply.github.com>
Signed-off-by: Frida Hou <201670829+Fridah-nv@users.noreply.github.com>
Signed-off-by: Fridah-nv <201670829+Fridah-nv@users.noreply.github.com> Signed-off-by: Frida Hou <201670829+Fridah-nv@users.noreply.github.com>
Fridah-nv
force-pushed
the
fridah/nemotron-datasets
branch
from
May 15, 2026 22:13
9bbcda2 to
f9dd1bc
Compare
cjluo-nv
approved these changes
May 16, 2026
7 tasks
kevalmorabia97
pushed a commit
that referenced
this pull request
May 19, 2026
## Summary - Add ``DATASET_COMBOS`` to ``modelopt.torch.utils.dataset_utils`` — single ``--dataset`` tokens that fan out to several entries in ``SUPPORTED_DATASET_CONFIG``. The per-entry ``num_samples`` is split evenly across the members inside ``get_dataset_dataloader``. - Two initial combos: - ``cnn_nemotron_v2_mix`` → ``cnn_dailymail`` + ``nemotron-post-training-dataset-v2``. Replaces the hardcoded two-element fallback list in ``hf_ptq.py`` when ``--dataset`` is omitted. - ``nemotron-post-training-v3`` → the seven ``nvidia/Nemotron-*`` SFT datasets registered in #1498 (mirroring the upstream [`nemotron-post-training-v3` collection](https://huggingface.co/collections/nvidia/nemotron-post-training-v3)). - ``get_supported_datasets()`` now appends combo names so they show up in ``--dataset`` help. - ``hf_ptq.py``'s default ``--calib_size`` bumped from ``512`` to ``1024`` so the ``cnn_nemotron_v2_mix`` combo's even split preserves the previous total sample count (was 512 per-dataset × 2 datasets = 1024; now 1024 split → 512 per-dataset × 2). ``--calib_size`` now denotes the total calibration budget regardless of combo cardinality. - Reject mixing a combo with one of its member datasets in the same ``--dataset`` list (e.g. ``cnn_dailymail,cnn_nemotron_v2_mix``) — combo would otherwise double-sample the explicit member with a smaller per-member quota. - Reject combo names in ``get_dataset_samples``; combos are dataloader-only. The error message points callers to ``get_dataset_dataloader``. - Validate ``DATASET_COMBOS`` at import time: empty member lists, name collisions with ``SUPPORTED_DATASET_CONFIG``, and references to unknown datasets raise ``ValueError`` up front. ## Test plan End-to-end validated against ``/hf-local/Qwen/Qwen3.5-0.8B`` via ``get_dataset_dataloader`` on the actual streamed data, plus 5 new unit tests in ``TestDatasetCombosExpansion`` (all 44 tests in ``test_dataset_utils.py`` pass with no regressions). - [x] ``python -c "from modelopt.torch.utils.dataset_utils import DATASET_COMBOS, get_supported_datasets; assert 'cnn_nemotron_v2_mix' in get_supported_datasets() and 'nemotron-post-training-v3' in get_supported_datasets()"`` - [x] ``hf_ptq.py`` with no ``--dataset`` flag still calibrates on cnn_dailymail + nemotron-post-training-dataset-v2 with the same total sample count as before. - [x] ``--dataset nemotron-post-training-v3 --calib_size 1024`` allocates 146 per member across the seven Nemotron datasets; full 1022-sample dataloader builds without error. - [x] ``--dataset cnn_dailymail,nemotron-post-training-v3 --calib_size 256,1024`` composes correctly: 256 from cnn_dailymail (as a plain entry) plus the 7-way split from the combo. (The earlier ``cnn_dailymail,cnn_nemotron_v2_mix`` example is rejected by design since ``cnn_dailymail`` is a member of that combo.) - [x] ``--dataset cnn_dailymail,cnn_nemotron_v2_mix`` raises ``ValueError`` with a clear message. - [x] ``get_dataset_samples("cnn_nemotron_v2_mix", ...)`` raises ``ValueError`` pointing to ``get_dataset_dataloader``. - [x] Unit tests: ``pytest tests/unit/torch/utils/test_dataset_utils.py`` — 44 passed. ## Post-validation fix End-to-end testing surfaced that the original ``nemotron-sft-agentic-v2`` entry kept the two splits (``interactive_agent``, ``tool_calling``) that pyarrow's streaming JSON reader cannot parse, and excluded ``search`` which is the only clean split. Failures reproduce deterministically across cache wipes with ``force_redownload``, so they are content-level defects in the published JSONL files at the pinned revision, not local artifacts: - ``interactive_agent`` — heterogeneous schema (``Column(.../member_id/type) changed from string to array``) at JSONL row 4. - ``tool_calling`` — malformed JSON row in a later shard, fails at sample ~885 with ``Missing a closing quotation mark in string``. - ``search`` — streams cleanly (verified to 2500 samples). Commit ``10f3cfd`` corrects ``nemotron-sft-agentic-v2`` to use only ``search``, with an updated comment. The CHANGELOG calls this out as a separate bullet so the behavior change on a previously-released dataset entry from #1498 is discoverable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Dataset combo support: a single dataset token can expand into multiple registered datasets with even sample splitting; predefined combos added (e.g., cnn_nemotron_v2_mix, nemotron-post-training-v3) and listed as supported. * **Updates** * Default dataset when none specified now uses cnn_nemotron_v2_mix. * Calibration size default increased from 512 to 1024. * **Bug Fixes** * nemotron-sft-agentic-v2 now uses only the deterministic "search" split to avoid streaming JSON errors. * **Tests** * Added coverage for combo expansion, splitting, overlap validation, and rejection behavior. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/Model-Optimizer/pull/1508?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chenjie Luo <chenjiel@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jenchen13
pushed a commit
that referenced
this pull request
May 27, 2026
](https://app.coderabbit.ai/change-stack/NVIDIA/Model-Optimizer/pull/1508?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
…FIG (#1498) Registers nemotron-{sft-instruction-following-chat-v2, science-v1, competitive-programming-v1, sft-agentic-v2, math-v2, sft-swe-v2, sft-multilingual-v1} so hf_ptq.py's --dataset flag (which enumerates get_supported_datasets() automatically) can select these for PTQ calibration. Splits with heterogeneous parquet schemas that crash streaming CastError mid-iteration are excluded per inline comments. Adds a parametrized smoke test that skips when HF_TOKEN is unset since the nvidia/Nemotron-* datasets are gated. ### What does this PR do? Type of change: ? <!-- Use one of the following: Bug fix, new feature, new example, new tests, documentation. --> <!-- Details about the change. --> ### Usage ```python # Add a code snippet demonstrating how to use this ``` ### Testing <!-- Mention how have you tested your change if applicable. --> ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: ✅ / ❌ / N/A <!--- If ❌, explain why. --> - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: ✅ / ❌ / N/A <!--- Mandatory --> - Did you write any new necessary tests?: ✅ / ❌ / N/A <!--- Mandatory for new features or examples. --> - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ / ❌ / N/A <!--- Only for new features, API changes, critical bug fixes or backward incompatible changes. --> - Did you get Claude approval on this PR?: ✅ / ❌ / N/A <!--- Run `/claude review`. NVIDIA org members can self-trigger for complex changes; orthogonal to CodeRabbit. --> ### Additional Information <!-- E.g. related issue. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Standardized chat-style preprocessing (joining message contents) and normalized chat key to "messages" across seven Nemotron dataset variants, with explicit dataset paths and curated splits. * **Tests** * Added registry-shape unit checks for each new dataset entry. * Added a gated integration smoke test that fetches sample data when a Hugging Face Hub token is present. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/Model-Optimizer/pull/1498?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Fridah-nv <201670829+Fridah-nv@users.noreply.github.com> Signed-off-by: Frida Hou <201670829+Fridah-nv@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/NVIDIA/Model-Optimizer/pull/1498?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
jenchen13
pushed a commit
that referenced
this pull request
May 27, 2026
## Summary - Add ``DATASET_COMBOS`` to ``modelopt.torch.utils.dataset_utils`` — single ``--dataset`` tokens that fan out to several entries in ``SUPPORTED_DATASET_CONFIG``. The per-entry ``num_samples`` is split evenly across the members inside ``get_dataset_dataloader``. - Two initial combos: - ``cnn_nemotron_v2_mix`` → ``cnn_dailymail`` + ``nemotron-post-training-dataset-v2``. Replaces the hardcoded two-element fallback list in ``hf_ptq.py`` when ``--dataset`` is omitted. - ``nemotron-post-training-v3`` → the seven ``nvidia/Nemotron-*`` SFT datasets registered in #1498 (mirroring the upstream [`nemotron-post-training-v3` collection](https://huggingface.co/collections/nvidia/nemotron-post-training-v3)). - ``get_supported_datasets()`` now appends combo names so they show up in ``--dataset`` help. - ``hf_ptq.py``'s default ``--calib_size`` bumped from ``512`` to ``1024`` so the ``cnn_nemotron_v2_mix`` combo's even split preserves the previous total sample count (was 512 per-dataset × 2 datasets = 1024; now 1024 split → 512 per-dataset × 2). ``--calib_size`` now denotes the total calibration budget regardless of combo cardinality. - Reject mixing a combo with one of its member datasets in the same ``--dataset`` list (e.g. ``cnn_dailymail,cnn_nemotron_v2_mix``) — combo would otherwise double-sample the explicit member with a smaller per-member quota. - Reject combo names in ``get_dataset_samples``; combos are dataloader-only. The error message points callers to ``get_dataset_dataloader``. - Validate ``DATASET_COMBOS`` at import time: empty member lists, name collisions with ``SUPPORTED_DATASET_CONFIG``, and references to unknown datasets raise ``ValueError`` up front. ## Test plan End-to-end validated against ``/hf-local/Qwen/Qwen3.5-0.8B`` via ``get_dataset_dataloader`` on the actual streamed data, plus 5 new unit tests in ``TestDatasetCombosExpansion`` (all 44 tests in ``test_dataset_utils.py`` pass with no regressions). - [x] ``python -c "from modelopt.torch.utils.dataset_utils import DATASET_COMBOS, get_supported_datasets; assert 'cnn_nemotron_v2_mix' in get_supported_datasets() and 'nemotron-post-training-v3' in get_supported_datasets()"`` - [x] ``hf_ptq.py`` with no ``--dataset`` flag still calibrates on cnn_dailymail + nemotron-post-training-dataset-v2 with the same total sample count as before. - [x] ``--dataset nemotron-post-training-v3 --calib_size 1024`` allocates 146 per member across the seven Nemotron datasets; full 1022-sample dataloader builds without error. - [x] ``--dataset cnn_dailymail,nemotron-post-training-v3 --calib_size 256,1024`` composes correctly: 256 from cnn_dailymail (as a plain entry) plus the 7-way split from the combo. (The earlier ``cnn_dailymail,cnn_nemotron_v2_mix`` example is rejected by design since ``cnn_dailymail`` is a member of that combo.) - [x] ``--dataset cnn_dailymail,cnn_nemotron_v2_mix`` raises ``ValueError`` with a clear message. - [x] ``get_dataset_samples("cnn_nemotron_v2_mix", ...)`` raises ``ValueError`` pointing to ``get_dataset_dataloader``. - [x] Unit tests: ``pytest tests/unit/torch/utils/test_dataset_utils.py`` — 44 passed. ## Post-validation fix End-to-end testing surfaced that the original ``nemotron-sft-agentic-v2`` entry kept the two splits (``interactive_agent``, ``tool_calling``) that pyarrow's streaming JSON reader cannot parse, and excluded ``search`` which is the only clean split. Failures reproduce deterministically across cache wipes with ``force_redownload``, so they are content-level defects in the published JSONL files at the pinned revision, not local artifacts: - ``interactive_agent`` — heterogeneous schema (``Column(.../member_id/type) changed from string to array``) at JSONL row 4. - ``tool_calling`` — malformed JSON row in a later shard, fails at sample ~885 with ``Missing a closing quotation mark in string``. - ``search`` — streams cleanly (verified to 2500 samples). Commit ``10f3cfd`` corrects ``nemotron-sft-agentic-v2`` to use only ``search``, with an updated comment. The CHANGELOG calls this out as a separate bullet so the behavior change on a previously-released dataset entry from #1498 is discoverable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Dataset combo support: a single dataset token can expand into multiple registered datasets with even sample splitting; predefined combos added (e.g., cnn_nemotron_v2_mix, nemotron-post-training-v3) and listed as supported. * **Updates** * Default dataset when none specified now uses cnn_nemotron_v2_mix. * Calibration size default increased from 512 to 1024. * **Bug Fixes** * nemotron-sft-agentic-v2 now uses only the deterministic "search" split to avoid streaming JSON errors. * **Tests** * Added coverage for combo expansion, splitting, overlap validation, and rejection behavior. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/Model-Optimizer/pull/1508?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chenjie Luo <chenjiel@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Registers nemotron-{sft-instruction-following-chat-v2, science-v1, competitive-programming-v1, sft-agentic-v2, math-v2, sft-swe-v2, sft-multilingual-v1} so hf_ptq.py's --dataset flag (which enumerates get_supported_datasets() automatically) can select these for PTQ calibration. Splits with heterogeneous parquet schemas that crash streaming CastError mid-iteration are excluded per inline comments.
Adds a parametrized smoke test that skips when HF_TOKEN is unset since the nvidia/Nemotron-* datasets are gated.
What does this PR do?
Type of change: ?
Usage
# Add a code snippet demonstrating how to use thisTesting
Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: ✅ / ❌ / N/AAdditional Information
Summary by CodeRabbit