SNVQ recalibration for flow based reads #8697

ilyasoifer · 2024-02-16T14:25:41Z

The pull request addresses two issues:

Improved and more robust parsing of FlowBasedReads. Specifically, the code now determines the minimal reportable quality
New tool AddFlowSNVQuality that allows users to convert the flow-based quality format when every base quality reports probability of an insertion or deletion to a conventional format that gives base qualities (total probability of mismatch and probability of each mismatch in separate tags).

We believe that this tool is going to be important for users of the Ultima Genomics data that care about calling SNVs, especially in somatic setting, so the goal was to make documentation more accessible.

Happy to receive feedback about it

xgboost added, initial xgboost version update --svn-qual-model added, placeholder code too FlowFeatureMapper refactor towards AppleSVNQR ApplySVNQR Integration Test (placeholder) typos fixed model now loading conf file reading added initial matrix build move snvqr feature creation to processor xgboost matrix logging feature implementation (partial) non-called-base support optimize for non-called-base independent features apply snvqr test file update all features initial coding matrix build seems ok BQ and histograms lots of changes ... quality interpolation Interpolator edge condition handled additional aliasing --add-apply-snvqr-features safeguard against non category boolean features BQ length (and polarity?) fixed "quality_interpolation_data" made optional ApplySNVQR --model optional FlowFeatureMapper optimization - reduced by ~70% conf optional when no model minor performance increase enforce score 0 in SNVQ BQ zero warning added zeroCount report

Also updated the printout of flow matrix to have more digits

… GATK and updated t0 parsing

…g probability is not given

ilyasoifer · 2024-03-12T19:48:38Z

@meganshand - this should be ready for your review, there is a parallel PR in picard that adds tools that can collect statistics from the output of this tool, so would be good to merge this one first
Thanks a lot!

meganshand · 2024-03-12T20:16:05Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQuality.java

+
+    @Advanced
+    @Hidden
+    @Argument(fullName="debug-collect-stats-into", doc = "", optional = true)


Why is this debug argument here while the debug read name argument is in AddFlowSNVQualityArgumnetCollection?

meganshand · 2024-03-12T20:16:52Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQuality.java

+    private SAMFileGATKReadWriter outputWriter;
+
+    @Advanced
+    @Argument(fullName=INCLUDE_QC_FAILED_READS_FULL_NAME, doc = "include reads with QC failed flag", optional = true)


Why have this here and the keepSupplementaryAlignments arg in the collection?

meganshand · 2024-03-14T13:21:31Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQuality.java

+        }
+
+        // collect input stats
+        if ( debugCollectStatsInto != null )


add braces to this one so it matches the others

meganshand · 2024-03-14T13:39:22Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQuality.java

+    }
+
+    private void collectOutputStats(GATKRead read) {
+        if ( read.hasAttribute("BQ") ) {


Use BASE_QUALITY_ATTRIBUTE_NAME here instead of "BQ" (and below as well)

meganshand · 2024-03-14T13:49:02Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQuality.java

+    @Argument(fullName="debug-collect-stats-into", doc = "", optional = true)
+    public String debugCollectStatsInto = null;
+
+    public static final String BASE_QUALITY_ATTRIBUTE_NAME = "BQ";


It looks like the BQ tag is defined in the spec as Offset to base alignment quality (BAQ). How much of a pain would it be to change this tag name (not sure how baked into your pipelines this is at this point).

Another option could be to put your updated base qualities in the output as the main base qualities, and move the old base qualities to the OQ tag (original qualities) which is how BQSR has worked in the past. This might make downstream tooling easier without deleting any data.

@ilyasoifer could you.please suggest? This tool seems to be collecting stats on BQ - i.e. it is not the one producing the field. I'm not that familiar with the upstream and downstream tooling,

@dror27 - agree with Megan that BQ is a confusing name (somehow I missed this point). We should fix this also in the picard tool in a different PR
So the desired interface is:

By default the tool overrides base qualities

There is a parameter that tells the tool to set a different tag rather than base qualities, but it is unset by default

Addressed. Tool now override base qualities by default. If --output-quality-attribute is provided, then base qualities are preserved and an attribute by the value of the parameter is set.

meganshand · 2024-03-15T18:45:56Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/groundtruth/SeriesStats.java

+        }
+    }
+
+    public void aux(int v, int auxValue) {


Do these aux functions get used anywhere?

meganshand · 2024-03-15T18:52:11Z

src/main/java/org/broadinstitute/hellbender/utils/read/FlowBasedRead.java

@@ -250,6 +254,33 @@ public FlowBasedRead(final SAMRecord samRecord, final String flowOrder, final in

    }

+    public FlowBasedRead(final FlowBasedRead other, final boolean deepCopy) {


Is this used anywhere? If not is it still needed?

meganshand · 2024-03-15T18:58:19Z

src/main/java/org/broadinstitute/hellbender/utils/read/FlowBasedRead.java

        }

        // access qual, convert to flow representation
        final byte[]      quals = samRecord.getBaseQualities();
        final byte[]      tp = samRecord.getSignedByteArrayAttribute(FLOW_MATRIX_TAG_NAME);
+        // initialize matrix


spurious comment?

meganshand · 2024-03-15T19:01:48Z

src/main/java/org/broadinstitute/hellbender/utils/read/FlowBasedRead.java

@@ -820,9 +869,9 @@ private void applyFilteringFlowMatrix(){
    private void clipProbs() {
        for ( int i = 0 ; i < getMaxHmer(); i++ ) {
            for ( int j =0; j < getNFlows(); j++) {
-                if ((flowMatrix[i][j] <= fbargs.probabilityRatioThreshold) &&
+                if ((flowMatrix[i][j] <= perHmerMinErrorProb*3) &&


where did this *3 come from? Why is it needed here?

@ilyasoifer

Clarified the function

meganshand · 2024-03-15T19:05:22Z

src/test/resources/large/add_flow_snvq/.gitattributes

did you mean to add this file?

dror27 · 2024-03-25T08:54:25Z

@meganshand - thank you for your comments. I have responded and updated. We have just a few left between @ilyasoifer and myself - then you can have it back for another round.

ilyasoifer · 2024-03-28T20:21:11Z

@dror27 - I addressed the comments that were assigned to me. There is some documentation task left and that failing test and then we are ready, I think.

dror27 · 2024-03-31T09:59:23Z

@dror27 - I addressed the comments that were assigned to me. There is some documentation task left and that failing test and then we are ready, I think.

I'm finding out that the failing test is due to different floating point accuracy between the platform on which the expected file is generated and the one running the test. I will change the way the expected and generated files are changed against to accommodate for that

< -5.6956674728026115
< -5.6956674728026115
< -5.6956674728026115
---
> -5.695667472802612
> -5.695667472802612
> -5.695667472802612

ilyasoifer · 2024-03-31T12:28:18Z

@meganshand - I think that we addressed all your comments, could you take another look please?

meganshand

This is looking really good. I just had a few small suggestions for documentation.

meganshand · 2024-04-01T18:51:07Z

...adinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQualityArgumentCollection.java

+    /**
+     * alternate quality attribute to set instead of the usual quality string
+     */
+    @Argument(fullName = OUTPUT_QUALITY_ATTRIBUTE_FULL_NAME, doc = "alternate quality attribute to set instead of the usual quality string.", optional = true)


Suggested change

@Argument(fullName = OUTPUT_QUALITY_ATTRIBUTE_FULL_NAME, doc = "alternate quality attribute to set instead of the usual quality string.", optional = true)

@Argument(fullName = OUTPUT_QUALITY_ATTRIBUTE_FULL_NAME, doc = "alternate SAM tag to put original quality scores instead of overwriting the QUAL field. If not used, QUAL will be overwritten.", optional = true)

meganshand · 2024-04-01T18:52:25Z

...adinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQualityArgumentCollection.java

    public SnvqModeEnum snvMode = SnvqModeEnum.Geometric;

+    /**
+     * alternate quality attribute to set instead of the usual quality string


Suggested change

* alternate quality attribute to set instead of the usual quality string

* By default this tool overwrites the QUAL field with the new qualities. Setting this argument saves the original qualities in the specified SAM tag.

meganshand · 2024-04-01T19:14:02Z

src/main/java/org/broadinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQuality.java

+    }
+
+    @VisibleForTesting
+    protected double[] generateHmerBaseErrorProbabilities(final int[] key, final double[][] errorProbBands, final int flow,


Just checking, this comment was added to AddFlowBaseQuality rather than here, right?

meganshand · 2024-04-01T19:20:15Z

...adinstitute/hellbender/tools/walkers/featuremapping/AddFlowSNVQualityArgumentCollection.java

+import java.util.List;
+
+/**
+ * Set of arguments for the {@link FlowFeatureMapper}


Suggested change

* Set of arguments for the {@link FlowFeatureMapper}

* Set of arguments for {@link AddFlowSNVQuality}

meganshand

👍 I'll merge when tests pass

dror27 and others added 30 commits January 17, 2024 11:19

[BIOIN-1385] FlowBasedRead now can estimate filling_value from the read

bf5d2d8

Also updated the printout of flow matrix to have more digits

[BIOIN-1385] Removed probabilityRatioThreshold

876a4e6

[BIOIN-1385] Removed mistaken usage by fillingValue

86d3ae4

[BIOIN-1385] Fixed bug in t0 parsing

c5e2ad8

[BIOIN-1385] Reverted the aggressive change in filling probability in…

fcbbd3c

… GATK and updated t0 parsing

[BIOIN-1385] Minor update of poorly_modelded_reads

ba29b3c

[BIOIN-1385] FlowBasedRead class cleanup

ac66953

[BIOIN-1385] Updated ground truth scorer for the case when the fillin…

8d9782f

…g probability is not given

Fixed typing error + missed file

a032030

ceil -> round

626a458

Set phred score for bins with no zeros to maximum rather than zero

b4d8341

Updated tests

2d69e85

Updated tests

92aab19

Updated tests

c564dc6

-10 qual fix

51ebdb5

Guard against negative qualities

a7947b9

(not final) reuse AddFlowBaseQuality code to accelerate

5e6da62

--debug-use-add-flow-base-quality-algorithm flasg

fcb1f83

use of add_base_q now default

8f17ea5

ApplySNVQR test data rebuilt

8634c11

add_base_q integration in progress ... missing mapping function

9ba77d3

allow unsorted/unmapped input to ApplySNVQR when no model specified

44b6763

SNVQ of called base adjusted to sum to 1

2f478de

update called base snvq

13aa6ee

--limit-phred-score replaces --limit-score

5850557

flow flags passed to add base quality

d6707d2

BQ adjusted

d0e4a0f

Fixed boundary case

f79da59

Snvq modes added. see --snvq-mode param

b2337dd

dror27 added 2 commits March 11, 2024 11:13

DEFAULT_FILLING_VALUE restored to 0.001

eacc100

snvq-mode restored

a2b7f54

ilyasoifer requested a review from meganshand March 12, 2024 19:42

ilyasoifer marked this pull request as ready for review March 12, 2024 19:43

meganshand self-assigned this Mar 12, 2024

meganshand suggested changes Mar 15, 2024

View reviewed changes

meganshand assigned dror27 and unassigned meganshand Mar 15, 2024

Dror Kessler and others added 4 commits March 16, 2024 09:45

AddFlowSNVQuality integeration test expected data adjusted

06284b0

fix comment spec on expected output

4ee1255

some of the comments addressed

bc0ff83

additional comments addressed

ff06e36

dror27 and others added 2 commits March 25, 2024 09:54

additional comments addressed

403bd29

Clarified function

a6afd8e

dror27 added 3 commits March 31, 2024 12:11

limit scores precision to regain platform independence

516c152

documented generateHmerBaseErrorProbabilities

4ee5b6d

AddFlowSNVQuality now overrides read base quality by default

d9c090b

ilyasoifer requested a review from meganshand March 31, 2024 12:27

meganshand self-assigned this Apr 1, 2024

meganshand reviewed Apr 1, 2024

View reviewed changes

meganshand removed their assignment Apr 1, 2024

additional doc comments addressed

0c73917

meganshand approved these changes Apr 4, 2024

View reviewed changes

meganshand merged commit 6739e6d into broadinstitute:master Apr 4, 2024
20 checks passed

ilyasoifer deleted the ultima.snv-qual.develop.rebase branch July 26, 2024 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNVQ recalibration for flow based reads #8697

SNVQ recalibration for flow based reads #8697

ilyasoifer commented Feb 16, 2024 •

edited

Loading

ilyasoifer commented Mar 12, 2024

meganshand Mar 12, 2024

dror27 Mar 18, 2024

meganshand Mar 12, 2024

dror27 Mar 18, 2024

meganshand Mar 14, 2024

meganshand Mar 14, 2024

meganshand Mar 14, 2024

dror27 Mar 18, 2024

ilyasoifer Mar 28, 2024 •

edited

Loading

dror27 Mar 31, 2024

meganshand Mar 15, 2024

dror27 Mar 21, 2024

meganshand Mar 15, 2024

dror27 Mar 21, 2024

meganshand Mar 15, 2024

meganshand Mar 15, 2024

dror27 Mar 21, 2024

ilyasoifer Mar 28, 2024

meganshand Mar 15, 2024

dror27 commented Mar 25, 2024

ilyasoifer commented Mar 28, 2024 •

edited

Loading

dror27 commented Mar 31, 2024 •

edited

Loading

ilyasoifer commented Mar 31, 2024

meganshand left a comment

meganshand Apr 1, 2024

meganshand Apr 1, 2024

meganshand Apr 1, 2024

meganshand Apr 1, 2024

meganshand left a comment

		@@ -250,6 +254,33 @@ public FlowBasedRead(final SAMRecord samRecord, final String flowOrder, final in

		}

		public FlowBasedRead(final FlowBasedRead other, final boolean deepCopy) {

	@Argument(fullName = OUTPUT_QUALITY_ATTRIBUTE_FULL_NAME, doc = "alternate quality attribute to set instead of the usual quality string.", optional = true)
	@Argument(fullName = OUTPUT_QUALITY_ATTRIBUTE_FULL_NAME, doc = "alternate SAM tag to put original quality scores instead of overwriting the QUAL field. If not used, QUAL will be overwritten.", optional = true)

	* alternate quality attribute to set instead of the usual quality string
	* By default this tool overwrites the QUAL field with the new qualities. Setting this argument saves the original qualities in the specified SAM tag.

	* Set of arguments for the {@link FlowFeatureMapper}
	* Set of arguments for {@link AddFlowSNVQuality}

SNVQ recalibration for flow based reads #8697

SNVQ recalibration for flow based reads #8697

Conversation

ilyasoifer commented Feb 16, 2024 • edited Loading

ilyasoifer commented Mar 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilyasoifer Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dror27 commented Mar 25, 2024

ilyasoifer commented Mar 28, 2024 • edited Loading

dror27 commented Mar 31, 2024 • edited Loading

ilyasoifer commented Mar 31, 2024

meganshand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meganshand left a comment

Choose a reason for hiding this comment

ilyasoifer commented Feb 16, 2024 •

edited

Loading

ilyasoifer Mar 28, 2024 •

edited

Loading

ilyasoifer commented Mar 28, 2024 •

edited

Loading

dror27 commented Mar 31, 2024 •

edited

Loading