Skip to content

Added evals to genomics-workflow-acceleration and parabricks library …#8

Open
delagoya wants to merge 1 commit into
mainfrom
feat/genomics-skills-evals
Open

Added evals to genomics-workflow-acceleration and parabricks library …#8
delagoya wants to merge 1 commit into
mainfrom
feat/genomics-skills-evals

Conversation

@delagoya

@delagoya delagoya commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

…skills. Some other small changes.

@trvachov

Copy link
Copy Markdown
Contributor

I reviewed this against current main. There are two distinct buckets: issues that stand on their own in PR #8, and integration work caused by changes that landed after this branch was created.

PR #8 issues that stand on their own

  1. Existing-BAM routing includes the wrong entry point.

    • library-skills/parabricks/SKILL.md:104-107
    • library-skills/parabricks/references/tool-index.md:76-79

    These lines include germline for variant recall from an existing BAM. The same skill correctly describes germline as the FASTQ-to-variants pipeline and routes BAM/CRAM inputs to haplotypecaller or deepvariant. Parabricks 4.7.1 likewise documents germline with --in-fq, not --in-bam. Please remove germline from the existing-BAM guidance or explicitly version-qualify it.

  2. A trigger label contradicts both the skill and its functional eval.

    • library-skills/genomics-workflow-acceleration/evals/trigger_evals.json:19-20 marks “Make my genomics pipeline faster and use GPUs” as should_trigger: false.
    • SKILL.md:75-88 says this request is in scope and, when no path is provided, the selected skill should ask for one.
    • Functional eval 14 in evals/evals.json expects that exact behavior.

    The trigger label should therefore be true.

  3. Fixture-backed evals do not give the baseline the fixture inputs.

    All 15 genomics cases have files: [], while 11 prompts reference paths under skills/genomics-workflow-acceleration/evals/fixtures/. The with-skill arm receives the staged skill tree, but the without-skill baseline does not, so the two arms do not receive identical task inputs. The current NVIDIA SkillEvaluator also reports evals/fixtures as unrecognized and ignored. Please place the inputs under the recognized eval input layout, declare them in the relevant cases' files, and have prompts reference the staged input path.

  4. Two workflow fixtures are structurally invalid.

    • sample-wdl/pipeline.wdl:13 contains invalid call BwaMem { input: ... } syntax; miniwdl check rejects it.
    • sample-nextflow/main.nf:21 emits (sample_id, r1, r2), while modules/local/bwa_mem/main.nf:11 declares (meta, reads) and then dereferences meta.id. This loses the paired-read shape and produces a tuple mismatch.

Integration work required because main moved

  1. Rebase and intentionally merge the eval datasets. PR evals: add nv-aces eval datasets across all 32 skills #9 added evals.json for both skills, so this PR is now conflicting. Please union/deduplicate the cases rather than taking either side wholesale. In particular, retain main's Parabricks runtime-check cases, which are not covered by this PR's routing-focused cases.

  2. Use unique string case IDs after resolving the conflicts. All 35 cases here use numeric IDs. Current main deliberately standardized IDs as strings because integer IDs crashed the repository's ACES HTML reporting path.

  3. Regenerate the plugin payload after rebasing. Current main's plugin-sync check reports stale copies for the two modified SKILL.md files and the two modified reference files. Run:

    python scripts/plugin_sync.py --write
  4. Add the required DCO signoff. The single commit currently has no Signed-off-by trailer, while CONTRIBUTING.md requires one.

I am not treating the .skillsource.json deletions as a blocker: current plugin synchronization explicitly ignores those files. I also am not attributing the evaluator's general lack of trigger_evals.json support to this PR, because the same warning occurs for pre-existing NIM skills.

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.

2 participants