Pass the genome fasta to ngs-samples; run it and the analysis workflow as one throttled step

[jbrestel]

Part of VEuPathDB/VEuPathDatasets#34 (epic VEuPathDB/VEuPathDatasets#13).

Two changes to the ngs-samples path, kept on one branch so they can be tested together. They are independent and separable at 44d1a07:

1. Contamination-aware subsampling — get the reference onto the cluster and write the new params into the generated ngs-samples config (517fbd0, 640512e).
2. One step for ngs-samples + analysis, with per-assay throttles — so the fetch/analyse pair holds a single throttle slot (44d1a07, 35f1119).

Both touch the same six callers: snpAndCnvDNASeq, bulkrnaseq, longReadRnaSeq, processChipSeqExperiment, processOriginsExperiment, rnaSeqSplicedLeaderAndPolyASitesExperiment.

---

1. Contamination-aware subsampling

Config params

MakeNgsSamplesNextflowConfig now emits the param set from the contamination-aware subsampling design:

	Param	Value	Why
Removed	genomeSize	was DB lookup	Measured from referenceFasta now
Added	referenceFasta	genome symlink	Required; pipeline fails fast without it
Added	targetCoverage	60	Was hardcoded in calculateMaxReads()
Added	minOnTargetFraction	0.05	Fraction floor / 20x inflation cap
Added	minPlausibleFraction	0.01	Wrong-genome flag threshold
Added	pilotSize	100000	Pilot draw size

assayType is unchanged but is now validated against three values pipeline-side instead of silently defaulting.

The four gate thresholds are constants in the step class, not XML params. The design treats this as one gate with explicit thresholds rather than per-graph knobs, and six duplicated paramValues is precisely how a threshold becomes tribal knowledge. Easy to promote to params later if a graph genuinely needs to differ.

Getting the reference to the cluster

genomeFile is passed as $$genomeSymLink$$ by all six callers, digested through relativePathToNextflowClusterPath, and emitted as referenceFasta.

No new mirroring machinery was needed — every graph already had a symLinkGenome step placing the fasta at $$dataDir$$/$$organismAbbrev$$.fasta, and copyToCluster mirrors all of $$dataDir$$, the same ride the samplesheet takes via the final symlink.

The real gap was a missing dependency edge: ngsSamplesNextflowConfig depended only on makeResultDir, so the config could be written before the symlink existed. Five graphs got away with it on incidental ordering. Fixed here — all six now depends on symLinkGenome.

Side effect worth noting

Retiring genomeSize removes the getGenomeSize call, so this step no longer touches the database at all. gusConfigFile and organismAbbrev fed only that lookup and are dropped from the six callers. Nothing else in the step read them.

---

2. One step for ngs-samples and the analysis workflow

Why

runNextflowOnCluster.xml ran runNgsSamplesClusterTask and runClusterTask as two steps sharing the runNextflowOnCluster load type. Each released its throttle slot on completion, so once ngs-samples had fetched an experiment's reads, the analysis run had to re-queue behind other experiments' fetches. There was no way to express "N experiments in flight": the throttle counts running steps, and between the two steps an experiment held nothing.

Running both from one step makes the pair hold a single slot, which is what gives the throttle its back-pressure.

What

• new RunNgsSamplesAndNextflow — nextflowRuns() returns [ngs-samples, analysis]; run loops them calling the inherited runAndMonitor
• deletes RunAndMonitorNextflowNGSSamples, whose only job was renaming log files and no-opping on an empty config
• runNextflowOnCluster.xml: two steps collapsed into one, still named runClusterTask, so downstream depends and existing DB step names are preserved
• new Main/t/run_ngs_samples_and_nextflow.t

The 24 callers that pass no ngsSamplesNextflowConfigFile (interpro, busco, tmhmm, signalp, maskGenome, …) drop that run entirely and lose a step that did nothing at all.

Resumability

Each run keeps its own job info, log, trace and stdout file names, unchanged from before. That is load-bearing rather than cosmetic: a re-entered step detects an already-finished run from its own log via _checkClusterTaskLogForDone and proceeds to the next one. A single shared log would produce a step that could never resume past its first run.

Because the names and the cluster paths are both unchanged, in-flight cluster state stays resumable across this deploy.

---

3. Per-assay throttle buckets

runNextflowOnCluster.xml is shared by 30 callers, so the step it declares can only name one bucket for all of them. stepLoadTypes takes no variables — neither WorkflowStep.substituteValues():777 nor substituteMacros():815 touches the attribute, so $$foo$$ would survive parsing and then die in checkLoadTypes as an unknown type. So each call site tags the step with the path form instead, which is the mechanism intended for this; LoadTypeTest.testConditionalLoadTypes already covers the shape.

Caller	Bucket
snpAndCnvDNASeq.xml	dnaseqNextflow
bulkrnaseq.xml	rnaseqNextflow
longReadRnaSeq.xml	longReadRnaSeqNextflow
processChipSeqExperiment.xml	chipseqNextflow
processOriginsExperiment.xml	originsNextflow
rnaSeqSplicedLeaderAndPolyASitesExperiment.xml	splicedLeaderNextflow

⚠️ Action required by every data loader before this merges

loadThrottle.prop lives in each workflow's config, not in version control, and WorkflowStep.checkLoadTypes() fails the graph build on a load type it can't find there. A missing key is not "unlimited" — it is Unknown stepLoadType: dnaseqNextflow and the workflow will not start. All six keys must be added to every loadThrottle.prop, whether or not you want to throttle that assay:

dnaseqNextflow=3
rnaseqNextflow=3
longReadRnaSeqNextflow=2
chipseqNextflow=3
originsNextflow=3
splicedLeaderNextflow=3

Those values are a starting point, not a recommendation. No failThrottle.prop change is needed: these are stepLoadTypes only, and the step's existing stepFailTypes="runNextflowOnCluster" key is already in use today.

How to set the number

Each bucket counts concurrently running runClusterTask steps for that assay. Since the fetch and the analysis now share that step, one slot ≈ one experiment's full cluster footprint — SRA download, fastqs, BAMs — held from fetch through analysis. So the number answers "how many experiments of this assay do I want on the cluster at once?"

The sizing driver is scratch disk and transfer bandwidth, not CPU: nextflow's own maxForks already governs parallelism within a run. Pick from available scratch ÷ per-experiment footprint, start low, and raise it after watching a real load.

Load types AND together in RunnableWorkflow.okToRun(), so runClusterTask is bound by both its own runNextflowOnCluster bucket (shared by all 30 callers) and its per-assay bucket. Consequences:

• A per-assay value ≥ runNextflowOnCluster is a no-op; the tighter cap always wins.
• Per-assay values should roughly sum to runNextflowOnCluster, or you have just renamed the shared cap.
• runNextflowOnCluster remains the ceiling on total concurrent cluster nextflow runs, including the 24 annotation callers that add no per-assay tag.
• total in loadThrottle.prop is unrelated and still required — Workflow.java:423 already enforces it.
• To effectively disable one assay's bucket, set it equal to runNextflowOnCluster rather than deleting the key.

---

Merge order

1. ngs-samples-nextflow's contamination-aware-subsampling branch first. referenceFasta has not landed on main — the modules consume these params but nextflow.config still declares genomeSize. Merging this PR first would break ngs-samples runs rather than being inert, since referenceFasta is required and the pipeline fails fast without it.
2. Derive the nextflow branch key and URL from the workflow being run ReFlow#14 first, or together with this. runAndMonitor derived the <workflow>.branch shared-config key from the step's nextflowWorkflow param, which is only correct while a step runs one workflow. Without that PR, the ngs-samples run picks up the analysis workflow's branch.
3. Loaders add the six loadThrottle.prop keys before this reaches their workflow.

Testing

• XML validated. Step classes pass perl -c.
• MakeNgsSamplesNextflowConfig executed against stub params to confirm the rendered config; output verified by inspection.
• Main/t/run_ngs_samples_and_nextflow.t: 21/21 pass. Pins the on-cluster file names to today's exact values, that each run gets its own log and job-info file, and that an empty ngs-samples config yields exactly one run.
• Cluster paths confirmed byte-identical. The new step class uses the existing relativePathToNextflowClusterPath helper rather than the base class's inline digest math; both were compared against five real dnaseq/interpro path shapes and agree.
• ReFlow/StepClasses/t/run_and_monitor_nextflow.t: 2 pass, 1 fails — tailLooksOk "Retried but failed" expects '' and gets undef. Pre-existing; it fails identically with these changes stashed.
• Not run end-to-end against a cluster.

Open items, not addressed here

• The ngs-samples log still never comes back off the cluster. copyLogFromCluster pulls only nextflow.txt, and CopyNextflowResultFromCluster copies a single fileparse'd name (outputFiles is split on commas only in $test mode). Needs a second copy step or a change to that class.
• runNgsSamplesClusterTask rows will be orphaned in workflowstep for any workflow already past that step. Worth confirming how the controller treats a step present in the DB but absent from the graph before this runs against a live instance — it is the one failure mode nothing here tests.
• copyToCluster → deletFromCluster still sits outside the throttled step, so staged working dirs remain on disk beyond the cap. Much narrower than before the merge, but not zero: a stalled or offline experiment holds disk while releasing its slot.

[jbrestel]

Added 44d1a07 — run ngs-samples and the analysis nextflow from a single workflow step, so it can be tested together with the genome-fasta change on this branch.

Why

runNextflowOnCluster.xml ran runNgsSamplesClusterTask and runClusterTask as two steps sharing the runNextflowOnCluster load type. Each released its throttle slot on completion, so once ngs-samples had fetched an experiment's reads the analysis run had to re-queue behind other experiments' fetches. There was no way to express "N experiments in flight" — the throttle counts running steps, and between the two steps an experiment held nothing.

Running both from one step makes the fetch/analyse pair hold a single slot, which is what gives the throttle back-pressure.

