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.

Sign up for GitHub

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

Jump to bottom

Handle situation with few or zero reads in TCap #917

Merged
merged 20 commits into from
Apr 6, 2023
Merged

Handle situation with few or zero reads in TCap #917

merged 20 commits into from
Apr 6, 2023

Conversation

kachulis
Copy link
Contributor

@kachulis kachulis commented Feb 10, 2023 •
edited
Loading

Closes #913

Currently on personal picard and gatk dockers, will require those changes before merging this. Updated to official dockers.

@github-actions
Copy link

Remember to squash merge!

4 similar comments
@github-actions
Copy link

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

@kachulis
Copy link
Contributor Author

needs broadinstitute/picard#1859 and broadinstitute/gatk#8198

@kachulis kachulis changed the title Handle situsation with few or zero reads in TCap Handle situation with few or zero reads in TCap Feb 10, 2023
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

@kachulis kachulis marked this pull request as ready for review March 20, 2023 19:28
@github-actions
Copy link

Remember to squash merge!

1 similar comment
@github-actions
Copy link

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

2 similar comments
@github-actions
Copy link

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

@kachulis
Copy link
Contributor Author

test this please

kachulis
kachulis commented Mar 23, 2023
View reviewed changes
tasks/broad/RNAWithUMIsTasks.wdl
java -Xms~{java_memory_size}m -Xmx~{max_heap}m -jar /usr/picard/picard.jar CollectMultipleMetrics \
INPUT=~{input_bam} \
OUTPUT=~{output_bam_prefix} \
PROGRAM=CollectInsertSizeMetrics \
Copy link
Contributor Author

@kachulis kachulis Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

These do not need to be specified explicitly, because by default CollectMultipleMetrics runs CollectAlignmentSummaryMetrics, CollectBaseDistributionByCycle, CollectInsertSizeMetrics, MeanQualityByCycle, and QualityScoreDistribution. Adding them explicitly (without also PROGRAM=null to clear the defaults) has no effect

@github-actions
Copy link

Remember to squash merge!

mmorgantaylor
mmorgantaylor approved these changes Mar 27, 2023
View reviewed changes
Copy link
Member

@mmorgantaylor mmorgantaylor left a comment

Choose a reason for hiding this comment

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

👍 looks good, and tested this on both regular samples and a no-reads sample

nikellepetrillo
nikellepetrillo reviewed Apr 6, 2023
View reviewed changes
pipelines/broad/genotyping/illumina/IlluminaGenotypingArray.changelog.md Outdated
* CheckFingerprint can allow LOD 0

# 1.12.12
2023-03-30 (Date of Last Commit)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2023-03-30 (Date of Last Commit)
2023-01-13 (Date of Last Commit)

nikellepetrillo
nikellepetrillo reviewed Apr 6, 2023
View reviewed changes
pipelines/broad/genotyping/illumina/IlluminaGenotypingArray.changelog.md
@@ -1,6 +1,11 @@
# 1.12.12
# 1.12.13
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like things have gotten out of order, this section should look like:

# 1.12.13
2023-03-30 (Date of Last Commit)

* CheckFingerprint can allow LOD 0

# 1.12.12
2023-01-13 (Date of Last Commit)

* Updated remaining uses of GATK to version 4.3.0.0

nikellepetrillo
nikellepetrillo approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@nikellepetrillo nikellepetrillo left a comment

Choose a reason for hiding this comment

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

minor changelog comment, otherwise looks good, ty!

@github-actions
Copy link

github-actions bot commented Apr 6, 2023

Remember to squash merge!

@github-actions
Copy link

github-actions bot commented Apr 6, 2023

Remember to squash merge!

@kachulis kachulis added the eng-only Scientific review not needed label Apr 6, 2023
@kachulis
Copy link
Contributor Author

kachulis commented Apr 6, 2023

test this please

@kevinpalis kevinpalis mentioned this pull request Apr 6, 2023
Closed
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

Remember to squash merge!

@kachulis kachulis merged commit 4655af4 into develop Apr 6, 2023
@kachulis kachulis deleted the ck_tcap_few_reads branch April 6, 2023 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmorgantaylor mmorgantaylor mmorgantaylor approved these changes

@nikellepetrillo nikellepetrillo nikellepetrillo approved these changes

@fleharty fleharty Awaiting requested review from fleharty

@gbggrant gbggrant Awaiting requested review from gbggrant

@samuelklee samuelklee Awaiting requested review from samuelklee

Assignees
No one assigned
Labels
eng-only Scientific review not needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RNAWithUMIs pipeline fails if there are very few reads
3 participants
@kachulis @mmorgantaylor @nikellepetrillo