diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 0000000..4693eb5 --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,37 @@ +# CLAUDE.md + +## External-Interface Change Sign-Off + +This plugin's *external interface* is: cvalues external project configs can +set/override, `` names, environment-script names, documented +file-naming conventions, and public Groovy/Java API (classes/methods other +plugins could extend or call). + +Before closing any task or change in this plugin, check whether it touches +the external interface. If yes: + +1. Add/update a row in `docs/developers/ExternalInterfaceChangeLog.adoc` + describing the change and its risk if unconfirmed. +2. Get explicit user sign-off before merging that change — do not silently + assume a breaking change is acceptable, even if it looks like dead or + already-broken code. + +Keep that table current for all changes, or whenever you notice it's +drifted from the ground truth (the code) — not just when a task happens to +touch it. + +This is a standing, cross-cutting process rule, not a one-shot task — apply +it for the lifetime of this plugin's development, not just the current +branch's task batch. + +### Version numbers in change documentation + +Never assert the version/tag a not-yet-released change will ship under +(e.g. "new in 1.2.73-205") unless that number is already decided and fixed +— branch-local "next version" labels are placeholders and the actual +release tag (e.g. it could become 1.4.0 instead) is often undecided until +release time. When documenting a still-unreleased addition, either omit the +version or say it's "on this branch, not yet released under a tag". When +documenting a removed/renamed/deprecated item, anchor on the last tag that +is certain to have shipped it (e.g. "present up to and including +1.2.73-204") rather than the uncertain next tag. \ No newline at end of file diff --git a/.claude/hooks/lintr-r-changes.sh b/.claude/hooks/lintr-r-changes.sh new file mode 100755 index 0000000..7253eb3 --- /dev/null +++ b/.claude/hooks/lintr-r-changes.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +set -ueo pipefail + +input="$(cat)" +filePath="$(jq -r '.tool_input.file_path // .tool_response.filePath // empty' <<<"$input")" + +[[ "$filePath" =~ \.[Rr]$ ]] || exit 0 +[[ -f "$filePath" ]] || exit 0 +command -v Rscript >/dev/null 2>&1 || exit 0 + +output="$(Rscript -e 'lints <- lintr::lint(commandArgs(trailingOnly = TRUE)[1]); if (length(lints) > 0) print(lints)' "$filePath" 2>&1)" + +[[ -z "$output" ]] && exit 0 + +jq -n --arg ctx "lintr found issues in $filePath: +$output" '{hookSpecificOutput: {hookEventName: "PostToolUse", additionalContext: $ctx}}' \ No newline at end of file diff --git a/.claude/hooks/shellcheck-bash-changes.sh b/.claude/hooks/shellcheck-bash-changes.sh new file mode 100755 index 0000000..c8482c7 --- /dev/null +++ b/.claude/hooks/shellcheck-bash-changes.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -ueo pipefail + +input="$(cat)" +filePath="$(jq -r '.tool_input.file_path // .tool_response.filePath // empty' <<<"$input")" + +[[ "$filePath" == *.sh ]] || exit 0 +[[ -f "$filePath" ]] || exit 0 + +if output="$(shellcheck "$filePath" 2>&1)"; then + exit 0 +fi + +jq -n --arg ctx "shellcheck found issues in $filePath: +$output" '{hookSpecificOutput: {hookEventName: "PostToolUse", additionalContext: $ctx}}' \ No newline at end of file diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..bcc06f1 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,21 @@ +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Write|Edit", + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/shellcheck-bash-changes.sh\" 2>/dev/null || true", + "statusMessage": "Running shellcheck..." + }, + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/lintr-r-changes.sh\" 2>/dev/null || true", + "statusMessage": "Running lintr..." + } + ] + } + ] + } +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index 23a091e..6976c43 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,6 @@ manifest.tmp build/ out/ shunit2/ +.claude/tasks/ +.claude/CLAUDE.local.md +.claude/local/ \ No newline at end of file diff --git a/AlignmentAndQCWorkflows.jar b/AlignmentAndQCWorkflows.jar index 14fa7d4..d5ff0ab 100644 Binary files a/AlignmentAndQCWorkflows.jar and b/AlignmentAndQCWorkflows.jar differ diff --git a/AlignmentAndQCWorkflows_1.2.73-205.iml b/AlignmentAndQCWorkflows_1.2.73-205.iml new file mode 100644 index 0000000..992fb11 --- /dev/null +++ b/AlignmentAndQCWorkflows_1.2.73-205.iml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/AlignmentAndQCWorkflows_1.2.73.iml b/AlignmentAndQCWorkflows_1.2.73.iml deleted file mode 100644 index ede7797..0000000 --- a/AlignmentAndQCWorkflows_1.2.73.iml +++ /dev/null @@ -1,36 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/README.md b/README.md index 2f80131..6497ed6 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,18 @@ The original script with a documentation of the underlying ideas can be found [h ## Change Logs +* 1.2.73-205 (branch-specific change) + - minor: Added `bwaPostAltJsK8Options` to allow setting opt K8 options for `bwa-postalt.js` + - minor: Upgrade from COWorkflows 1.2.76 to COWorkflowsBasePlugin 1.4.2. + - minor: Old `FLAG_USE_EXISTING_PAIRED_BAMS` was renamed to `FLAG_USE_ONLY_EXISTING_PAIRED_BAMS` + - minor: Removed `FLAG_RUN_SLIM_WORKFLOW`. Corresponding code is unused for years. + - patch: Added `SAM_SORT_MEMSIZE` to allow reducing memory for SAM sorting. Default: `500M` per thread (see issue #97: unbounded growth with ALT-contig/`bwa-postalt.js` processing was traced to `samtools sort`, not to `bwa-postalt.js` itself; bounding this cvalue fixes it for all alignment workflow runs, not only ALT-aware ones). + * `SAMPESORT_MEMSIZE` (the original name for this cvalue) is deprecated but still works: `SAM_SORT_MEMSIZE` defaults to `${SAMPESORT_MEMSIZE}`, so existing project configs that set the old name keep functioning unchanged. + - patch: Little refactorings and groovification of old Java code + +* 1.2.73-204 (branch-specific change) + - minor: Separate BWA from BWAKIT version. Default `BWAKIT_VERSION` to `BWA_VERSION`. Independently set `K8_VERSION` (default 0.2.5). Changed associated module-loading code in environment setup file `tbi-lsf-cluster.sh`. + * 1.2.73-203 (branch-specific change) - minor: Optional ALT-chromosome processing via bwa.kit's `bwa-postaln.js`. * Set `runBwaPostAltJs=true` to activate the ALT chromosome processing. Default: `false`. diff --git a/buildinfo.txt b/buildinfo.txt index 862e3b7..525deae 100644 --- a/buildinfo.txt +++ b/buildinfo.txt @@ -1,4 +1,4 @@ -dependson=COWorkflows:1.2.76 +dependson=COWorkflowsBasePlugin:1.4.2 JDKVersion=1.8 GroovyVersion=2.4 -RoddyAPIVersion=3.0 +RoddyAPIVersion=3.7 diff --git a/docs/developers/CodingGuidelines.adoc b/docs/developers/CodingGuidelines.adoc new file mode 100644 index 0000000..2c9743c --- /dev/null +++ b/docs/developers/CodingGuidelines.adoc @@ -0,0 +1,312 @@ + +:toc: left + += Coding Guidelines + +Conventions covering Python, Bash, Perl, R, Groovy, and Java, plus +scientific-code practices from the OTP project's +https://gitlab.com/one-touch-pipeline/otp/-/wikis/Workflow-Development/Coding-Guidelines[Coding Guidelines wiki page] (Bash, Perl, and general failure-mode-awareness +sections below cite it as source). These are generic conventions meant to +guide new and changed code in any of the covered languages; they are not an +audit of existing code, and they name no specific files or classes. + +NOTE: Deviations found in existing code belong in dedicated tasks, not in this +document. Repository-specific tooling and directory-scope notes for this +plugin are in `docs/developers/RepositoryCodingConventions.adoc`, kept +separate so this document stays applicable to any codebase. + +== General Coding Conventions + +Coding cycle: + +. Design the interface. +. Present the design for review; optionally refine (1). +. Write tests to capture the behaviour. +. Implement functionality without modifying any tests. +. Present the changes for review; optionally refine the implementation (3). + +=== Design Guidelines + +* Assess for every change whether the code follows + the SOLID principles and flag deviations. +* Identify and name OOP patterns in code and docs. +* Analyse the code for code smells and name them explicitly. +* Adhere to Clean Code principles and discipline. +* Avoid comments about *what* code does -- prefer comments that state *why* a + specific algorithm, data structure, pattern, or approach was chosen (unless + trivial or obvious). +* Prefer object-functional or purely functional style; avoid procedural style. +* Apply 12-factor app principles. +* Prefer strongly and statically typed code over weakly or dynamically typed + solutions. +* Apply the principle of least surprise. +* Use "tell, don't ask" for object-oriented code. +* Use command-query separation for any code (functional, procedural, and + object-oriented). +* Name variables and functions by what they do, not by the type they refer + to. Avoid Hungarian notation. Use type annotations. + +Avoid over-engineering: only make changes that are directly requested or +clearly necessary. Don't add features, refactor surrounding code, or make +"improvements" beyond what was asked. Don't add error handling for scenarios +that cannot happen. + +=== Failure-Mode Awareness (source: OTP Coding Guidelines) + +Scientific/bioinformatics code needs the same rigor as wet-lab work -- errors +in data analysis can be as damaging as experimental mistakes. The primary +risk is *"Happy-Path-Coding"*: implementing as if all external interactions +succeed, and ignoring failure modes. + +Common failure points to design against, especially in production workflows +processing many samples across a cluster (where failure probabilities +compound): + +* Missing or misnamed files. +* Incorrect parameters (type, range, compatibility, consistency). +* Syntactically or semantically invalid file content. +* Empty files (from undetected errors or incomplete writes). +* Format inconsistencies between tools (e.g. BWA vs. Bowtie BAMs). +* Filesystem I/O errors on NFS and other networked/cluster filesystems. +* Truncated input files from external sources. +* Failing external programs that don't report errors properly. +* Memory allocation failures with large datasets. + +Mitigations: study the error-handling model of the language you're using; +distinguish spike/exploratory solutions from production code; add automated +tests (unit, integration, end-to-end); use reusable error-checking patterns +and functions instead of ad-hoc checks; make input assumptions explicit; +choose the language deliberately (avoid Bash for complex logic, see below); +use type checkers (Mypy) and linters, or use statically typed languages. + +=== Test Writing + +* Aim for full line coverage. +* Test extreme/boundary values. Consider property-based testing where + applicable. +* Check for division by zero or overflow/underflow. +* Think about how to break the code, and document possible (security) bugs. + +== Language-Specific Conventions + +=== Groovy + +* Annotate classes/methods `@CompileStatic` by default; use `@CompileDynamic` + (or `TypeChecking.SKIP`) only where dynamic dispatch is genuinely needed, + and say why at the point it's used. +* Write idiomatic Groovy, not Java-with-different-syntax: use Groovy's + collection/closure operators, GStrings, safe-navigation (`?.`) and Elvis + (`?:`) operators, and truthiness rather than porting Java verbosity + wholesale. +* Avoid `null` where possible; where a value is genuinely optional, annotate + it explicitly (e.g. JetBrains `@Nullable`) and enable build-level + null-checks so that everything left un-annotated is treated as `@NotNull` + by default -- this is what makes "prefer strongly typed code" (see Design + Guidelines) meaningful for null-handling specifically. +* Use Spock for testing (BDD-style `given`/`when`/`then` blocks); use Gradle + as the build tool where the project's build system allows it. + +=== Java + +* Prefer the most modern Java idioms actually available at the project's configured language level (e.g. streams, `Optional`, and -- only where the language level supports them -- `var`, records, and pattern matching) over older boilerplate; check the project's +actual target JDK version before assuming a given feature is usable, rather +than defaulting to the newest language you happen to know (a repository may +be pinned to an older JDK for reasons outside this document's scope -- see +that project's own repository-specific conventions). + +=== Python + +* Whenever possible use Mypy for type checking. +** Always use explicit type annotations. +* Avoid unsafe `cast`. Only use it when the local context makes the type + unambiguous, or when necessary because other solutions add unnecessary + complexity -- explain and get user-confirmed before using it. +* Avoid generic "types" such as `str`, `list`, or `dict`. Always prefer + existing explicit types, like `Path`, and don't stop with nested types. For + instance, instead of `dict`, use `Dict[str, str]` for actual + string-to-string maps. +* Be aware of other types in Python, like named tuple. Types should express + the part of the meaning of code that the variables don't (and should not) + capture, and much of what is usually only documented in comments. +* Use pytest for testing. Consider `hypothesis` for property-based testing. + +=== Bash + +* No braces on variables unless required (`:-`, `:?`, ambiguous boundary). +* Prefer `$(...)` over backticks; double-quote variables and subshells. +* Always declare `local` variables in functions. +* Use subshells for function calls, to prevent leakage of local state to calling environment. If it is much simpler to allow the side-effect of modifying the outer environment, ask the user and present the alternative implementations. Criteria are simplicity and readibility that may conflict with safety (because of possible side-effects). +* Bind positional params (`$1`, ...) to named `local` variables via `:?` with + a purpose description. +* Apply the same quality standards to shell as to any other language. +* Always use `set -ueo pipefail` (or the available subset/superset). On + Bash >= 4.4, also add `shopt -s inherit_errexit` so `errexit` propagates + into subshells and command substitutions, which don't inherit it by + default. +* Prefer a dedicated, non-shell language for complex logic (OTP guideline: + "writing safe Bash code is extremely hard, if not impossible" -- Bash + lacks real data structures, has fragile/surprising error-handling + semantics, and known version-specific bugs). Keep shell scripts to + orchestration/glue; move non-trivial logic to a language with better + error handling and data structures where practical. +* Use `shellcheck` on every script -- ideally both as a local check before + commit and in CI. +* Function guideline: "if you write a comment saying what the following code + is doing, substitute the code and comment by a function call" -- but use + functions sparingly in Bash; if a script needs many, that's a signal to + switch languages (per the point above). +* Function return values: a function's exit/return code should only signal + success/failure (an exception channel), not carry a result value. Return + actual results via stdout and capture with command substitution: ++ +[source,bash] +---- +yourFunction() { + doSomething + echo "$result" +} +resultValue=$(yourFunction) +---- ++ +Alternatively, for complex data use I/O to files. +* For debugging, `set -o verbose`/`set -o xtrace` print commands as read/ + after expansion; combine with `set -o errtrace` and an `ERR` trap to log + the failing command and exit code before propagating. +* Exit codes must stay `<= 127`. Codes above 127 collide with the + `128 + SIGNAL_NUMBER` convention for signal-terminated processes, making + failure diagnosis ambiguous. + +=== Perl (source: OTP Coding Guidelines) + +* Prefer the `autodie` pragma (`use autodie;`) so most I/O failures raise + exceptions automatically instead of failing silently. Note its limitation: + list-context reads can still fail silently even with `autodie`. +* Where `autodie` isn't used, check I/O explicitly rather than trusting a + bare read/write/close to succeed: ++ +[source,perl] +---- +use Carp; +my $line = readline(STDIN) + || croak "Couldn't read header from standard input: $!"; + +open(my $outFh, '>', $filename) + || croak "Error opening file '$filename': $!"; +print $outFh "Some text" + || croak "Couldn't write to '$filename': $!"; +close $outFh + || croak "Error closing file '$filename': $!"; +---- +* Avoid the unsafe idiom of relying on `<>`/`` in a boolean/loop + context without checking `defined`-ness -- it can fail silently on a read + error rather than only at legitimate EOF. +* The backtick operator does not automatically propagate a called command's + exit code as a Perl-level failure -- check `$?` explicitly: ++ +[source,perl] +---- +`executeAndFail --with this-parameter`; +($? & 127) || die "Command failed with code " . ($? >> 8); +---- + +=== R + +Adopted from the https://style.tidyverse.org/[tidyverse Style Guide] +(naming/formatting) and the +https://contributions.bioconductor.org/r-code.html[Bioconductor coding +guidelines] (defensive/safety practices), after checking both against the +Design Guidelines above and against one rejected alternative: + +Naming and formatting (tidyverse): + +* Use `snake_case` for object and function names (lowercase, `_`-separated) + -- avoids ambiguity with S3 method-dispatch naming (`function.class`). +* Variable names are nouns; function names are verbs (the R-specific form of + this document's general naming rule). +* Use `<-` for assignment, never `=`. +* Use `TRUE`/`FALSE`, never the reassignable `T`/`F` abbreviations. +* Space after commas, never before; space around infix operators (`==`, + `+`, `<-`) except high-precedence ones (`::`, `$`, `[`, `^`, `:`). +* Use `styler` (auto-formatting) and `lintr` (static analysis -- R's + equivalent of `shellcheck`) to enforce the above mechanically rather than + by review alone. + +Defensive programming and safety (Bioconductor): + +* Validate function arguments with `stopifnot()` (or an equivalent explicit + check) at the top of the function -- the direct R application of this + document's Failure-Mode Awareness section: don't assume a caller always + passes valid input. +* Prefer vectorized operations (`vapply()`, `apply()`-family) over explicit + `for` loops; prefer `vapply()` over `sapply()` specifically, since + `sapply()`'s return type depends on its input and can silently change + shape. +* Use `seq_len(n)`/`seq_along(x)` instead of `1:n`/`1:length(x)` -- the + classic R footgun: `1:length(x)` on a zero-length `x` yields `c(1, 0)` + instead of an empty sequence, so a loop guarded this way runs when it + should not. +* Use the `message()`/`warning()`/`stop()` hierarchy deliberately: + `message()` for diagnostic progress output, `warning()` for handled-but- + unusual situations, `stop()` for actual errors; reserve `cat()`/`print()` + for final result display only. +* Avoid `<<-` (implicit outer-scope assignment); prefer explicit function + return values -- consistent with this document's Bash guidance on + avoiding hidden mutation of the calling environment. +* Use `is(x, "Class")` rather than `class(x) == "Class"` for type checks. +* Use `testthat` for unit tests -- the R analog to the Python `pytest` + recommendation above. + +Type safety (S4) and static analysis: + +R has no compile-time type system comparable to Python's type hints + Mypy +-- it is dynamically typed at its core. The closest, and recommended, +approximations: + +* Prefer *S4* (`setClass()`) over plain named lists or data frames for any + structured value with more than a couple of related fields/invariants + (e.g. a parsed-options record, a result table with associated units/ + provenance). Declare field types via the `representation`/`slots` + argument (`setClass("Foo", representation(x = "numeric", label = + "character"))`) and add a `setValidity()` method for invariants that a + type alone can't express (ranges, cross-field consistency). This is + enforced at object-construction/`validObject()` time -- not compile-time, + but rigorous and automatic, and it's what makes SOLID's + interface/substitution principles literally applicable to R code (see + Compatibility assessment above) rather than just analogous. +* Where a full S4 class is overkill, at minimum validate with + `stopifnot()`/`is()` checks per the Defensive programming rules above -- + don't leave non-trivial structured data as an untyped list with no + validation anywhere. +* For genuine static analysis (bugs caught without running the code): + `lintr` (already recommended above; AST-based, R's `shellcheck` analog) + and `codetools::checkUsage()` (ships with base R -- finds unused + variables, undefined globals, and other usage errors). `goodpractice` + bundles `lintr` with cyclomatic-complexity and test-coverage checks into + one report if a more comprehensive gate is wanted later. +* Considered and not currently recommended: the `typed` package (more + literal Python-style annotations, but still niche/experimental) and + `vctrs` (excellent for building reusable custom vector types, but + heavier-weight than most script-local data structures need) -- revisit + either if R code in a given codebase grows into a proper package with a + reusable data model. + +== Performance and Resource Management (source: OTP Coding Guidelines) + +Don't over-optimize early -- focus on correctness and data-flow architecture +first, not micro-patterns. One area that *does* need attention from the +start on shared cluster infrastructure is garbage-collected runtimes' +default memory behavior: + +* *JVM (Java/Groovy)*: the default JVM heap can expand to a large fraction of + node memory, which is fine on a dedicated machine but can starve other jobs + on a shared cluster node. Set an explicit, deliberately small default (e.g. + `JAVA_OPTS=-Xmx1024m`) and only raise it where a specific job actually + needs more, rather than relying on the JVM's automatic sizing. +* *Python*: consider bounding heap growth for long-running or per-sample + scripts with `resource.setrlimit(resource.RLIMIT_DATA, ...)` if a script + turns out to use unexpectedly large amounts of memory for what it does + (the OTP guideline cites a simple FASTQ parser that used 300 MB + unnecessarily as a real example worth catching this way). +* *R and Perl*: no specific tuning guidance from the source material beyond + R's general-purpose `gc()`; revisit if a concrete memory problem in an R or + Perl script warrants it. \ No newline at end of file diff --git a/docs/developers/ExternalInterfaceChangeLog.adoc b/docs/developers/ExternalInterfaceChangeLog.adoc new file mode 100644 index 0000000..25ae01c --- /dev/null +++ b/docs/developers/ExternalInterfaceChangeLog.adoc @@ -0,0 +1,141 @@ + +:toc: left + += External-Interface Change Log + +This document tracks changes to this plugin's *external interface* -- +cvalues external project configs can set/override, `` names, +environment-script names, documented file-naming conventions, and public +Groovy/Java API (classes/methods other plugins could extend or call). + +== Identified items + +[cols="1,3,3,2,4", options="header"] +|=== +| # | Change | Source task | External surface | Risk if not confirmed + +| 1 +| `useCombinedAlignAndSampe` cvalue removed entirely (was already default `false`/deprecated) +| `drop-bwa-aln-sampe-usecombinedalignandsampe` (done by `origin/cleanups` merge) +| Project XML `` override +| Any external project config still setting this cvalue will have it silently ignored (ineffective), not error -- a config author could believe they're selecting a code path that no longer exists + +| 2 +| `runSlimWorkflow` cvalue removed entirely +| `drop-bwa-aln-sampe-usecombinedalignandsampe` (done by merge) +| same as above +| same as above + +| 3 +| `useAcceleratedHardware` cvalue removal (planned) +| `drop-convey-accelerated-execution` +| Project XML `` override, incl. `analysisBisulfiteCore.xml` +| Any external config relying on Convey/accelerated hardware execution silently loses that behavior with no error + +| 4 +| `CONVEY_SAMBAMBA_SAMSORT_THREADS` / `CONVEY_SAMBAMBA_SAMSORT_MEMSIZE` cvalues removed (planned) +| `drop-convey-accelerated-execution` +| Project XML `` override +| Same as #3 + +| 5 +| `` entries removed: `alignment`, `accelerated:alignment`, `alignAndPair`, `accelerated:alignAndPair`, `accelerated:alignAndPairSlim`, `sampesort`, `sampesortSlim`, plus several QC tools (`samtoolsIndex`, `samtoolsFlagstat`, `insertSizes`, `genomeCoverage`, `genomeCoveragePyScript`, `readBinsCoverage`, `collectBamMetrics`, `chromosomeDiff`, `insertSizesBucketSortScript`) +| `drop-bwa-aln-sampe-usecombinedalignandsampe`, `drop-convey-accelerated-execution`, `drop-unused-scripts` (mostly done by `origin/cleanups` merge) +| Project XML resource-set overrides (`...`), *and cluster job-monitoring*: tool names are the job names operators/monitoring scripts see and match against in `bjobs`/`qstat`-style listings and Roddy's execution store (see `audit-migrated-wiki-content` finding 5 -- this was previously undocumented anywhere) +| An external project config that customizes resource sets for any of these tool names will have that override become dead configuration (Roddy's behavior for an override of a nonexistent tool name is not verified here -- could be silently ignored or could error at config-validation time; *open question, needs checking against Roddy core*). Separately, any external cluster-monitoring dashboard/script that matches on one of these job names by name will silently stop seeing it once the tool is removed/renamed -- this is a second, independent audience to consider beyond project-config authors, and the stakes are higher than a config no-op (a monitoring gap can hide a real production failure) + +| 6 +| `listOfUsedTools` in `analysisExome.xml`, `analysisGenomePancanSlim.xml`, `analysisBisulfiteCore.xml` still lists tool names whose definitions were deleted by the `origin/cleanups` merge (pre-existing gap in that merge, not caused by any task here) +| flagged by several tasks' plans, not owned by one +| Project XML `imports="qcAnalysis"` chains +| Likely a latent config-validation warning/error waiting to surface; not new, but should be fixed alongside whichever task touches these files next + +| 7 +| `K8_BINARY` cvalue proposed as a *new* addition to `analysisQC.xml` (currently only has a hardcoded fallback in shell scripts, no XML cvalue at all) +| `review-finding-6-uncoordinated-defaults` +| Project XML `` +| Additive, not breaking -- flagged here only so it's not missed as a documented, user-facing new option + +| 8 +| `workflowEnvironmentScript` default switched from `workflowEnvironment_tbiPbs` to `workflowEnvironment_tbiLsf`, and the `workflowEnvironment_tbiPbs` tool + `tbi-pbs-cluster.sh` deleted entirely +| Already done in the reviewed diff (`1.2.73-201` -> `1.2.73-204`), predates the `.claude/tasks/` batch +| Project XML `` override +| Any external project config that still explicitly sets `workflowEnvironmentScript=workflowEnvironment_tbiPbs` breaks outright (tool no longer exists) -- this is a *known, already-shipped* breaking change, not something to fix, just to document/announce + +| 9 +| Public Groovy/Java API removal candidates: `AlignmentAndQCConfig.getUseCombinedAlignAndSampe()`/`getRunSlimWorkflow()` (deprecated getters), `QCPipeline.mergeAndRemoveDuplicatesFat()`, `COProjectsRuntimeService.getMergedBamFilesForDataSet()` (both overloads), `LaneFileGroup.alignAndPair()`/`alignAll()`, `LaneFile.align()`, the whole `AlignedSequenceFileGroup`/`AlignedSequenceFile` classes +| `drop-bwa-aln-sampe-usecombinedalignandsampe`, `drop-convey-accelerated-execution` +| *Groovy/Java class/method API*, not just XML config +| These files/methods share names with classes in `COWorkflowsBasePlugin`/`COWorkflows` (`plugins/COWorkflows_1.2.20/src/de/dkfz/b080/co/files/{LaneFile,LaneFileGroup,AlignedSequenceFileGroup}.java`, `COProjectsRuntimeService.groovy`) -- *unresolved: verify whether this plugin's classes `extends`/override those base classes, or are independent copies from before the plugin was split out of COWorkflows.* If they override base-class methods, deleting them here could be fine (base no-ops take over) or could break an `@Override` contract depending on Roddy's generic-method dispatch. Do not delete without checking `COWorkflowsBasePlugin` inheritance first. + +| 10 +| `bwaMemSortSlim.sh`'s exit-code surface: folding the never-checked `procID_logwrite` into the new fail-fast wait helper means its failure now produces a *new* exit code (proposed `18`) that never occurred before +| `bash-wait-deadlock-fix` +| Job exit code (observable by external job-monitoring/alerting around Roddy, not by project XML) +| Low risk, but any external tooling that maps specific exit codes to alert dashboards should be told about the new code + +| 11 +| README `ALT_FILE` extension `.aln` -> `.alt` +| `review-finding-8-readme-alt-file-extension` +| Documentation only +| *Not a real interface change* -- the code always used `.alt`; only the README text was wrong. Included here only to explicitly rule it out as a compatibility concern. + +| 12 +| Script deletions: `failedReadFilter.pl`, `coveragePlotGenome.r`, `coveragePlotsPerChrom.r`, `insertSizeDistribution.r`, `hartigansDip.R` (candidate) +| `drop-unused-scripts` +| None found via `` registration or internal script calls +| Low risk -- flagged only in case some external wrapper invokes these scripts directly by path outside of Roddy's tool-registration mechanism (not discoverable from inside this repo) + +| 13 +| `SAMPESORT_MEMSIZE` cvalue (present up to and including `1.2.73-204`, the last tagged release) renamed to `SAM_SORT_MEMSIZE` on this branch, with default changed from `2000000000` (~2 GiB) to `500M` per thread; old name kept as a deprecated alias (`SAM_SORT_MEMSIZE` defaults to `${SAMPESORT_MEMSIZE}`); `bwaPostAltJsK8Options` cvalue added on this branch (not yet released under any tag -- the eventual release version for this branch is undecided, so not asserted here) +| `verify-and-close-alt-hla-memory-fix` (issue #97), commits `f1f1d46`/`9634047` plus this rename +| Project XML ``, and all shell scripts under `resources/analysisTools/` that read the memory-size variable (now read `SAM_SORT_MEMSIZE` instead of `SAMPESORT_MEMSIZE`) (`SAMPESORT_MEMSIZE` also existed under this name with a `20G` default years earlier in this plugin's history before being dropped, then reintroduced with a different, much lower default -- external configs from that era setting `SAMPESORT_MEMSIZE` explicitly are unaffected by the rename, but any relying on the historical *default* would see a large change if they ever return) +| Additive/renamed, not breaking for current configs: external configs that set `SAMPESORT_MEMSIZE` keep working unchanged via the default-chaining fallback (`SAM_SORT_MEMSIZE` value `"${SAMPESORT_MEMSIZE}"`), the same idiom already used elsewhere in `analysisQC.xml` (e.g. `BWAKIT_VERSION` defaulting to `${BWA_VERSION}`). `500M` per thread (`samtools sort -@ 8`, so ~4G total) is the value empirically validated in issue #97 to bound the previously-unbounded memory growth seen with ALT-contig/`bwa-postalt.js` processing. Risk: any external tooling that greps job scripts or Roddy configs for the literal string `SAMPESORT_MEMSIZE` (rather than reading the resolved value) would no longer find it in the backend scripts, only in the deprecated cvalue definition +|=== + +== Decision (2026-07-08): items 1-4 -- silent removal, documented as deprecated + +User decision: silent removal is appropriate for `useCombinedAlignAndSampe`, +`runSlimWorkflow`, `useAcceleratedHardware`, +`CONVEY_SAMBAMBA_SAMSORT_THREADS`/`CONVEY_SAMBAMBA_SAMSORT_MEMSIZE` -- no +compatibility shim, no fail-fast guard. Two conditions attached: + +. Document the removal in the README.md "Change Logs" section for whichever + release includes it. +. Document each variable in `docs/` (retroactively, if it was never + documented before), marked deprecated -- state explicitly that each was + only valid up to and including `1.2.73-204` and was likely already + dysfunctional by that version (per `review-finding-1-usecombinedalignandsampe-default`: + `useCombinedAlignAndSampe`'s default already made the flag a no-op at + `1.2.73-204`, so "deprecated" here means "document what it used to do," + not "still works but discouraged"). + +This closes the open question for items 1-4. Item 5 (`` removals) and +item 9 (Groovy/Java API removals) are separate, unresolved questions -- not +covered by this decision -- since they involve different failure modes +(possible Roddy config-validation errors; possible base-class override +breakage) rather than a silent-no-op cvalue read. + +== Open follow-ups + +* Add a README.md "Change Logs" entry for items 1-4 when + `drop-bwa-aln-sampe-usecombinedalignandsampe` / `drop-convey-accelerated-execution` ship. +* Add/retrofit documentation in `docs/users/ConfiguringAndRunning.adoc` (see + `migrate-gitlab-wiki-to-docs`) for `useCombinedAlignAndSampe`, + `runSlimWorkflow`, `useAcceleratedHardware`, + `CONVEY_SAMBAMBA_SAMSORT_THREADS`, `CONVEY_SAMBAMBA_SAMSORT_MEMSIZE` marked + deprecated/removed-as-of, even if they were never documented while active. +* Get user sign-off on item 8 (env-script default switch, already shipped -- + announce only, same silent/documented approach presumably applies but + wasn't explicitly asked -- confirm). +* Resolve item 5's open question: check Roddy core's behavior when a project + config overrides a `` name that no longer exists in the imported + base configuration (silent ignore vs. validation error). +* Resolve item 9's open question: check whether `AlignmentAndQCWorkflows`'s + `LaneFile`/`LaneFileGroup`/`AlignedSequenceFileGroup`/`COProjectsRuntimeService` + override or extend same-named classes in `COWorkflowsBasePlugin`/`COWorkflows` + before any of their methods are deleted. +* Once all tasks in the current batch are implemented, produce a single + user-facing README.md "Change Logs" entry summarizing all confirmed + breaking changes together, rather than scattering them across per-task + edits. diff --git a/docs/developers/README.adoc b/docs/developers/README.adoc new file mode 100644 index 0000000..6587554 --- /dev/null +++ b/docs/developers/README.adoc @@ -0,0 +1,6 @@ + +== Developer Documentation + +* link:CodingGuidelines.adoc[General coding conventions] +* link:RepositoryCodingConventions.adoc[Repository-specific coding conventions] +* link:ExternalInterfaceChangeLog.adoc[External-interface change log] \ No newline at end of file diff --git a/docs/developers/RepositoryCodingConventions.adoc b/docs/developers/RepositoryCodingConventions.adoc new file mode 100644 index 0000000..49c2da6 --- /dev/null +++ b/docs/developers/RepositoryCodingConventions.adoc @@ -0,0 +1,25 @@ + +:toc: left + += Repository-Specific Coding Conventions + +This document holds conventions specific to *this* repository -- how the +generic rules in link:CodingGuidelines.adoc[] map onto this +plugin's actual layout and tooling. + +== Language and Directory Scope + +* This plugin is *not* a Gradle project but is built via `Roddy/helperScripts/compileRoddyPlugin.sh` from the Roddy project. +* This plugin must build against **JDK 8 and Groovy 2.4/2.5** -- the same + versions the `Roddy` core it plugs into targets. + +== Tooling + +* `shellcheck` is wired as a local `PostToolUse` hook for Bash file changes + made through Claude Code (`.claude/hooks/shellcheck-bash-changes.sh`, + registered in `.claude/settings.json`). It reports findings but does not + block edits. +* Bash unit tests use `shunit2` (not vendored/committed -- clone + `https://github.com/kward/shunit2.git` locally, per + `docs/developers/WorkflowStructureAndTesting.adoc` once that page is + migrated from the wiki) and are run via `resources/tests/runTests.sh`. diff --git a/resources/analysisTools/bisulfiteWorkflow/bwaMemSortSlimWithReadConversionForBisulfiteData.sh b/resources/analysisTools/bisulfiteWorkflow/bwaMemSortSlimWithReadConversionForBisulfiteData.sh index 4f253a5..d06537a 100755 --- a/resources/analysisTools/bisulfiteWorkflow/bwaMemSortSlimWithReadConversionForBisulfiteData.sh +++ b/resources/analysisTools/bisulfiteWorkflow/bwaMemSortSlimWithReadConversionForBisulfiteData.sh @@ -178,7 +178,7 @@ else ${SAMTOOLS_BINARY} view -uSbh -F 2304 - | \ $MBUF_LARGE | \ ${SAMTOOLS_BINARY} sort -@ 8 \ - -m ${SAMPESORT_MEMSIZE} \ + -m ${SAM_SORT_MEMSIZE} \ -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ tee ${NP_COVERAGEQC_IN} \ ${NP_READBINS_IN} \ diff --git a/resources/analysisTools/qcPipeline/bwaMemSort.sh b/resources/analysisTools/qcPipeline/bwaMemSort.sh index 42af49d..2f60a2c 100755 --- a/resources/analysisTools/qcPipeline/bwaMemSort.sh +++ b/resources/analysisTools/qcPipeline/bwaMemSort.sh @@ -59,7 +59,7 @@ SAMTOOLS_SORT_BINARY=samtools-0.1.19 #then # # Single end read is active # nice ${UNZIPTOOL} ${UNZIPTOOL_OPTIONS} $RAW_SEQ_1 ${TRIM_STEP} ${REVERSE_STEP} | mbuffer -q -m 1G -l /dev/null > $FNPIPE1 & -# ${BWA_BINARY} samse -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} $FNPIPE1 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP} +# ${BWA_BINARY} samse -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} $FNPIPE1 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP} #if [[ $useAdaptorTrimming == false ]] #then @@ -73,8 +73,8 @@ SAMTOOLS_SORT_BINARY=samtools-0.1.19 # # ## -r STR read group header line such as `@RG\tID:foo\tSM:bar' [null] # #Samtools 0.1.19 uses multithreaded sorting => therefore it is hardcoded here. -# ${BWA_BINARY} mem -t ${THREADS} -R "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" $BWA_MEM_OPTIONS ${INDEX_PREFIX} $FNPIPE1 $FNPIPE2 | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ ${THREADS} -m ${SAMPESORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP} -## ${BWA_BINARY} sampe -P -T -t 8 ${BWA_SAMPESORT_OPTIONS} -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} ${FILENAME_SEQ_2} $FNPIPE1 $FNPIPE2 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP} +# ${BWA_BINARY} mem -t ${THREADS} -R "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" $BWA_MEM_OPTIONS ${INDEX_PREFIX} $FNPIPE1 $FNPIPE2 | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ ${THREADS} -m ${SAM_SORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP} +## ${BWA_BINARY} sampe -P -T -t 8 ${BWA_SAMPESORT_OPTIONS} -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} ${FILENAME_SEQ_2} $FNPIPE1 $FNPIPE2 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP} # #else @@ -115,7 +115,7 @@ if [[ "$ON_CONVEY" == "true" ]]; then cat ${FNPIPE_FLAGSTATS_IN} | ${SAMBAMBA_BINARY} flagstat /dev/stdin > $FLAGSTAT_TMP & procID_FSTATS=$! (set -o pipefail; ${bwaBinary} mem -t 12 -R "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" $BWA_MEM_OPTIONS ${INDEX_PREFIX} ${INPUT_PIPES} | $MBUFFER_2 | \ ${SAMBAMBA_BINARY} view -f bam -S -l 0 -t 8 /dev/stdin | $MBUFFER_2 | \ - tee ${FNPIPE_FLAGSTATS_IN} | ${SAMBAMBA_BINARY} sort --tmpdir=${LCLSCRATCH} -l 9 -t 8 -m ${SAMPESORT_MEMSIZE} /dev/stdin -o $TMP_FILE; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_MEMSORT=$! + tee ${FNPIPE_FLAGSTATS_IN} | ${SAMBAMBA_BINARY} sort --tmpdir=${LCLSCRATCH} -l 9 -t 8 -m ${SAM_SORT_MEMSIZE} /dev/stdin -o $TMP_FILE; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_MEMSORT=$! wait $procID_MEMSORT [[ ! `cat ${DIR_TEMP}/${bamname}_ec` -eq "0" ]] && echo "bwa mem-sambamba pipe returned a non-zero exit code and the job will die now." && exit 100 wait $procID_FSTATS @@ -124,7 +124,7 @@ elif [[ ${useBioBamBamSort} == false ]]; then cat ${FNPIPE_INDEX_IN} | ${SAMTOOLS_SORT_BINARY} index - ${TMP_FILE_INDEX} & procID_IDX=$! (set -o pipefail; ${BWA_BINARY} mem -t ${THREADS} -R "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" $BWA_MEM_OPTIONS ${INDEX_PREFIX} ${INPUT_PIPES} | $MBUFFER_2 | \ ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | \ - ${SAMTOOLS_SORT_BINARY} sort -@ ${THREADS} -m ${SAMPESORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} ${FNPIPE_INDEX_IN} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP}; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_MEMSORT=$! + ${SAMTOOLS_SORT_BINARY} sort -@ ${THREADS} -m ${SAM_SORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} ${FNPIPE_INDEX_IN} | ${SAMTOOLS_SORT_BINARY} flagstat - > ${FLAGSTAT_TMP}; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_MEMSORT=$! wait $procID_MEMSORT [[ ! `cat ${DIR_TEMP}/${bamname}_ec` -eq "0" ]] && echo "bwa mem-samtools pipe returned a non-zero exit code and the job will die now." && exit 100 wait procID_IDX diff --git a/resources/analysisTools/qcPipeline/bwaMemSortSlim.sh b/resources/analysisTools/qcPipeline/bwaMemSortSlim.sh index 5b0154c..282bc60 100755 --- a/resources/analysisTools/qcPipeline/bwaMemSortSlim.sh +++ b/resources/analysisTools/qcPipeline/bwaMemSortSlim.sh @@ -170,7 +170,7 @@ else (set -o pipefail; ${BWA_BINARY} mem -t ${BWA_MEM_THREADS} -R "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" $BWA_MEM_OPTIONS ${INDEX_PREFIX} ${INPUT_PIPES} 2> $FILENAME_BWA_LOG | $MBUF_LARGE | \ optionalBwaPostAltJs "$hlaPrefix" "$bwaPostAltJsMinPaRatio" | tee $NP_COMBINEDANALYSIS_IN | \ ${SAMTOOLS_BINARY} view -uSbh - | $MBUF_LARGE | \ - ${SAMTOOLS_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ + ${SAMTOOLS_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ tee ${NP_COVERAGEQC_IN} ${NP_READBINS_IN} ${NP_FLAGSTATS} ${NP_SAMTOOLS_INDEX_IN} > ${tempSortedBamFile}; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_MEMSORT=$! # filter samtools error log (cat $NP_SORT_ERRLOG | uniq > $FILENAME_SORT_LOG) & procID_logwrite=$! diff --git a/resources/analysisTools/qcPipeline/bwaSampeSort.sh b/resources/analysisTools/qcPipeline/bwaSampeSort.sh index f854f26..e1216b0 100755 --- a/resources/analysisTools/qcPipeline/bwaSampeSort.sh +++ b/resources/analysisTools/qcPipeline/bwaSampeSort.sh @@ -61,7 +61,7 @@ if [[ ${useSingleEndProcessing} == "true" ]] then # Single end read is active nice ${UNZIPTOOL} ${UNZIPTOOL_OPTIONS} $RAW_SEQ_1 ${TRIM_STEP} ${REVERSE_STEP} | mbuffer -q -m 1G -l /dev/null > $FNPIPE1 & - ${BWA_BINARY} samse -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} $FNPIPE1 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} $FNPIPE_INDEX $FNPIPE_FLAGSTATS > /dev/null + ${BWA_BINARY} samse -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} $FNPIPE1 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} $FNPIPE_INDEX $FNPIPE_FLAGSTATS > /dev/null else if [[ $useAdaptorTrimming == false ]] then @@ -89,7 +89,7 @@ else cat $o2 ${TRIM_STEP} ${REVERSE_STEP} | mbuffer -q -m 6G -l /dev/null > $FNPIPE2 & fi - ${BWA_BINARY} sampe -P -T -t 8 ${BWA_SAMPESORT_OPTIONS} -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} ${FILENAME_SEQ_2} $FNPIPE1 $FNPIPE2 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} $FNPIPE_INDEX $FNPIPE_FLAGSTATS > /dev/null + ${BWA_BINARY} sampe -P -T -t 8 ${BWA_SAMPESORT_OPTIONS} -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" ${INDEX_PREFIX} ${FILENAME_SEQ_1} ${FILENAME_SEQ_2} $FNPIPE1 $FNPIPE2 2> ${FILENAME_BWA_LOG} | $MBUFFER_2 | ${SAMTOOLS_BINARY} view -uSbh - | $MBUFFER_2 | ${SAMTOOLS_SORT_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${TMP_FILE_SORT} | tee ${TMP_FILE_SAMTOOLS} $FNPIPE_INDEX $FNPIPE_FLAGSTATS > /dev/null fi #Wait for unfinished samtools jobs. diff --git a/resources/analysisTools/qcPipeline/bwaSampeSortSlim.sh b/resources/analysisTools/qcPipeline/bwaSampeSortSlim.sh index 005c7d1..0fb906e 100755 --- a/resources/analysisTools/qcPipeline/bwaSampeSortSlim.sh +++ b/resources/analysisTools/qcPipeline/bwaSampeSortSlim.sh @@ -93,7 +93,7 @@ else # we have to make the BAM (set -o pipefail; ${BWA_BINARY} samse -T -t 8 -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" \ ${INDEX_PREFIX} ${FILENAME_SEQ_1} $FNPIPE1 2> ${FILENAME_BWA_LOG} | $MBUF_2G | \ tee $NP_COMBINEDANALYSIS_IN | ${SAMTOOLS_BINARY} view -uSbh - | $MBUF_2G | \ - tee $NP_FLAGSTATS_IN | ${SAMTOOLS_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ + tee $NP_FLAGSTATS_IN | ${SAMTOOLS_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ tee ${NP_COVERAGEQC_IN} ${NP_READBINS_IN} $NP_INDEX_IN > ${tempSortedBamFile}; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_SAMSESORT=$! # filter samtools error log (cat $NP_SORT_ERRLOG | uniq > $FILENAME_SORT_LOG) & procID_logwrite=$! @@ -129,7 +129,7 @@ else # we have to make the BAM (set -o pipefail; ${BWA_BINARY} sampe -P -T -t 8 ${BWA_SAMPESORT_OPTIONS} -r "@RG\tID:${ID}\tSM:${SM}\tLB:${LB}\tPL:ILLUMINA" \ ${INDEX_PREFIX} ${FILENAME_SEQ_1} ${FILENAME_SEQ_2} $FNPIPE1 $FNPIPE2 2> ${FILENAME_BWA_LOG} | \ $MBUF_2G | tee $NP_COMBINEDANALYSIS_IN | ${SAMTOOLS_BINARY} view -uSbh - | $MBUF_2G | \ - tee $NP_FLAGSTATS_IN | ${SAMTOOLS_BINARY} sort -@ 8 -m ${SAMPESORT_MEMSIZE} -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ + tee $NP_FLAGSTATS_IN | ${SAMTOOLS_BINARY} sort -@ 8 -m ${SAM_SORT_MEMSIZE} -o - ${tempFileForSort} 2>$NP_SORT_ERRLOG | \ tee ${NP_COVERAGEQC_IN} ${NP_READBINS_IN} $NP_INDEX_IN > ${tempSortedBamFile}; echo $? > ${DIR_TEMP}/${bamname}_ec) & procID_SAMPESORT=$! # filter samtools error log (cat $NP_SORT_ERRLOG | uniq > $FILENAME_SORT_LOG) & procID_logwrite=$! diff --git a/resources/analysisTools/qcPipeline/flags_isizes_PEaberrations.pl b/resources/analysisTools/qcPipeline/flags_isizes_PEaberrations.pl index d329493..9945af7 100755 --- a/resources/analysisTools/qcPipeline/flags_isizes_PEaberrations.pl +++ b/resources/analysisTools/qcPipeline/flags_isizes_PEaberrations.pl @@ -119,13 +119,13 @@ if ($_ =~ /^\@/) # there might be a SAM header {next;} $all++; - @help = split ("\t", $_); - $flag = $help[1]; + my ($qname, $flag, $rname, $pos, $mapq, $cigar, $rnext, $pnext, $tlen, $seq, $qual) = + split ("\t", $_); # not unmapped, no duplicate and no secondary/supplementary alignment, and mapqual >= X if (!($flag & 4) && !($flag & 1024) && !($flag & 256) && !($flag & 2048)) { $uniq++; - if ($help[4] >= $minmapq) + if ($mapq >= $minmapq) { # of these, with mapqual >= X $minmapuniq++; @@ -135,15 +135,15 @@ next; } # is the read itself on a wanted chromosome? - if (defined $chroms{$help[2]}) + if (defined $chroms{$rname}) { $onchr++; # and the mate on a wanted chromosome - if (defined $chroms{$help[6]} || $help[6] eq "=") # same chrom is usually indicated by "=" instead of repeating the name + if (defined $chroms{$rnext} || $rnext eq "=") # same chrom is usually indicated by "=" instead of repeating the name { $both++; # paired end aberration: mate also has to be mapped, on a different chrom - if (!($flag & 8) && $help[6] ne "=" && ($help[2] ne $help[6])) + if (!($flag & 8) && $rnext ne "=" && ($rname ne $rnext)) # keep matrix symmetrical to see whether there is a bias, e.g. more 1->10 than 10->1 { $aberrant++; @@ -151,14 +151,14 @@ # is more interesting if ($flag & 64) { - $chrompairs{$help[2]}{$help[6]}++; + $chrompairs{$rname}{$rnext}++; } } # for insert sizes, take first read of a proper pair (-f 67 = 64 (first in pair) + 2 (proper pair) + 1 (paired)); # discarding duplicates (-F 1024) is already done further up if ($flag & 64 && $flag & 2 && $flag & 1) { - $entry = abs($help[8]); # insert size + $entry = abs($tlen); # insert size if ($entry < $min) { $min = $entry; diff --git a/resources/analysisTools/qcPipeline/workflowLib.sh b/resources/analysisTools/qcPipeline/workflowLib.sh index 57fb0d6..702aa47 100755 --- a/resources/analysisTools/qcPipeline/workflowLib.sh +++ b/resources/analysisTools/qcPipeline/workflowLib.sh @@ -272,11 +272,12 @@ ALT_FILE="${ALT_FILE:-$INDEX_PREFIX.alt}" # By default assume that bwa-postalt.js and k8 are located besides bwa (like in bwakit). bwaPostAltJsPath="${bwaPostAltJsPath:-"$(dirname "$(which bwa)")"/bwa-postalt.js}" K8_BINARY="${K8_BINARY:-"$(dirname "$(which bwa)")"/k8}" +declare -a bwaPostAltJsK8Options="$bwaPostAltJsK8Options" optionalBwaPostAltJs() { local hlaPrefix="${1:-}" local minPaRatio="${2:-}" if [[ "$runBwaPostAltJs" == "true" ]]; then - $K8_BINARY "$bwaPostAltJsPath" ${hlaPrefix:+-p "$hlaPrefix"} ${minPaRatio:+-r "$minPaRatio"} "$ALT_FILE" + $K8_BINARY ${bwaPostAltJsK8Options[@]} "$bwaPostAltJsPath" ${hlaPrefix:+-p "$hlaPrefix"} ${minPaRatio:+-r "$minPaRatio"} "$ALT_FILE" else cat - fi diff --git a/resources/configurationFiles/analysisQC.xml b/resources/configurationFiles/analysisQC.xml index 091e8f3..45bab88 100644 --- a/resources/configurationFiles/analysisQC.xml +++ b/resources/configurationFiles/analysisQC.xml @@ -36,6 +36,8 @@ description="The bwakit module contains the bwa-postalt.js script. Used if runBwaPostAltJs=true"/> + @@ -81,6 +83,11 @@ + + + diff --git a/src/de/dkfz/b080/co/QualityControlWorkflowPlugin.java b/src/de/dkfz/b080/co/QualityControlWorkflowPlugin.java index a841951..aa31a88 100644 --- a/src/de/dkfz/b080/co/QualityControlWorkflowPlugin.java +++ b/src/de/dkfz/b080/co/QualityControlWorkflowPlugin.java @@ -9,7 +9,7 @@ public class QualityControlWorkflowPlugin extends BasePlugin { public static final String CURRENT_VERSION_STRING = "1.2.73"; - public static final String CURRENT_VERSION_BUILD_DATE = "Wed Feb 21 15:47:18 CET 2018"; + public static final String CURRENT_VERSION_BUILD_DATE = "Wed Apr 12 10:50:50 CEST 2023"; @Override public String getVersionInfo() { diff --git a/src/de/dkfz/b080/co/common/AlignmentAndQCConfig.groovy b/src/de/dkfz/b080/co/common/AlignmentAndQCConfig.groovy index afb81da..1fe2681 100644 --- a/src/de/dkfz/b080/co/common/AlignmentAndQCConfig.groovy +++ b/src/de/dkfz/b080/co/common/AlignmentAndQCConfig.groovy @@ -20,11 +20,19 @@ class AlignmentAndQCConfig extends COConfig { public static final String CVALUE_RUN_FINGERPRINTING = "runFingerprinting" public static final String CVALUE_FINGERPRINTING_SITES_FILE="fingerprintingSitesFile" - public AlignmentAndQCConfig(ExecutionContext context) { + AlignmentAndQCConfig(ExecutionContext context) { super(context) } - public String getSingleBamParameter() { + void setUseOnlyExistingTargetBam(boolean value = true) { + setConfig("useOnlyExistingTargetBam", value.toString(), "boolean") + } + + void setExtractSamplesFromOutputFiles(boolean value = true) { + setConfig("extractSamplesFromOutputFiles", value.toString(), "boolean") + } + + String getSingleBamParameter() { return configValues.get("bam", ""); } @@ -33,22 +41,26 @@ class AlignmentAndQCConfig extends COConfig { } boolean getUseExistingLaneBams() { - return configValues.getBoolean(COConstants.FLAG_USE_EXISTING_PAIRED_BAMS, false) + return configValues.getBoolean("useExistingLaneBams", false) } - public String getIndexPrefix() { + boolean getUseOnlyExistingPairedBams() { + return configValues.getBoolean("useExistingPairedBams", false) + } + + String getIndexPrefix() { return configValues.getString(CVALUE_INDEX_PREFIX, "") } - public File getChromosomeSizesFile() { + File getChromosomeSizesFile() { return new File (configValues.getString(CVALUE_CHROMOSOME_SIZES_FILE, "")) } - public File getTargetRegionsFile() { + File getTargetRegionsFile() { return new File (configValues.getString(CVALUE_TARGET_REGIONS_FILE, "")) } - public Integer getTargetSize() { + Integer getTargetSize() { Integer returnValue = configValues.getString(CVALUE_TARGET_SIZE, null) as Integer if (null == returnValue) { returnValue = configValues.getString(CVALUE_TARGETSIZE, null) as Integer @@ -56,29 +68,54 @@ class AlignmentAndQCConfig extends COConfig { return returnValue } - public boolean getRunExomeAnalysis() { - return configValues.getBoolean(COConstants.FLAG_RUN_EXOME_ANALYSIS) + boolean getRunExomeAnalysis() { + return configValues.getBoolean("runExomeAnalysis") } - public File getCytosinePositionIndex() { + File getCytosinePositionIndex() { return new File(configValues.getString(CVALUE_CYTOSINE_POSITIONS_INDEX)) } - public File getClipIndex() { + File getClipIndex() { return new File(configValues.getString(CVALUE_CLIP_INDEX)) } - public boolean getRunFingerprinting() { + boolean getRunFingerprinting() { return configValues.getBoolean(CVALUE_RUN_FINGERPRINTING, true) } - public File getFingerprintingSitesFile() { + File getFingerprintingSitesFile() { return new File(configValues.getString(CVALUE_FINGERPRINTING_SITES_FILE)) } - public Boolean getUseOnlyExistingPairedBams() { - return configValues.getBoolean(COConstants.FLAG_USE_EXISTING_PAIRED_BAMS, false); + boolean getRunFastqcOnly() { + return configValues.getBoolean("runFastQCOnly", false) + } + + boolean getRunFastqc() { + return configValues.getBoolean("runFastQC", true) } + boolean getRunAlignmentOnly() { + return configValues.getBoolean("runAlignmentOnly", false) + } + + boolean getRunCoveragePlots() { + return configValues.getBoolean("runCoveragePlots", true) + } + + boolean getRunCollectBamFileMetrics() { + return configValues.getBoolean("runCollectBamFileMetrics", false) + } + + @Deprecated + boolean getUseCombinedAlignAndSampe() { + return true + } + + @Deprecated + boolean getRunSlimWorkflow() { + return true + } } diff --git a/src/de/dkfz/b080/co/common/COProjectsRuntimeService.groovy b/src/de/dkfz/b080/co/common/COProjectsRuntimeService.groovy index b78e0f8..0fd6b61 100644 --- a/src/de/dkfz/b080/co/common/COProjectsRuntimeService.groovy +++ b/src/de/dkfz/b080/co/common/COProjectsRuntimeService.groovy @@ -12,14 +12,13 @@ import de.dkfz.roddy.execution.io.ExecutionResult; import de.dkfz.roddy.execution.io.ExecutionService; import de.dkfz.roddy.execution.io.fs.FileSystemAccessProvider import de.dkfz.roddy.tools.LoggerWrapper +import groovy.transform.CompileStatic import java.util.function.Consumer; -/** - * Created by heinold on 15.01.16. - */ -@groovy.transform.CompileStatic -public class COProjectsRuntimeService extends BasicCOProjectsRuntimeService { + +@CompileStatic +class COProjectsRuntimeService extends BasicCOProjectsRuntimeService { private static LoggerWrapper logger = LoggerWrapper.getLogger(BasicCOProjectsRuntimeService.class.getName()); protected static void getFileCompression(ExecutionContext run, List allLaneFiles) { @@ -174,6 +173,14 @@ public class COProjectsRuntimeService extends BasicCOProjectsRuntimeService { return laneFiles } + protected static int indexOfPathElement(String pathnamePattern, String element) { + int index = pathnamePattern.split(StringConstants.SPLIT_SLASH).findIndexOf { it -> it == element } + if (index < 0) { + throw new RuntimeException("Couldn't match '${element}' in '${pathnamePattern}") + } + return index + } + public List getLaneFileGroupsFromFastqList(ExecutionContext context, Sample sample, String libraryID) { COConfig coConfig = new COConfig(context) List fastqFiles = coConfig.getFastqList().collect { String it -> new File(it); } @@ -238,11 +245,22 @@ public class COProjectsRuntimeService extends BasicCOProjectsRuntimeService { runIndex = 2; sampleName = split[0..1].join(StringConstants.UNDERSCORE); } - String run = split[runIndex..-2].join(StringConstants.UNDERSCORE); - String lane = String.format("L%03d", laneID); - - - BamFile bamFile = COBaseFile.constructSourceFile(BamFile, f, context, new COFileStageSettings(lane, run, sample, context.getDataSet())) as BamFile + RunID run = new RunID(split[runIndex..-2].join(StringConstants.UNDERSCORE)) + LaneID lane = new LaneID(String.format("L%03d", laneID)) + + + BamFile bamFile = + COBaseFile.constructSourceFile( + BamFile, + f, + context, + new COFileStageSettings( + lane, + run, + (LibraryID) null, + sample, + context.dataSet) + ) as BamFile return bamFile; }) BamFileGroup bamFileGroup = new BamFileGroup(bamFiles); diff --git a/src/de/dkfz/b080/co/files/AlignedSequenceFileGroup.java b/src/de/dkfz/b080/co/files/AlignedSequenceFileGroup.java index d96d0e9..811e664 100644 --- a/src/de/dkfz/b080/co/files/AlignedSequenceFileGroup.java +++ b/src/de/dkfz/b080/co/files/AlignedSequenceFileGroup.java @@ -1,5 +1,7 @@ package de.dkfz.b080.co.files; +import de.dkfz.b080.co.common.LaneID; +import de.dkfz.b080.co.common.RunID; import de.dkfz.roddy.config.Configuration; import de.dkfz.roddy.core.ExecutionContext; import de.dkfz.roddy.execution.jobs.BEJobResult; @@ -35,15 +37,16 @@ public BamFile pairAndSortSlim() { String libString = configuration.getConfigurationValues().getString(COConstants.PRM_CVAL_LIBRARY); String sampleName = laneFile0.getSample().getName(); String pid = context.getDataSet().getId(); - String run = laneFile0.getRunID(); - String lane = laneFile0.getLaneId(); + RunID run = laneFile0.getRunID(); + LaneID lane = laneFile0.getLaneId(); String lb = sampleName + "_" + pid + (libString.equals("addToOldLib") ? "" : "_lib2"); String laneId0 = "RAW_SEQ_FILE_1_INDEX=" + ((COFileStageSettings) laneFile0.getFileStage()).getNumericIndex(); String laneId1 = "RAW_SEQ_FILE_2_INDEX=" + ((COFileStageSettings) laneFile1.getFileStage()).getNumericIndex(); final String TOOL = "sampesortSlim"; - BamFile bamFile = GenericMethod.callGenericTool(TOOL, seqFile0, seqFile1, laneFile0, laneFile1, "SAMPLE=" + sampleName, "RUN=" + run, "LANE=" + lane, "LB=" + lb, laneId0, laneId1); + BamFile bamFile = GenericMethod.callGenericTool(TOOL, seqFile0, seqFile1, laneFile0, laneFile1, + "SAMPLE=" + sampleName, "RUN=" + run.toString(), "LANE=" + lane.toString(), "LB=" + lb, laneId0, laneId1); return bamFile; } @@ -78,7 +81,7 @@ public BamFile pairAndSort() { parameters.put(COConstants.PRM_RAW_SEQ_2, laneFile1.getAbsolutePath()); // Could be moved to the config / scripts - parameters.put("ID", parentFile.getRunID() + "_" + parentFile.getLaneId()); + parameters.put("ID", parentFile.getRunID().toString() + "_" + parentFile.getLaneId().toString()); parameters.put("SM", "sample_" + parentFile.getSample().getName() + "_" + context.getDataSet()); parameters.put("LB", parentFile.getSample().getName() + "_" + context.getDataSet() + (libString.equals("addToOldLib") ? "" : "_lib2")); @@ -90,7 +93,14 @@ public BamFile pairAndSort() { List parentFiles = new LinkedList(); parentFiles.addAll(filesInGroup); - Job job = new Job(context, context.createJobName(parentFiles.get(0), COConstants.TOOL_SAMPESORT, true), COConstants.TOOL_SAMPESORT, null, parameters, parentFiles, Arrays.asList((BaseFile)bamFile, indexFile, flagstatsFile)); + Job job = new Job( + context, + context.createJobName(parentFiles.get(0), COConstants.TOOL_SAMPESORT, true), + COConstants.TOOL_SAMPESORT, + (List) null, + parameters, + parentFiles, + Arrays.asList((BaseFile)bamFile, indexFile, flagstatsFile)); BEJobResult jobResult = job.run(); flagstatsFile.setCreatingJobsResult(jobResult); diff --git a/src/de/dkfz/b080/co/files/LaneFile.java b/src/de/dkfz/b080/co/files/LaneFile.java index e415669..aed6b85 100644 --- a/src/de/dkfz/b080/co/files/LaneFile.java +++ b/src/de/dkfz/b080/co/files/LaneFile.java @@ -1,5 +1,8 @@ package de.dkfz.b080.co.files; +import de.dkfz.b080.co.common.IndexID; +import de.dkfz.b080.co.common.LaneID; +import de.dkfz.b080.co.common.RunID; import de.dkfz.roddy.config.Configuration; import de.dkfz.roddy.core.ExecutionContext; import de.dkfz.roddy.execution.io.ExecutionResult; @@ -34,7 +37,7 @@ public class LaneFile extends COBaseFile implements ITestdataSource { private FastqcFile fastqcFile; private AlignedSequenceFile alignedSequenceFile; - public LaneFile(ConstructionHelperForBaseFiles helper) { + public LaneFile(BaseFile.ConstructionHelperForBaseFiles helper) { super(helper); } @@ -137,15 +140,15 @@ public AlignedSequenceFile getAlignedSequenceFile() { return alignedSequenceFile; } - public String getLaneId() { - return ((COFileStageSettings) fileStageSettings).getLaneId(); + public LaneID getLaneId() { + return ((COFileStageSettings) fileStageSettings).getLaneID(); } - public String getIndex() { + public IndexID getIndex() { return ((COFileStageSettings) fileStageSettings).getIndex(); } - public String getRunID() { + public RunID getRunID() { return ((COFileStageSettings) fileStageSettings).getRunID(); } @@ -215,7 +218,7 @@ public boolean createTestData() { String cmd = String.format(getDecompressionString() + " %s | head -n %s | " + recompressionString + " > %s", filePath, testLineCnt, targetFilePath); logger.log(Level.INFO, cmd); ExecutionResult er = es.execute(cmd); - if (!er.successful) { + if (!er.isSuccessful()) { logger.severe("Could not create testdata for file " + targetFilePath); } diff --git a/src/de/dkfz/b080/co/files/LaneFileGroup.java b/src/de/dkfz/b080/co/files/LaneFileGroup.java index d6d19a2..73d8188 100644 --- a/src/de/dkfz/b080/co/files/LaneFileGroup.java +++ b/src/de/dkfz/b080/co/files/LaneFileGroup.java @@ -1,5 +1,7 @@ package de.dkfz.b080.co.files; +import de.dkfz.b080.co.common.LaneID; +import de.dkfz.b080.co.common.RunID; import de.dkfz.roddy.config.Configuration; import de.dkfz.roddy.core.ExecutionContext; import de.dkfz.roddy.execution.io.ExecutionResult; @@ -33,7 +35,11 @@ public class LaneFileGroup extends FileGroup { private FastqcGroup allFastqcFiles; private AlignedSequenceFileGroup allAlignedFiles; - public LaneFileGroup(ExecutionContext executionContext, String id, String run, Sample sample, List files) { + public LaneFileGroup(ExecutionContext executionContext, + String id, + String run, + Sample sample, + List files) { super(files); this.id = id; this.run = run; @@ -68,7 +74,9 @@ public Sample getSample() { //This method could be a candidate for public FastqcGroup calcFastqcForAll() { - final boolean useSingleEndProcessing = getExecutionContext().getConfiguration().getConfigurationValues().getBoolean(COConstants.FLAG_USE_SINGLE_END_PROCESSING, false); + final boolean useSingleEndProcessing = + getExecutionContext().getConfiguration().getConfigurationValues(). + getBoolean(COConstants.FLAG_USE_SINGLE_END_PROCESSING, false); LinkedList files = new LinkedList(); for (LaneFile f : filesInGroup) { @@ -85,7 +93,9 @@ public FastqcGroup calcFastqcForAll() { } public AlignedSequenceFileGroup alignAll() { - final boolean useSingleEndProcessing = getExecutionContext().getConfiguration().getConfigurationValues().getBoolean(COConstants.FLAG_USE_SINGLE_END_PROCESSING, false); + final boolean useSingleEndProcessing = + getExecutionContext().getConfiguration().getConfigurationValues(). + getBoolean(COConstants.FLAG_USE_SINGLE_END_PROCESSING, false); LinkedList files = new LinkedList(); int i = 0; for (LaneFile f : filesInGroup) { @@ -108,37 +118,44 @@ public AlignedSequenceFileGroup alignAll() { public BamFile alignAndPairSlim() { ExecutionContext context = getExecutionContext(); Configuration configuration = context.getConfiguration(); - boolean useAcceleratedHardware = configuration.getConfigurationValues().getBoolean(COConstants.FLAG_USE_ACCELERATED_HARDWARE); + boolean useAcceleratedHardware = + configuration.getConfigurationValues(). + getBoolean(COConstants.FLAG_USE_ACCELERATED_HARDWARE); // Bad hack: Decrease the file stage level by one! LaneFile laneFile0 = filesInGroup.get(0).getFSDecreasedCopy(); LaneFile laneFile1 = filesInGroup.get(1).getFSDecreasedCopy(); - String libString = configuration.getConfigurationValues().getString(COConstants.PRM_CVAL_LIBRARY); + String libString = + configuration.getConfigurationValues(). + getString(COConstants.PRM_CVAL_LIBRARY); String sampleName = laneFile0.getSample().getName(); String pid = context.getDataSet().getId(); - String run = laneFile0.getRunID(); - String lane = laneFile0.getLaneId(); + RunID run = laneFile0.getRunID(); + LaneID lane = laneFile0.getLaneId(); String lb = sampleName + "_" + pid + (libString.equals("addToOldLib") ? "" : "_lib2"); String laneId0 = "RAW_SEQ_FILE_1_INDEX=" + ((COFileStageSettings) laneFile0.getFileStage()).getNumericIndex(); String laneId1 = "RAW_SEQ_FILE_2_INDEX=" + ((COFileStageSettings) laneFile1.getFileStage()).getNumericIndex(); - final String TOOL = useAcceleratedHardware ? COConstants.TOOL_ACCELERATED_ALIGNANDPAIR_SLIM : COConstants.TOOL_ALIGNANDPAIR_SLIM; + final String TOOL = + useAcceleratedHardware ? + COConstants.TOOL_ACCELERATED_ALIGNANDPAIR_SLIM : + COConstants.TOOL_ALIGNANDPAIR_SLIM; BamFile bamFile = GenericMethod.callGenericTool(TOOL, laneFile0, laneFile1, "SAMPLE=" + sampleName, "sample=" + sampleName, - "RUN=" + run, "run=" + run, - "LANE=" + lane, "lane=" + lane, + "RUN=" + run.toString(), "run=" + run.toString(), + "LANE=" + lane.toString(), "lane=" + lane.toString(), "LB=" + lb, laneId0, laneId1); return bamFile; } public BamFile alignAndPair() { - ExecutionContext run = getExecutionContext(); + ExecutionContext runContext = getExecutionContext(); LaneFile laneFile0 = filesInGroup.get(0); LaneFile laneFile1 = filesInGroup.get(1); - Configuration configuration = run.getConfiguration(); + Configuration configuration = runContext.getConfiguration(); BamFile bamFile = (BamFile) BaseFile.constructManual(BamFile.class, this); FlagstatsFile flagstatsFile = (FlagstatsFile)BaseFile.constructManual(FlagstatsFile.class, bamFile); BamIndexFile bamIndexFile = (BamIndexFile) BaseFile.constructManual(BamIndexFile.class, bamFile); @@ -152,15 +169,15 @@ public BamFile alignAndPair() { final String TOOL = useAcceleratedHardware ? COConstants.TOOL_ACCELERATED_ALIGNANDPAIR : COConstants.TOOL_ALIGNANDPAIR; String sampleName = laneFile0.getSample().getName(); - String pid = run.getDataSet().getId(); + String pid = runContext.getDataSet().getId(); - Map parameters = run.getDefaultJobParameters(TOOL); + Map parameters = runContext.getDefaultJobParameters(TOOL); parameters.put(COConstants.PRM_FILENAME_SORTED_BAM, bamFile.getAbsolutePath()); parameters.put(COConstants.PRM_FILENAME_FLAGSTAT, flagstatsFile.getAbsolutePath()); parameters.put(COConstants.PRM_FILENAME_BAM_INDEX, bamIndexFile.getAbsolutePath()); parameters.put(COConstants.PRM_RAW_SEQ_1, laneFile0.getAbsolutePath()); parameters.put(COConstants.PRM_RAW_SEQ_2, laneFile1.getAbsolutePath()); - parameters.put(COConstants.PRM_ID, laneFile0.getRunID() + "_" + laneFile0.getLaneId()); + parameters.put(COConstants.PRM_ID, laneFile0.getRunID().toString() + "_" + laneFile0.getLaneId().toString()); parameters.put(COConstants.PRM_SM, "sample_" + sampleName + "_" + pid); parameters.put(COConstants.PRM_LB, sampleName + "_" + pid + (libString.equals("addToOldLib") ? "" : "_lib2")); @@ -171,7 +188,14 @@ public BamFile alignAndPair() { List parentFiles = new LinkedList<>(); parentFiles.addAll(filesInGroup); - Job job = new Job(run, run.createJobName(parentFiles.get(0), TOOL, true), TOOL, null, parameters, parentFiles, Arrays.asList((BaseFile) bamFile, flagstatsFile)); + Job job = new Job( + runContext, + runContext.createJobName(parentFiles.get(0), TOOL,true), + TOOL, + (List) null, + parameters, + parentFiles, + Arrays.asList((BaseFile) bamFile, flagstatsFile)); BEJobResult jobResult = job.run(); flagstatsFile.setCreatingJobsResult(jobResult); if (indexCreated) bamIndexFile.setCreatingJobsResult(jobResult); @@ -222,7 +246,7 @@ public void createTestDataForLaneFiles() { fullCommandList.append(" wait"); logger.log(Level.INFO, fullCommandList.toString()); ExecutionResult er = es.execute(fullCommandList.toString()); - if (!er.successful) { + if (!er.isSuccessful()) { logger.severe("Could not create testdata for one or more files."); } } diff --git a/src/de/dkfz/b080/co/methods/Common.groovy b/src/de/dkfz/b080/co/methods/Common.groovy index be48b17..bfe636a 100644 --- a/src/de/dkfz/b080/co/methods/Common.groovy +++ b/src/de/dkfz/b080/co/methods/Common.groovy @@ -83,7 +83,7 @@ class Common { if (bamFileFileStage.stage.isMoreDetailedOrEqualTo(COFileStage.RUN)) { runId = bamFileFileStage.runID; if (bamFileFileStage.stage.isMoreDetailedOrEqualTo(COFileStage.LANE)) - lane = bamFileFileStage.laneId; + lane = bamFileFileStage.laneID; } def temp = run.getDefaultJobParameters(QCSUMMARY); diff --git a/src/de/dkfz/b080/co/methods/Samtools.groovy b/src/de/dkfz/b080/co/methods/Samtools.groovy index 3ddb7a3..12bd286 100644 --- a/src/de/dkfz/b080/co/methods/Samtools.groovy +++ b/src/de/dkfz/b080/co/methods/Samtools.groovy @@ -9,12 +9,13 @@ import de.dkfz.roddy.execution.jobs.ScriptCallingMethod import de.dkfz.roddy.execution.jobs.StaticScriptProviderClass import de.dkfz.roddy.knowledge.files.BaseFile import de.dkfz.roddy.tools.LoggerWrapper +import groovy.transform.CompileStatic /** * * @author michael */ -@groovy.transform.CompileStatic +@CompileStatic @StaticScriptProviderClass class Samtools { diff --git a/src/de/dkfz/b080/co/qcworkflow/BisulfiteCoreWorkflow.groovy b/src/de/dkfz/b080/co/qcworkflow/BisulfiteCoreWorkflow.groovy index 8a57aff..487ffc7 100644 --- a/src/de/dkfz/b080/co/qcworkflow/BisulfiteCoreWorkflow.groovy +++ b/src/de/dkfz/b080/co/qcworkflow/BisulfiteCoreWorkflow.groovy @@ -1,87 +1,68 @@ package de.dkfz.b080.co.qcworkflow import de.dkfz.b080.co.common.AlignmentAndQCConfig -import de.dkfz.b080.co.common.BasicCOProjectsRuntimeService import de.dkfz.b080.co.common.COProjectsRuntimeService import de.dkfz.b080.co.files.* -import de.dkfz.roddy.config.Configuration -import de.dkfz.roddy.config.RecursiveOverridableMapContainerForConfigurationValues -import de.dkfz.roddy.core.DataSet import de.dkfz.roddy.core.ExecutionContext import de.dkfz.roddy.core.ExecutionContextError import de.dkfz.roddy.execution.io.ExecutionService import de.dkfz.roddy.execution.io.fs.FileSystemAccessProvider - -import static de.dkfz.b080.co.files.COConstants.FLAG_EXTRACT_SAMPLES_FROM_OUTPUT_FILES +import groovy.transform.CompileStatic /** * @author michael */ -@groovy.transform.CompileStatic -public class BisulfiteCoreWorkflow extends QCPipeline { +@CompileStatic +class BisulfiteCoreWorkflow extends QCPipeline { @Override - public boolean execute(ExecutionContext context) { - Configuration cfg = context.getConfiguration(); - RecursiveOverridableMapContainerForConfigurationValues cfgValues = cfg.getConfigurationValues(); - cfgValues.put(FLAG_EXTRACT_SAMPLES_FROM_OUTPUT_FILES, "false", "boolean"); //Disable sample extraction from output for alignment workflows. - //cfgValues.put(COConstants.FLAG_USE_ACCELERATED_HARDWARE, "false", "boolean"); //Disable accelerated hardware usage for testing - - // Run flags - final boolean runFastQCOnly = cfgValues.getBoolean(COConstants.FLAG_RUN_FASTQC_ONLY, false) - final boolean runFastQC = cfgValues.getBoolean(COConstants.FLAG_RUN_FASTQC, true) - final boolean runAlignmentOnly = cfgValues.getBoolean(COConstants.FLAG_RUN_ALIGNMENT_ONLY, false); - final boolean runCoveragePlots = cfgValues.getBoolean(COConstants.FLAG_RUN_COVERAGE_PLOTS, true); - final boolean runCollectBamFileMetrics = cfgValues.getBoolean(COConstants.FLAG_RUN_COLLECT_BAMFILE_METRICS, false); + boolean execute(ExecutionContext context) { + AlignmentAndQCConfig cfg = new AlignmentAndQCConfig(context) + cfg.setUseOnlyExistingTargetBam(true) - COProjectsRuntimeService runtimeService = (COProjectsRuntimeService) context.getRuntimeService(); - - List samples = runtimeService.getSamplesForContext(context); + COProjectsRuntimeService runtimeService = (COProjectsRuntimeService) context.runtimeService + List samples = runtimeService.getSamplesForContext(context) - BamFileGroup mergedBamFiles = new BamFileGroup(); + BamFileGroup mergedBamFiles = new BamFileGroup() Map coverageTextFilesBySample = [:] for (Sample sample in samples) { - - List availableLibrariesForSample = sample.getLibraries(); - BamFileGroup mergedBamsPerLibrary = new BamFileGroup(); + BamFileGroup mergedBamsPerLibrary = new BamFileGroup() // Create per library merged bams - for (String library in availableLibrariesForSample) { + for (String library in sample.libraries) { BamFileGroup sortedBamFiles = [] - List rawSequenceGroups = runtimeService.loadLaneFilesForSampleAndLibrary(context, sample, library) + List rawSequenceGroups = runtimeService. + loadLaneFilesForSampleAndLibrary(context, sample, library) if (rawSequenceGroups == null || rawSequenceGroups.size() > 0) { for (LaneFileGroup rawSequenceGroup : rawSequenceGroups) { - if (runFastQC && !runAlignmentOnly) - rawSequenceGroup.calcFastqcForAll(); - if (runFastQCOnly) - continue; - + if (cfg.runFastqc && !cfg.runAlignmentOnly) + rawSequenceGroup.calcFastqcForAll() + if (cfg.runFastqcOnly) + continue - BamFile bamFile = rawSequenceGroup.alignAndPairSlim(); - //rawSequenceGroup.alignAndPairSlim() + BamFile bamFile = rawSequenceGroup.alignAndPairSlim() - bamFile.setAsTemporaryFile(); // Bam files created with sai files are only temporary. - sortedBamFiles.addFile(bamFile); + bamFile.setAsTemporaryFile() // Bam files created with sai files are only temporary. + sortedBamFiles.addFile(bamFile) } } - if (!sortedBamFiles.getFilesInGroup()) continue; + if (!sortedBamFiles.filesInGroup) continue - if (runAlignmentOnly) continue; + if (cfg.runAlignmentOnly) continue - BamFile mergedLibraryBam; - if (availableLibrariesForSample.size() == 1) { - mergedLibraryBam = sortedBamFiles.mergeAndRemoveDuplicatesSlim(sample); - if (runCollectBamFileMetrics) mergedLibraryBam.collectMetrics(); + BamFile mergedLibraryBam + if (sample.libraries.size() == 1) { + mergedLibraryBam = sortedBamFiles.mergeAndRemoveDuplicatesSlim(sample) + if (cfg.runCollectBamFileMetrics) mergedLibraryBam.collectMetrics() - Sample.SampleType sampleType = sample.getType(); - if (!coverageTextFilesBySample.containsKey(sampleType)) - coverageTextFilesBySample.put(sampleType, new CoverageTextFileGroup()); - coverageTextFilesBySample.get(sampleType).addFile(mergedLibraryBam.calcReadBinsCoverage()); + if (!coverageTextFilesBySample.containsKey(sample.type)) + coverageTextFilesBySample.put(sample.type, new CoverageTextFileGroup()) + coverageTextFilesBySample.get(sample.type).addFile(mergedLibraryBam.calcReadBinsCoverage()) mergedBamFiles.addFile(mergedLibraryBam); // Unfortunately, due to the way Roddy works, the following call needs to be encapsulated into @@ -101,38 +82,39 @@ public class BisulfiteCoreWorkflow extends QCPipeline { } // Merge library bams into per sample bams - if(availableLibrariesForSample.size() > 1) { - BamFile mergedBam = mergedBamsPerLibrary.mergeSlim(sample); + if(sample.libraries.size() > 1) { + BamFile mergedBam = mergedBamsPerLibrary.mergeSlim(sample) // Unfortunately, due to the way Roddy works, the following call needs to be encapsulated into // a method, in order to put library and merged methylation results into different directories. // This allows for selection via onMethod="BisulfiteCoreWorkflow.mergedMethylationCallingMeta". mergedBam.mergedMethylationCallingMeta() - if (runCollectBamFileMetrics) mergedBam.collectMetrics(); + if (cfg.runCollectBamFileMetrics) mergedBam.collectMetrics() - Sample.SampleType sampleType = sample.getType(); - if (!coverageTextFilesBySample.containsKey(sampleType)) - coverageTextFilesBySample.put(sampleType, new CoverageTextFileGroup()); - coverageTextFilesBySample.get(sampleType).addFile(mergedBam.calcReadBinsCoverage()); + if (!coverageTextFilesBySample.containsKey(sample.type)) + coverageTextFilesBySample.put(sample.type, new CoverageTextFileGroup()) + coverageTextFilesBySample.get(sample.type).addFile(mergedBam.calcReadBinsCoverage()) - mergedBamFiles.addFile(mergedBam); + mergedBamFiles.addFile(mergedBam) } } - if (!mergedBamFiles.getFilesInGroup()) { - context.addErrorEntry(ExecutionContextError.EXECUTION_NOINPUTDATA.expand("There were no merged bam files available.")); + if (!mergedBamFiles.filesInGroup) { + context.addErrorEntry(ExecutionContextError.EXECUTION_NOINPUTDATA. + expand("There were no merged bam files available.")) return false; } - if (runCoveragePlots && coverageTextFilesBySample.keySet().size() >= 2) { - coverageTextFilesBySample.get(Sample.SampleType.CONTROL).plotAgainst(coverageTextFilesBySample.get(Sample.SampleType.TUMOR)); - } else if (runCoveragePlots && coverageTextFilesBySample.keySet().size() == 1) { + if (cfg.runCoveragePlots && coverageTextFilesBySample.keySet().size() >= 2) { + coverageTextFilesBySample.get(Sample.SampleType.CONTROL). + plotAgainst(coverageTextFilesBySample.get(Sample.SampleType.TUMOR)); + } else if (cfg.runCoveragePlots && coverageTextFilesBySample.keySet().size() == 1) { //TODO: Think if this conflicts with plotAgainst on rerun! Maybe missing files are not recognized. - ((CoverageTextFileGroup) coverageTextFilesBySample.values().toArray()[0]).plot(); + ((CoverageTextFileGroup) coverageTextFilesBySample.values().toArray()[0]).plot() } - return true; + return true } @@ -144,18 +126,18 @@ public class BisulfiteCoreWorkflow extends QCPipeline { List samples = runtimeService.getSamplesForContext(context) for (Sample sample : samples) { LinkedHashMap laneFileGroups = [:] - for (String lib : sample.getLibraries()) { + for (String lib : sample.libraries) { for (LaneFileGroup group : runtimeService.loadLaneFilesForSampleAndLibrary(context, sample, lib)) { - String key = group.getRun() + " " + group.getId() + String key = group.run + " " + group.id if (laneFileGroups.containsKey(key)) { context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. - expand("Duplicate lane identifiers for ${group.getRun()}_${group.getId()} among libraries " + - "(pid=${context.getDataSet()}, sample=${sample.getName()})! Check run and FASTQ names.")) + expand("Duplicate lane identifiers for ${group.run}_${group.id} among libraries " + + "(pid=${context.dataSet}, sample=${sample.name})! Check run and FASTQ names.")) returnValue = false } else { laneFileGroups[key] = group } - cnt += group.getFilesInGroup().size() + cnt += group.filesInGroup.size() } } } @@ -169,14 +151,16 @@ public class BisulfiteCoreWorkflow extends QCPipeline { protected boolean checkCytosinePositionIndex(ExecutionContext context) { AlignmentAndQCConfig aqcfg = new AlignmentAndQCConfig(context) - FileSystemAccessProvider accessProvider = FileSystemAccessProvider.getInstance() - File cposidx = aqcfg.getCytosinePositionIndex() + FileSystemAccessProvider accessProvider = FileSystemAccessProvider.instance + File cposidx = aqcfg.cytosinePositionIndex if (cposidx.toString().equals("")) { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("${AlignmentAndQCConfig.CVALUE_CYTOSINE_POSITIONS_INDEX} is not defined!")) + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. + expand("${AlignmentAndQCConfig.CVALUE_CYTOSINE_POSITIONS_INDEX} is not defined!")) return false } else if (!accessProvider.fileExists(cposidx) || !accessProvider.isReadable(cposidx)) { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("Cytosine position index '${cposidx}' is not accessible!")) + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. + expand("Cytosine position index '${cposidx}' is not accessible!")) return false } else { return true @@ -186,10 +170,11 @@ public class BisulfiteCoreWorkflow extends QCPipeline { protected boolean checkClipIndex(ExecutionContext context) { AlignmentAndQCConfig aqcfg = new AlignmentAndQCConfig(context) - FileSystemAccessProvider accessProvider = FileSystemAccessProvider.getInstance() - if (!aqcfg.getClipIndex().toString().startsWith('$' + ExecutionService.RODDY_CVALUE_DIRECTORY_EXECUTION) - && !accessProvider.isReadable(aqcfg.getClipIndex())) { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("Clip index '${aqcfg.getClipIndex()}' is not accessible!")); + FileSystemAccessProvider accessProvider = FileSystemAccessProvider.instance + if (!aqcfg.clipIndex.toString().startsWith('$' + ExecutionService.RODDY_CVALUE_DIRECTORY_EXECUTION) + && !accessProvider.isReadable(aqcfg.clipIndex)) { + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. + expand("Clip index '${aqcfg.clipIndex}' is not accessible!")) return false } else { return true @@ -197,7 +182,7 @@ public class BisulfiteCoreWorkflow extends QCPipeline { } @Override - public boolean checkExecutability(ExecutionContext context) { + boolean checkExecutability(ExecutionContext context) { boolean result = super.checkExecutability(context) result &= checkCytosinePositionIndex(context) result &= checkClipIndex(context) diff --git a/src/de/dkfz/b080/co/qcworkflow/PostMergeQCAnalysisWorkflow.java b/src/de/dkfz/b080/co/qcworkflow/PostMergeQCAnalysisWorkflow.java index ca5708e..167fe32 100644 --- a/src/de/dkfz/b080/co/qcworkflow/PostMergeQCAnalysisWorkflow.java +++ b/src/de/dkfz/b080/co/qcworkflow/PostMergeQCAnalysisWorkflow.java @@ -1,10 +1,11 @@ package de.dkfz.b080.co.qcworkflow; -import de.dkfz.b080.co.common.BasicCOProjectsRuntimeService; +import de.dkfz.b080.co.common.AlignmentAndQCConfig; import de.dkfz.b080.co.common.COProjectsRuntimeService; -import de.dkfz.b080.co.files.*; -import de.dkfz.roddy.config.Configuration; -import de.dkfz.roddy.config.RecursiveOverridableMapContainerForConfigurationValues; +import de.dkfz.b080.co.files.BamFile; +import de.dkfz.b080.co.files.BamFileGroup; +import de.dkfz.b080.co.files.CoverageTextFileGroup; +import de.dkfz.b080.co.files.Sample; import de.dkfz.roddy.core.ExecutionContext; import de.dkfz.roddy.core.ExecutionContextError; import de.dkfz.roddy.core.Workflow; @@ -13,9 +14,6 @@ import java.util.List; import java.util.Map; -import static de.dkfz.b080.co.files.COConstants.FLAG_EXTRACT_SAMPLES_FROM_OUTPUT_FILES; -import static de.dkfz.b080.co.files.COConstants.FLAG_USE_EXISTING_MERGED_BAMS; - /** * A short workflow which only does post merge alignment quality control */ @@ -25,16 +23,12 @@ public PostMergeQCAnalysisWorkflow() {} @Override public boolean execute(ExecutionContext context) { - Configuration cfg = context.getConfiguration(); - RecursiveOverridableMapContainerForConfigurationValues cfgValues = cfg.getConfigurationValues(); - cfgValues.put(FLAG_EXTRACT_SAMPLES_FROM_OUTPUT_FILES, "true", "boolean"); //Enable sample extraction from output for this workflow. - cfgValues.put(FLAG_USE_EXISTING_MERGED_BAMS, "true", "boolean"); //Enable usage of existing merged bams for this workflow. - final boolean runCoveragePlots = cfgValues.getBoolean(COConstants.FLAG_RUN_COVERAGE_PLOTS, true); - final boolean runExomeAnalysis = cfgValues.getBoolean(COConstants.FLAG_RUN_EXOME_ANALYSIS); + AlignmentAndQCConfig cfg = new AlignmentAndQCConfig(context); + cfg.setExtractSamplesFromOutputFiles(true); // Enable sample extraction from output for this workflow. + cfg.setUseOnlyExistingTargetBam(true); // Enable usage of existing merged bams for this workflow. COProjectsRuntimeService runtimeService = (COProjectsRuntimeService) context.getRuntimeService(); - - List samples = runtimeService.getSamplesForContext(context); + List samples = runtimeService.metadataAccessor.getSamples(context); if (samples.size() == 0) return false; @@ -42,10 +36,11 @@ public boolean execute(ExecutionContext context) { Map coverageTextFilesBySample = new LinkedHashMap<>(); for (Sample sample : samples) { - BamFile mergedBam = new BamFile(runtimeService.getMergedBamFileForDataSetAndSample(context, sample)); + BamFile mergedBam = new BamFile(runtimeService.metadataAccessor. + getMergedBamFileFromFilesystem(context, null, sample)); mergedBam.performPostMergeQCAnalysis(); - if (runExomeAnalysis) { + if (cfg.getRunExomeAnalysis()) { mergedBam.extractTargetsCalculateCoverage(); } @@ -62,7 +57,7 @@ public boolean execute(ExecutionContext context) { return false; } - if (runCoveragePlots && coverageTextFilesBySample.keySet().size() >= 2) { + if (cfg.getRunCoveragePlots() && coverageTextFilesBySample.keySet().size() >= 2) { coverageTextFilesBySample.get(Sample.SampleType.CONTROL).plotAgainst(coverageTextFilesBySample.get(Sample.SampleType.TUMOR)); } else if (coverageTextFilesBySample.keySet().size() == 1) { ((CoverageTextFileGroup) coverageTextFilesBySample.values().toArray()[0]).plot(); diff --git a/src/de/dkfz/b080/co/qcworkflow/QCPipeline.groovy b/src/de/dkfz/b080/co/qcworkflow/QCPipeline.groovy index 3ace2ed..2496958 100644 --- a/src/de/dkfz/b080/co/qcworkflow/QCPipeline.groovy +++ b/src/de/dkfz/b080/co/qcworkflow/QCPipeline.groovy @@ -34,7 +34,7 @@ public class QCPipeline extends Workflow { final boolean runFastQCOnly = cfgValues.getBoolean(COConstants.FLAG_RUN_FASTQC_ONLY, false); final boolean runAlignmentOnly = cfgValues.getBoolean(COConstants.FLAG_RUN_ALIGNMENT_ONLY, false); final boolean runCoveragePlots = cfgValues.getBoolean(COConstants.FLAG_RUN_COVERAGE_PLOTS, true); - final boolean runSlimWorkflow = cfgValues.getBoolean(COConstants.FLAG_RUN_SLIM_WORKFLOW, false); + final boolean runSlimWorkflow = true final boolean runExomeAnalysis = cfgValues.getBoolean(COConstants.FLAG_RUN_EXOME_ANALYSIS); final boolean runCollectBamFileMetrics = cfgValues.getBoolean(COConstants.FLAG_RUN_COLLECT_BAMFILE_METRICS, false); @@ -94,22 +94,10 @@ public class QCPipeline extends Workflow { } private BamFileGroup createSortedBams(ExecutionContext context, COProjectsRuntimeService runtimeService, Sample sample) { - Configuration cfg = context.getConfiguration(); - - RecursiveOverridableMapContainerForConfigurationValues cfgValues = cfg.getConfigurationValues(); - // Run flags - final boolean runFastQCOnly = cfgValues.getBoolean(COConstants.FLAG_RUN_FASTQC_ONLY, false); - final boolean runFastQC = cfgValues.getBoolean(COConstants.FLAG_RUN_FASTQC, true); - final boolean runAlignmentOnly = cfgValues.getBoolean(COConstants.FLAG_RUN_ALIGNMENT_ONLY, false); - - final boolean useOnlyExistingPairedBams = cfgValues.getBoolean(COConstants.FLAG_USE_EXISTING_PAIRED_BAMS, false); - // Usage flags - final boolean useCombinedAlignAndSampe = cfgValues.getBoolean(COConstants.FLAG_USE_COMBINED_ALIGN_AND_SAMPE, false); - final boolean runSlimWorkflow = cfgValues.getBoolean(COConstants.FLAG_RUN_SLIM_WORKFLOW, false); - + AlignmentAndQCConfig cfg = new AlignmentAndQCConfig(context) BamFileGroup sortedBamFiles = new BamFileGroup(); - if (useOnlyExistingPairedBams) { + if (cfg.useOnlyExistingPairedBams) { //Start from the paired bams instead of the lane files. sortedBamFiles = runtimeService.getPairedBamFilesForDataSet(context, sample); } else { @@ -119,23 +107,25 @@ public class QCPipeline extends Workflow { if (rawSequenceGroups == null || rawSequenceGroups.size() == 0) return sortedBamFiles; for (LaneFileGroup rawSequenceGroup : rawSequenceGroups) { - if (runFastQC && !runAlignmentOnly) + if (cfg.runFastqc && !cfg.runAlignmentOnly) rawSequenceGroup.calcFastqcForAll(); - if (runFastQCOnly) + if (cfg.runFastqcOnly) continue; BamFile bamFile = null; + final boolean runSlimWorkflow = true - if (useCombinedAlignAndSampe) { //I.e. bwa mem - if (runSlimWorkflow) { + // TODO Remove all this dead code. + if (cfg.useCombinedAlignAndSampe) { //I.e. bwa mem + if (cfg.runSlimWorkflow) { bamFile = rawSequenceGroup.alignAndPairSlim(); } else { bamFile = rawSequenceGroup.alignAndPair(); } } else { //I.e. bwa align rawSequenceGroup.alignAll(); - if (runSlimWorkflow) { + if (cfg.runSlimWorkflow) { bamFile = rawSequenceGroup.getAllAlignedFiles().pairAndSortSlim(); } else { bamFile = rawSequenceGroup.getAllAlignedFiles().pairAndSort(); @@ -157,7 +147,7 @@ public class QCPipeline extends Workflow { //To avoid problems with qcsummary the step is done manually. sortedBamFiles.runDefaultOperations(); - for (BamFile sortedBam : sortedBamFiles.getFilesInGroup()) + for (BamFile sortedBam : sortedBamFiles.filesInGroup) sortedBam.createQCSummaryFile(); BamFile mergedBam = sortedBamFiles.mergeAndRemoveDuplicates(); @@ -172,23 +162,28 @@ public class QCPipeline extends Workflow { private boolean valueIsEmpty(ExecutionContext context, Object value, String variableName) { if (value == null || value.toString() == "") { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("Expected value to be set: ${variableName}")) + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. + expand("Expected value to be set: ${variableName}")) return true } return false } private boolean fileIsAccessible(ExecutionContext context, File file, String variableName) { - if (valueIsEmpty(context, file, variableName) || !FileSystemAccessProvider.getInstance().checkFile(file, false, context)) { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("File '${file}' not accessible: ${variableName}")) + if (valueIsEmpty(context, file, variableName) || + !FileSystemAccessProvider.instance.checkFile(file, false, context)) { + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. + expand("File '${file}' not accessible: ${variableName}")) return false } return true } private boolean directoryIsAccessible(ExecutionContext context, File directory, String variableName) { - if (valueIsEmpty(context, directory, variableName) || !FileSystemAccessProvider.getInstance().checkDirectory(directory, context, false)) { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("Directory '${directory}' not accessible: ${variableName}")) + if (valueIsEmpty(context, directory, variableName) || + !FileSystemAccessProvider.instance.checkDirectory(directory, context, false)) { + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. + expand("Directory '${directory}' not accessible: ${variableName}")) return false } return true @@ -199,26 +194,31 @@ public class QCPipeline extends Workflow { boolean returnValue returnValue = - !valueIsEmpty(context, config.getIndexPrefix(), AlignmentAndQCConfig.CVALUE_INDEX_PREFIX) && - directoryIsAccessible(context, new File(config.getIndexPrefix()).getParentFile(), AlignmentAndQCConfig.CVALUE_INDEX_PREFIX) - returnValue = - fileIsAccessible(context, config.getChromosomeSizesFile(), AlignmentAndQCConfig.CVALUE_CHROMOSOME_SIZES_FILE) - if (config.getRunExomeAnalysis()) { - returnValue = - fileIsAccessible(context, config.getTargetRegionsFile(), AlignmentAndQCConfig.CVALUE_TARGET_REGIONS_FILE) && - !valueIsEmpty(context, config.getTargetSize(), AlignmentAndQCConfig.CVALUE_TARGET_SIZE) + !valueIsEmpty(context, config.indexPrefix, + AlignmentAndQCConfig.CVALUE_INDEX_PREFIX) && + directoryIsAccessible(context, new File(config.indexPrefix).parentFile, + AlignmentAndQCConfig.CVALUE_INDEX_PREFIX) + returnValue &= + fileIsAccessible(context, config.chromosomeSizesFile, + AlignmentAndQCConfig.CVALUE_CHROMOSOME_SIZES_FILE) + if (config.runExomeAnalysis) { + returnValue &= + fileIsAccessible(context, config.targetRegionsFile, + AlignmentAndQCConfig.CVALUE_TARGET_REGIONS_FILE) && + !valueIsEmpty(context, config.targetSize, + AlignmentAndQCConfig.CVALUE_TARGET_SIZE) } return returnValue } private boolean checkSamples(ExecutionContext context) { - BasicCOProjectsRuntimeService runtimeService = (BasicCOProjectsRuntimeService) context.getRuntimeService() + BasicCOProjectsRuntimeService runtimeService = (BasicCOProjectsRuntimeService) context.runtimeService List samples = runtimeService.getSamplesForContext(context) if (samples.size() == 0) { - context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("No samples found for PID ${context.getDataSet()}!")) + context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID.expand("No samples found for PID ${context.dataSet}!")) return false } else { - logger.postAlwaysInfo("Found " + samples.size() + " samples for dataset " + context.getDataSet().getId()); + logger.postAlwaysInfo("Found " + samples.size() + " samples for dataset " + context.dataSet.id); return true } } @@ -228,7 +228,8 @@ public class QCPipeline extends Workflow { AlignmentAndQCConfig cfg = new AlignmentAndQCConfig(context) - BasicCOProjectsRuntimeService runtimeService = (BasicCOProjectsRuntimeService) context.getRuntimeService() + BasicCOProjectsRuntimeService runtimeService = + (BasicCOProjectsRuntimeService) context.runtimeService List samples = runtimeService.getSamplesForContext(context) if (cfg.useOnlyExistingTargetBam && cfg.fastqFileListIsSet) { @@ -248,13 +249,13 @@ public class QCPipeline extends Workflow { for (Sample sample : samples) { List laneFileGroups = ((COProjectsRuntimeService) runtimeService).loadLaneFilesForSample(context, sample) for (LaneFileGroup lfg : laneFileGroups) { - cnt += lfg.getFilesInGroup().size() + cnt += lfg.filesInGroup.size() } - logger.postAlwaysInfo("Processed sample " + sample.getName() + " and found " + laneFileGroups.size() + " groups of lane files.") + logger.postAlwaysInfo("Processed sample " + sample.name + " and found " + laneFileGroups.size() + " groups of lane files.") } if (cnt <= 0) { context.addErrorEntry(ExecutionContextError.EXECUTION_NOINPUTDATA. - expand("No lane files found for PID ${context.getDataSet()}!")) + expand("No lane files found for PID ${context.dataSet}!")) returnValue = false } } @@ -284,7 +285,7 @@ public class QCPipeline extends Workflow { } def accessProvider = FileSystemAccessProvider.getInstance() - def bamFile = new File(aqcfg.getSingleBamParameter()) + def bamFile = new File(aqcfg.singleBamParameter) if (!accessProvider.fileExists(bamFile) || !accessProvider.isReadable(bamFile)) { context.addErrorEntry(ExecutionContextError.EXECUTION_SETUP_INVALID. expand("A 'bam' parameter was set, but the BAM file is not readable: '${bamFile}'")) @@ -296,7 +297,7 @@ public class QCPipeline extends Workflow { public boolean checkFingerprintingSitesFile(ExecutionContext context) { def aqcfg = new AlignmentAndQCConfig(context) - def accessProvider = FileSystemAccessProvider.getInstance() + def accessProvider = FileSystemAccessProvider.instance boolean result = true if (aqcfg.runFingerprinting) { if (!accessProvider.fileExists(aqcfg.fingerprintingSitesFile) @@ -323,7 +324,8 @@ public class QCPipeline extends Workflow { public boolean createTestdata(ExecutionContext context) { boolean allOk = true; - COProjectsRuntimeService runtimeService = (COProjectsRuntimeService) context.getRuntimeService(); + COProjectsRuntimeService runtimeService = + (COProjectsRuntimeService) context.runtimeService List samples = runtimeService.getSamplesForContext(context); for (Sample sample : samples) { @@ -332,7 +334,7 @@ public class QCPipeline extends Workflow { List rawSequenceGroups = runtimeService.getLanesForSample(context, sample); for (LaneFileGroup lfg : rawSequenceGroups) { - for (LaneFile lf : lfg.getFilesInGroup()) { + for (LaneFile lf : lfg.filesInGroup) { allLaneFiles.addFile(lf); } }