What

• new RunNgsSamplesAndNextflow — nextflowRuns() returns [ngs-samples, analysis]; run loops them calling the inherited runAndMonitor
• deletes RunAndMonitorNextflowNGSSamples, whose only job was renaming log files and no-opping on an empty config
• runNextflowOnCluster.xml: two steps collapsed into one, still named runClusterTask so downstream depends and existing DB step names are preserved
• new Main/t/run_ngs_samples_and_nextflow.t

The 24 callers that pass no ngsSamplesNextflowConfigFile (interpro, busco, tmhmm, signalp, maskGenome, …) drop that run entirely and lose a step that did nothing. The 6 that do pass one — including the five XMLs already touched by this branch — get the merged behaviour.

Depends on

VEuPathDB/ReFlow#14. runAndMonitor derived the <workflow>.branch config key from the step's nextflowWorkflow param, so without that PR the ngs-samples run picks up the analysis workflow's branch.

Verified

• 21/21 new assertions pass. They pin the on-cluster file names (ngs-samples-nextflow.log, clusterJobInfo.txt, …) to today's exact values, that each run gets its own log and job-info file, and that an empty ngs config yields exactly one run.
• Cluster paths are byte-identical. The new class uses the existing relativePathToNextflowClusterPath helper rather than the base class's inline digest math; I compared both against 5 real dnaseq/interpro path shapes and they agree. With file names unchanged too, in-flight cluster state stays resumable across this deploy.

Two things left open

1. The ngs-samples log still never comes back off the cluster — copyLogFromCluster pulls only nextflow.txt, and CopyNextflowResultFromCluster copies a single fileparse'd name (outputFiles is split on commas only in $test mode). Needs a second copy step or a change to that class.
2. runNgsSamplesClusterTask rows will be orphaned in workflowstep for any workflow already past that step. Worth confirming how the controller treats a step present in the DB but absent from the graph before this hits a live instance.

🤖 Generated with Claude Code

[jbrestel]

Added 35f1119 — per-assay throttle buckets, plus how to set them.

⚠️ Action required by every data loader before this merges

loadThrottle.prop lives in each workflow's config, not in version control, and WorkflowStep.checkLoadTypes() fails the graph build on a load type it can't find there. A missing key is not "unlimited" — it's Unknown stepLoadType: dnaseqNextflow and the workflow won't start. So these six keys have to be added to every loadThrottle.prop, whether or not you want to throttle that assay:

dnaseqNextflow=3
rnaseqNextflow=3
longReadRnaSeqNextflow=2
chipseqNextflow=3
originsNextflow=3
splicedLeaderNextflow=3

Those numbers are a starting point, not a recommendation — see below. No failThrottle.prop change is needed: these are stepLoadTypes only, and the step's existing stepFailTypes="runNextflowOnCluster" key is already in use today.

What the number means

Each bucket counts concurrently running runClusterTask steps for that assay. Since 44d1a07 merged the ngs-samples fetch and the analysis run into that one step, one slot ≈ one experiment's full cluster footprint — SRA download, fastqs, BAMs — held from fetch through analysis. So the number answers "how many experiments of this assay do I want on the cluster at once?"

Sizing driver is scratch disk and transfer bandwidth, not CPU: nextflow's own maxForks already governs parallelism within a run. Pick from available scratch ÷ per-experiment footprint, start low, raise it once you've watched a real load.

How it composes

Load types AND together in RunnableWorkflow.okToRun(), so runClusterTask is now bound by both its own runNextflowOnCluster bucket (shared by all 30 callers of that subgraph) and its per-assay bucket. Consequences:

• A per-assay value ≥ runNextflowOnCluster is a no-op. The tighter cap always wins.
• Per-assay values should roughly sum to runNextflowOnCluster, or you've just renamed the shared cap.
• runNextflowOnCluster stays the ceiling on total concurrent cluster nextflow runs, including the 24 annotation callers (interpro, busco, tmhmm, …) that pass no ngs-samples config and add no per-assay tag.
• total in loadThrottle.prop is unrelated and still required — Workflow.java:423 already enforces it.

To effectively disable one assay's bucket, set it equal to runNextflowOnCluster rather than deleting the key.

Why call-site tags rather than a variable

runNextflowOnCluster.xml is shared by 30 callers, so the step can only name one bucket for everyone. stepLoadTypes takes no variables — neither WorkflowStep.substituteValues():777 nor substituteMacros():815 touches the attribute, so $$foo$$ would survive parsing and then die in checkLoadTypes as an unknown type. Tagging from the call site with the path form (stepLoadTypes="runClusterTask:dnaseqNextflow") is the mechanism intended for this, and LoadTypeTest.testConditionalLoadTypes already covers the shape.

Still not covered

copyToCluster → deletFromCluster sits outside the tagged step, so staged working dirs stay on disk beyond the cap. Much narrower than before the merge, but not zero — a stalled or offline experiment holds disk while releasing its slot.

🤖 Generated with Claude Code