Skip to content

Fix ifbench dependency#925

Merged
Kipok merged 1 commit intomainfrom
igitman/ifbench-fix-2
Oct 9, 2025
Merged

Fix ifbench dependency#925
Kipok merged 1 commit intomainfrom
igitman/ifbench-fix-2

Conversation

@Kipok
Copy link
Collaborator

@Kipok Kipok commented Oct 9, 2025

Summary by CodeRabbit

  • Chores
    • Updated container setup to pre-download additional NLP resources, improving completeness of the runtime environment and offline readiness.
  • Bug Fixes
    • Reduced setup failures caused by missing language processing components during benchmarking and related tasks, leading to smoother, more reliable runs.

Signed-off-by: Igor Gitman <igitman@nvidia.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Adds an extra NLTK resource download (averaged_perceptron_tagger_eng) to the IFBench setup command in the Dockerfile for nemo-skills, extending the existing Python -c setup that installs punkt, punkt_tab, stopwords, and en_core_web_sm.

Changes

Cohort / File(s) Summary
Docker build setup (NLTK resources)
dockerfiles/Dockerfile.nemo-skills
Extends NLTK data bootstrap to include averaged_perceptron_tagger_eng; adjusts line continuation in the Python -c command within the IFBench setup step.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I packed my Docker burrow tight and snug,
A tagger tucked beside the punkt and plug.
With stopwords trimmed and spaCy spun,
I thump the build—another run!
Carrot-shaped logs hop by in green,
My image purrs: all deps are clean. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title indicates a fix to an IFBench dependency but is too generic to convey the specific change, which is adding the averaged_perceptron_tagger_eng NLTK model download in the Dockerfile setup step. Please update the title to clearly reference the new NLTK model download in the IFBench setup, for example “Add averaged_perceptron_tagger_eng NLTK download to IFBench Dockerfile.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch igitman/ifbench-fix-2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba7655 and b0e1582.

📒 Files selected for processing (1)
  • dockerfiles/Dockerfile.nemo-skills (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: unit-tests
🔇 Additional comments (1)
dockerfiles/Dockerfile.nemo-skills (1)

56-57: Thanks for covering the renamed POS‑tagger corpora.

The extra nltk.download('averaged_perceptron_tagger_eng') keeps the IFBench setup aligned with the newer language-specific NLTK resources and should prevent runtime lookups from failing. ✅


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kipok Kipok merged commit 66855db into main Oct 9, 2025
7 checks passed
@Kipok Kipok deleted the igitman/ifbench-fix-2 branch October 9, 2025 22:58
Glorf pushed a commit that referenced this pull request Oct 10, 2025
Signed-off-by: Igor Gitman <igitman@nvidia.com>
dgtm777 pushed a commit that referenced this pull request Oct 29, 2025
Signed-off-by: Igor Gitman <igitman@nvidia.com>
dgtm777 pushed a commit that referenced this pull request Mar 18, 2026
Signed-off-by: Igor Gitman <igitman@nvidia.com>
Signed-off-by: dgitman <dgitman@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant