Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TASK-6324 - Simplify QC analysis by launching a single job #2469

Open
wants to merge 25 commits into
base: release-3.x.x
Choose a base branch
from

Conversation

jtarraga
Copy link
Member

@jtarraga jtarraga commented Jun 19, 2024

TASK-6324 Simplify QC analysis by launching a single job:

  • Alignment QC analysis

  • Sample QC analysis

…b that executes sequentially samtools, plot-bamstats and fastqc, #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentFastQcMetricsAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/executors/DockerWrapperAnalysisExecutor.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/samtools/SamtoolsWrapperAnalysisExecutor.java
	modified:   opencga-analysis/src/test/java/org/opencb/opencga/analysis/alignment/AlignmentAnalysisTest.java
	modified:   opencga-core/src/main/java/org/opencb/opencga/core/tools/OpenCgaToolExecutor.java
	modified:   opencga-core/src/main/java/org/opencb/opencga/core/tools/result/ExecutionResultManager.java
…TASK-6324

On branch TASK-6324
Changes to be committed:
	new file:   opencga-app/app/analysis/genome-plot/circos.R
…nused parameters, #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/fastqc/FastqcWrapperAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/fastqc/FastqcWrapperAnalysisExecutor.java
… unused parameters, #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/samtools/SamtoolsWrapperAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/samtools/SamtoolsWrapperAnalysisExecutor.java
…tools stats/flagstats, fastqc), #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
…r sonnar issues, #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/test/java/org/opencb/opencga/analysis/alignment/AlignmentAnalysisTest.java
…pos and sonnar issues, #TASK-6326, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/mutationalSignature/MutationalSignatureAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/mutationalSignature/MutationalSignatureLocalAnalysisExecutor.java
	modified:   opencga-core/src/main/java/org/opencb/opencga/core/tools/variant/MutationalSignatureAnalysisExecutor.java
On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/stats/SampleVariantStatsAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/stats/SampleVariantStatsLocalAnalysisExecutor.java
	modified:   opencga-core/src/main/java/org/opencb/opencga/core/tools/variant/SampleVariantStatsAnalysisExecutor.java
…lot analysis), fix some typos and sonnar issues, #TASK-6326, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/genomePlot/GenomePlotAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/genomePlot/GenomePlotLocalAnalysisExecutor.java
	modified:   opencga-core/src/main/java/org/opencb/opencga/core/tools/variant/GenomePlotAnalysisExecutor.java
On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
…e QC step, #TASK-6326, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java
…ASK-6324

On branch TASK-6324
Changes to be committed:
	deleted:    opencga-analysis/src/main/R/genome-plot/circos.R
	deleted:    opencga-analysis/src/main/R/mutational-signature/mutational-signature.r
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
…lysis folder, #TASK-6326, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/circos/CircosLocalAnalysisExecutor.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/genomePlot/GenomePlotLocalAnalysisExecutor.java
On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/test/java/org/opencb/opencga/analysis/variant/OpenCGATestExternalResource.java
	modified:   opencga-analysis/src/test/java/org/opencb/opencga/analysis/variant/VariantAnalysisTest.java
	deleted:    opencga-storage/opencga-storage-core/src/test/resources/AR2.10039966-01T.copynumber.caveman.vcf.gz
	deleted:    opencga-storage/opencga-storage-core/src/test/resources/AR2.10039966-01T_vs_AR2.10039966-01G.annot.brass.vcf.gz
	deleted:    opencga-storage/opencga-storage-core/src/test/resources/AR2.10039966-01T_vs_AR2.10039966-01G.annot.pindel.vcf.gz
	new file:   opencga-storage/opencga-storage-core/src/test/resources/cancer-cnvs.vcf.gz
	new file:   opencga-storage/opencga-storage-core/src/test/resources/cancer-indels.vcf.gz
	new file:   opencga-storage/opencga-storage-core/src/test/resources/cancer-rearrs.vcf.gz
	new file:   opencga-storage/opencga-storage-core/src/test/resources/cancer-snvs.vcf.gz
	new file:   opencga-storage/opencga-storage-core/src/test/resources/genome-plot-config.json
logger.info("Signagture fit max. rare sigs.: {}", signatureParams.getFitMaxRareSigs());
logger.info("Signagture fit signatures file: {}", signaturesFile);
logger.info("Signagture fit rare signatures file: {}", rareSignaturesFile);
logger.info("Signature id: {}", signatureParams.getId());

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks Low

Change this code to not log user-controlled data. See more on SonarCloud
logger.info("Signagture fit signatures file: {}", signaturesFile);
logger.info("Signagture fit rare signatures file: {}", rareSignaturesFile);
logger.info("Signature id: {}", signatureParams.getId());
logger.info("Signature description: {}", signatureParams.getDescription());

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks Low

Change this code to not log user-controlled data. See more on SonarCloud
@jtarraga jtarraga requested a review from j-coll June 19, 2024 09:36
}
} else {
throw new ToolException("Something wrong happened running Samtools flagstat analysis");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your future self will appreciate if you add some extra information here. Like the number of lines, and/or the actual first line

+ "' is already used");
}
}
}
}
} else {
String msg = "Skipping sample variant stats analysis by user";
addWarning(msg);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this even a warning, or should it be just an info event?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard to say!

@@ -495,6 +499,20 @@ protected final void addAttribute(String key, Object value) throws ToolException
erm.addAttribute(key, value);
}

protected final void addStepAttributes(ExecutionResult executionResult) throws ToolException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to include more information from the nested ExecutionResult, like events (maybe that's the only interesting thing)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean to add the "nested" ExecutionResult events into the list of "parent" Execution events?,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, something like that. Otherwise, they're lost (not technically, as you can always read the nested job result file, but they'd be quite hidden).

… #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
…-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
	modified:   opencga-core/src/main/java/org/opencb/opencga/core/tools/result/ExecutionResultManager.java
…as wrong running dockers (samtools stats/flagstats and fastqc), #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentFastQcMetricsAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/executors/DockerWrapperAnalysisExecutor.java
	modified:   opencga-analysis/src/test/java/org/opencb/opencga/analysis/alignment/AlignmentAnalysisTest.java
@jtarraga jtarraga requested a review from j-coll June 20, 2024 07:53
return steps;
}

@Override
protected void run() throws ToolException {
// Create the tool runner
toolRunner = new ToolRunner(opencgaHome, catalogManager, StorageEngineFactory.get(variantStorageManager.getStorageConfiguration()));
toolRunner = new ToolRunner(getOpencgaHome().toString(), catalogManager,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can better use the constructor

  • public ToolRunner(String opencgaHome, CatalogManager catalogManager, VariantStorageManager variantStorageManager)

instead of

  • ToolRunner(String opencgaHome, CatalogManager catalogManager, StorageEngineFactory storageEngineFactory)

… attributes, #TASK-6325, #TASK-6324

On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
@jtarraga jtarraga requested a review from j-coll June 21, 2024 09:40
On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
On branch TASK-6324
Changes to be committed:
	modified:   opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java
@jtarraga jtarraga changed the title Simplify QC analysis by launching a single job TASK-6324 Simplify QC analysis by launching a single job Jun 21, 2024
j-coll
j-coll previously approved these changes Jun 25, 2024
@j-coll j-coll changed the title TASK-6324 Simplify QC analysis by launching a single job TASK-6324 - Simplify QC analysis by launching a single job Jun 25, 2024
@jtarraga jtarraga changed the base branch from develop to release-3.2.x July 22, 2024 10:17
@jtarraga jtarraga dismissed j-coll’s stale review July 22, 2024 10:17

The base branch was changed.

@halender
Copy link
Contributor

@juanfeSanahuja juanfeSanahuja self-requested a review August 14, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants