Add INCLUDE multilingual MCQ benchmark#1483
Conversation
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
3e420d2 to
97f7522
Compare
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
Signed-off-by: naymaraq <dkaramyan@nvidia.com>
📝 WalkthroughWalkthroughAdds a new ChangesINCLUDE Multilingual MCQ Dataset Module
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@nemo_skills/dataset/include/include_benchmark_utils.py`:
- Around line 285-286: In the normalize_entry_field function, replace the use of
entry.get(key, "") with direct dictionary access entry[key]. This ensures that
if a schema field is missing from the dataset, the code will fail immediately
with a clear KeyError rather than silently substituting an empty string and
potentially corrupting data, which aligns with the coding guideline to not use
.get() for fields expected to be present.
- Line 186: The task string variable contains unescaped zero-width-space
characters (appearing as invisible spaces in the Dutch text "Hieronder staan
meerkeuzevragen") that cause syntax errors. Replace these zero-width-space
characters with their properly escaped Unicode representation using \u200b.
Locate the task string containing the Dutch template prompt and replace each
zero-width-space with the escaped form to ensure the string is properly
formatted and syntactically valid.
In `@nemo_skills/dataset/include/prepare.py`:
- Around line 50-55: In the collect_entries function, add the strict=True
parameter to the zip() call that pairs datasets and languages. Change
zip(datasets, languages) to zip(datasets, languages, strict=True) to ensure that
both lists have matching lengths and raise a clear error if they do not, rather
than silently ignoring mismatched elements.
🪄 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: 3b619224-aa69-414d-9e23-e9aa2bc5ecb6
📒 Files selected for processing (3)
nemo_skills/dataset/include/__init__.pynemo_skills/dataset/include/include_benchmark_utils.pynemo_skills/dataset/include/prepare.py
| ), | ||
| "Dutch": MCQFormat( | ||
| answer_prefix="Antwoord: Laten we stap voor stap nadenken.", | ||
| task='Hieronder staan meerkeuzevragen (met antwoorden) over {subject}. Denk stap voor stap en eindig uw antwoord met "{ans_suffix}", waarbij X de letter van het juiste antwoord is.', |
There was a problem hiding this comment.
Fix zero-width-space characters in Dutch template.
Line 186 contains unescaped zero-width-space characters that will cause a Python syntax error. These must be escaped as \u200b.
🔧 Proposed fix
- task='Hieronder staan meerkeuzevragen (met antwoorden) over {subject}. Denk stap voor stap en eindig uw antwoord met "{ans_suffix}", waarbij X de letter van het juiste antwoord is.',
+ task='Hieronder staan\u200b\u200bmeerkeuzevragen (met antwoorden) over {subject}. Denk stap voor stap en eindig uw antwoord met "{ans_suffix}", waarbij X de letter van het juiste antwoord is.',🧰 Tools
🪛 Ruff (0.15.15)
[error] 186-186: Invalid unescaped character zero-width-space, use "\u200B" instead
Replace with escape sequence
(PLE2515)
[error] 186-186: Invalid unescaped character zero-width-space, use "\u200B" instead
Replace with escape sequence
(PLE2515)
🤖 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 `@nemo_skills/dataset/include/include_benchmark_utils.py` at line 186, The task
string variable contains unescaped zero-width-space characters (appearing as
invisible spaces in the Dutch text "Hieronder staan meerkeuzevragen") that
cause syntax errors. Replace these zero-width-space characters with their
properly escaped Unicode representation using \u200b. Locate the task string
containing the Dutch template prompt and replace each zero-width-space with the
escaped form to ensure the string is properly formatted and syntactically valid.
| def normalize_entry_field(entry, key): | ||
| return (entry.get(key, "") or "").replace(" ", "_") |
There was a problem hiding this comment.
Use direct dictionary access instead of .get() for expected schema fields.
The function uses .get(key, "") with an empty string default for fields that are defined in the Schema and expected to be present in the dataset. Per the coding guidelines: "Don't use .get() for accessing dictionary keys if the code expects them to be present; use direct access data[key_name] to fail with a clear error instead of silently corrupting data."
If a schema field is missing from the dataset, the code should fail immediately with a clear KeyError rather than silently substituting an empty string, which could corrupt the prepared data.
🛡️ Proposed fix
def normalize_entry_field(entry, key):
- return (entry.get(key, "") or "").replace(" ", "_")
+ value = entry[key]
+ return (value or "").replace(" ", "_")As per coding guidelines: "Don't use .get() for accessing dictionary keys if the code expects them to be present; use direct access data[key_name] to fail with a clear error instead of silently corrupting data."
🤖 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 `@nemo_skills/dataset/include/include_benchmark_utils.py` around lines 285 -
286, In the normalize_entry_field function, replace the use of entry.get(key,
"") with direct dictionary access entry[key]. This ensures that if a schema
field is missing from the dataset, the code will fail immediately with a clear
KeyError rather than silently substituting an empty string and potentially
corrupting data, which aligns with the coding guideline to not use .get() for
fields expected to be present.
Source: Coding guidelines
| def collect_entries(languages, datasets): | ||
| entries = [] | ||
| for dataset, lang in zip(datasets, languages): | ||
| for entry in tqdm(dataset, desc=f"Preparing {lang} dataset"): | ||
| entries.append(format_entry(entry=entry, language=lang)) | ||
| return entries |
There was a problem hiding this comment.
Add strict=True to zip() for safer iteration.
The zip() call pairs datasets and languages without strict=True, which could silently ignore mismatched list lengths. Although the validation in main() should prevent this, adding strict=True provides an explicit safety check that will raise a clear error if the lists unexpectedly differ in length.
🔒 Proposed fix
def collect_entries(languages, datasets):
entries = []
- for dataset, lang in zip(datasets, languages):
+ for dataset, lang in zip(datasets, languages, strict=True):
for entry in tqdm(dataset, desc=f"Preparing {lang} dataset"):
entries.append(format_entry(entry=entry, language=lang))
return entries🧰 Tools
🪛 Ruff (0.15.15)
[warning] 52-52: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🤖 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 `@nemo_skills/dataset/include/prepare.py` around lines 50 - 55, In the
collect_entries function, add the strict=True parameter to the zip() call that
pairs datasets and languages. Change zip(datasets, languages) to zip(datasets,
languages, strict=True) to ensure that both lists have matching lengths and
raise a clear error if they do not, rather than silently ignoring mismatched
elements.
1 participant
Add the INCLUDE benchmark (CohereLabs/include-base-44) with prepare.py, include_benchmark_utils.py, and dataset config. Prompts are rendered in-language per sample (task instruction, options, and answer prefix), zero-shot, graded via the multichoice eval type.
Summary by CodeRabbit