fix(data): warn when behavioral dataset config keys are silently ignored#3271
Open
tianyi-zhang-02 wants to merge 1 commit into
Open
fix(data): warn when behavioral dataset config keys are silently ignored#3271tianyi-zhang-02 wants to merge 1 commit into
tianyi-zhang-02 wants to merge 1 commit into
Conversation
load_response_dataset instantiates with dataset_class(**data_config) and every built-in loader accepts **kwargs, so a standard ResponseDatasetConfig key the class does not support is swallowed with no feedback — e.g. subset: socratic on gsm8k silently loads the hardcoded 'main' config, and split_validation_size/seed silently do nothing on the 12 loaders that never call split_train_validation. Add a dispatcher-level guard that warns when a user-set behavioral key (subset, split, split_validation_size, seed) is not declared anywhere in the resolved class's __init__ MRO. MRO-aware so kwargs-forwarding subclasses (intent datasets) are not flagged; functools.partial registry entries (AIME variants) are unwrapped; None values and a falsy split_validation_size are skipped to keep documented config boilerplate quiet. Closes NVIDIA-NeMo#3270 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.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
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.
What does this PR do ?
load_response_datasetinstantiates withdataset_class(**data_config), and every built-in loader accepts**kwargs— so a standardResponseDatasetConfigkey the class doesn't support is silently swallowed. E.g.subset: socraticongsm8ksilently loads the hardcodedmainconfig, andsplit_validation_size/seedsilently do nothing on the 12 loaders that never callsplit_train_validation(while the SFT guide documents them as generic keys).This adds a dispatcher-level guard,
warn_on_unsupported_dataset_config_keys, that warns when a user-set behavioral key (subset,split,split_validation_size,seed) is not declared anywhere in the resolved class's__init__MRO.Design notes:
kwargs-forwarding subclasses that delegate to a declaring base (the intent datasets) are not flagged.functools.partialregistry entries (AIME variants) are unwrapped before inspection.Nonevalues and falsysplit_validation_sizeare skipped, so documented config boilerplate (subset: null,split_validation_size: 0) stays quiet — none of the shippedexamples/configsemit warnings.kwargs.get(...)without declaring it in any__init__signature would see a spurious warning. None of the built-in loaders do this (verified), and it's warning-only by design — but flagging it for review. Scoped to the response dispatcher; the preference dispatcher has the same pattern and can adopt the shared helper as a follow-up.Issues
Closes #3270. Generalizes the failure mode @RayenTian flagged in the #3131 review.
Usage
Before your PR is "Ready for review"
None/0quiet; MRO forwarding quiet;partialunwrapped; non-class input safe) plus an end-to-end dispatcher test, intests/unit/data/datasets/test_response_dataset_registry.py(registry-dispatch scope).ruff check/ruff format --checkclean;pyrefly==0.24.2(CI-pinned) reports no new errors on the two whitelisted files touched.cc @RayenTian @yuki-97