`+` delimited dual index barcodes, and arbitrary raw barcodes #62

theJasonFan · 2024-02-28T23:10:01Z

Addresses #60.

We now term barcodes delimited by "+" a "semantic" barcode --- a representation that is a 1-1 mapping between a fixed length barcodes (e.g, AAA+CCC) and its read structure 3B3B.

Tasks

update per_sample_metrics.tsv and per_project_metrics.tsv
update sample_barcode_hop_,metrics.tsv
update most_frequent_unmatched.tsv

Details for per-sample metrics

When using sample sheets, dual index barcodes are reported in metrics with + delimiter.

For example, the sample sheet:

[Demux]
[Data]
Sample_ID,Index1_Sequence,Index2_Sequence
s1,CCCCC,AAAAA

yields a barcode for s1 that appear as CCCCC+AAAAA

When using two-column format arbitrary barcodes are allowed. For example:

Sample_ID,Sample_Barcode
s1,TTT+=!-AAA

yields a barcode for s1 that appear as TTT+=!-AAA.

Details for sample barcode hop and unmatched barcode metrics

Barcodes now also appear also in their delimited semantic representations

Tests

For extracting semantic barcode representations from SampleMetadata, the above mentioned behaviors are now tested and exampled in included explicit test cases.
Added end-to-end tests with small examples to check basic values in output per-sample and barcode hopping metrics.

Future Todos

Add release note and review updates to README
~~Design schema for displaying and reading multi-segment barcodes~~
~~Validate multi-segment barcodes. As reflected in the test and implementation, we allow for arbitrary barcodes.~~

nh13

@theJasonFan minor nitpicks, please fixup, then merge at your leisure.

nh13 · 2024-02-29T02:34:40Z

src/lib/metrics.rs

@@ -436,10 +436,11 @@ impl DemuxedGroupSampleMetrics {
        best_barcode_template_count: usize,
        sample_metadata: &SampleMetadata,
    ) -> SampleMetricsProcessed {
+        let barcode = Self::get_metrics_barcode(sample_metadata);


any reason not to inline this below?

Inlined. Please re-review.

nh13 · 2024-02-29T02:35:42Z

src/lib/metrics.rs

@@ -457,6 +458,19 @@ impl DemuxedGroupSampleMetrics {
                / self.base_qual_counter.index_bases_total_seen as f64,
        }
    }
+
+    fn get_metrics_barcode(sample_metadata: &SampleMetadata) -> String {


might as well add some function documentation

why not make this a method on SampleMetadata?

Moved. Please re-review. Note that moving this to SampleMetadata highlights how we should be better about validating the struct.

theJasonFan · 2024-02-29T17:44:35Z

src/lib/metrics.rs

+                    .map(|(barcode, count)| {
+                        let b1 = BString::from(&barcode[..b1_len]);
+                        let b2 = BString::from(&barcode[b1_len..]);
+                        let barcode = BString::from(format!("{}+{}", b1, b2));


@nh13 @jiangweiyao, where hopped barcodes get delimited.

Now we have two places where this formatting occurs, once in the sample metadata and once in the metric modules. Can you refactor to once place please and call from both?

What happens if b2 is empty? I don't think this is doing the right thing.

I strongly agree that this is code-smell coming from different barcode representations. Finding a solution might take more time / effort.

For hop metrics, we know that the reported barcodes can be split and delimited. It is only used for dual index barcodes. The SampleBarcodeHopTracker only updates counts for delimited and hopped barcodes. See:

singular-demux/src/lib/metrics.rs

Line 655 in e9706b6

let index1a = &barcode[0..self.checker.index1_length];

src/lib/metrics.rs

theJasonFan · 2024-02-29T17:46:42Z

src/lib/run.rs

+        let delim = DelimFile::default();
+        let hop_metrics: Vec<BarcodeCount> = delim.read_tsv(&hop_metrics).unwrap();
+        assert_eq!(hop_metrics.len(), 1);
+        assert_eq!(hop_metrics[0].barcode, "TTT+AAA");


@nh13 @jiangweiyao This test demultiplexes a small example of dual indexed data where a single read hops a pair of sample barcodes. We check for that barcode in the output barcode_hop_metrics.tsv

theJasonFan · 2024-02-29T17:48:24Z

src/lib/run.rs

+        assert_eq!(per_sample_metrics.len(), 3);
+
+        assert_eq!(per_sample_metrics[0].barcode_name, "s1");
+        assert_eq!(per_sample_metrics[0].barcode, "TTT+AAA");


@nh13 @jiangweiyao This test demultiplexes a small example of dual indexed data. We check for the barcode in the output per_sample_metrics. Admittedly, the test is does not exhaustively check for all the metrics (a test for that is above in the source). The focus here is on (a) the actual delimited barcode and the # of matches.

theJasonFan · 2024-02-29T17:48:47Z

src/lib/run.rs

+        assert_eq!(per_sample_metrics[1].templates, 0);
+
+        assert_eq!(per_sample_metrics[2].barcode_name, "Undetermined");
+        assert_eq!(per_sample_metrics[2].barcode, "NNNNNN");


@nh13 @jiangweiyao note that the "undetermined" barcode is not delimited.

theJasonFan · 2024-02-29T17:49:32Z

src/lib/sample_metadata.rs

+    /// **Note**: this expects that `self` is a valid SampleMetdata struct. i.e., a after
+    /// an update and sanitization with `update_with_and_set_demux_barcode()`, or is a sentinal
+    /// value where the barcode is all Ns.
+    pub fn get_semantic_barcode(&self) -> BString {


@nh13 refactored this into SampleMetadta and added a docstring per your previous comment.

Will want to use this in metrics.rs given you are creating the unmatched barcode there. Sorry, you may have to revert this to being not a method on samplemetadata

theJasonFan · 2024-02-29T17:50:06Z

src/lib/sample_metadata.rs

@@ -788,4 +811,51 @@ Sample2,GGGG
        ];
        assert_eq!(actual, expected);
    }
+    #[test]
+    fn test_get_semantic_barcode() {


@nh13 Moved and renamed this test after your suggestion to move the method into SampleMetadata

theJasonFan

@nh13 @jiangweiyao ready for re-review

theJasonFan · 2024-02-29T21:48:02Z

@jiangweiyao Also updated to use delimited barcodes in most_frequent_unmatched.tsv

theJasonFan · 2024-02-29T21:48:35Z

src/lib/run.rs

+        let delim = DelimFile::default();
+        let unmatched_metrics: Vec<BarcodeCount> = delim.read_tsv(&unmatched_metrics).unwrap();
+        assert_eq!(unmatched_metrics.len(), 1);
+        assert_eq!(unmatched_metrics[0].barcode, "TTT+AAA");


We also check the unmatched barcode

Probably need to test unmatched metrics for a single-barcode (not dual) given the above comment

nh13

I think the unmatched delimiting of the barcode has a bug. Can you take a look?

nh13 · 2024-03-01T14:01:29Z

src/lib/metrics.rs

+                    .map(|(barcode, count)| {
+                        let b1 = BString::from(&barcode[..b1_len]);
+                        let b2 = BString::from(&barcode[b1_len..]);
+                        let barcode = BString::from(format!("{}+{}", b1, b2));


Now we have two places where this formatting occurs, once in the sample metadata and once in the metric modules. Can you refactor to once place please and call from both?

What happens if b2 is empty? I don't think this is doing the right thing.

nh13 · 2024-03-01T14:02:16Z

src/lib/run.rs

+        let delim = DelimFile::default();
+        let unmatched_metrics: Vec<BarcodeCount> = delim.read_tsv(&unmatched_metrics).unwrap();
+        assert_eq!(unmatched_metrics.len(), 1);
+        assert_eq!(unmatched_metrics[0].barcode, "TTT+AAA");


Probably need to test unmatched metrics for a single-barcode (not dual) given the above comment

nh13 · 2024-03-01T14:02:35Z

src/lib/sample_metadata.rs

+    /// **Note**: this expects that `self` is a valid SampleMetdata struct. i.e., a after
+    /// an update and sanitization with `update_with_and_set_demux_barcode()`, or is a sentinal
+    /// value where the barcode is all Ns.
+    pub fn get_semantic_barcode(&self) -> BString {


Will want to use this in metrics.rs given you are creating the unmatched barcode there. Sorry, you may have to revert this to being not a method on samplemetadata

src/lib/run.rs

theJasonFan

@nh13 added the test with single indexing run unmatched barcodes. Other end-to-end tests should cover other single indexing run metrics with matched barcodes.

nh13

LGTM!

…arcodes

tests: simple test for per sample and barcode hop metrics

tests: add test for unmatched single index barcode

…ensive

theJasonFan marked this pull request as ready for review February 28, 2024 23:12

theJasonFan requested review from nh13 and jiangweiyao February 28, 2024 23:12

theJasonFan linked an issue Feb 28, 2024 that may be closed by this pull request

Implement dual index barcodes, delimited by +. #60

Closed

3 tasks

nh13 approved these changes Feb 29, 2024

View reviewed changes

theJasonFan commented Feb 29, 2024

View reviewed changes

src/lib/metrics.rs Show resolved Hide resolved

theJasonFan commented Feb 29, 2024

View reviewed changes

theJasonFan requested a review from nh13 February 29, 2024 17:50

theJasonFan commented Feb 29, 2024

View reviewed changes

nh13 requested changes Mar 1, 2024

View reviewed changes

theJasonFan commented Mar 1, 2024

View reviewed changes

src/lib/run.rs Show resolved Hide resolved

theJasonFan commented Mar 1, 2024

View reviewed changes

src/lib/run.rs Show resolved Hide resolved

theJasonFan commented Mar 1, 2024

View reviewed changes

nh13 approved these changes Mar 1, 2024

View reviewed changes

theJasonFan added 4 commits March 4, 2024 13:07

feat!: use + for dual index barcodes and allow custom arbitrary raw b…

058d95d

…arcodes

feat!: sementic delimited barcodes for dual index hop metrics

b664115

tests: simple test for per sample and barcode hop metrics

feat!: track delimited unmatched barcodes

e093a31

tests: add test for unmatched single index barcode

tests: check entire BarcodeCount structs instead of members, more def…

a48d6c9

…ensive

theJasonFan force-pushed the 60/jf/multi-barcode-metrics branch from 0588ccd to a48d6c9 Compare March 4, 2024 18:07

theJasonFan merged commit 80f50fb into main Mar 4, 2024
6 checks passed

theJasonFan deleted the 60/jf/multi-barcode-metrics branch March 5, 2024 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`+` delimited dual index barcodes, and arbitrary raw barcodes #62

`+` delimited dual index barcodes, and arbitrary raw barcodes #62

theJasonFan commented Feb 28, 2024 •

edited

Loading

nh13 left a comment

nh13 Feb 29, 2024

theJasonFan Feb 29, 2024

nh13 Feb 29, 2024

theJasonFan Feb 29, 2024

theJasonFan Feb 29, 2024

nh13 Mar 1, 2024

theJasonFan Mar 1, 2024

theJasonFan Feb 29, 2024

theJasonFan Feb 29, 2024

theJasonFan Feb 29, 2024

theJasonFan Feb 29, 2024

nh13 Mar 1, 2024

theJasonFan Feb 29, 2024

theJasonFan left a comment •

edited

Loading

theJasonFan commented Feb 29, 2024

theJasonFan Feb 29, 2024

nh13 Mar 1, 2024

nh13 left a comment

nh13 Mar 1, 2024

nh13 Mar 1, 2024

nh13 Mar 1, 2024

theJasonFan left a comment

nh13 left a comment

+ delimited dual index barcodes, and arbitrary raw barcodes #62

+ delimited dual index barcodes, and arbitrary raw barcodes #62

Conversation

theJasonFan commented Feb 28, 2024 • edited Loading

Tasks

Details for per-sample metrics

Details for sample barcode hop and unmatched barcode metrics

Tests

Future Todos

nh13 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

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

theJasonFan left a comment • edited Loading

Choose a reason for hiding this comment

theJasonFan commented Feb 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nh13 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

theJasonFan left a comment

Choose a reason for hiding this comment

nh13 left a comment

Choose a reason for hiding this comment

`+` delimited dual index barcodes, and arbitrary raw barcodes #62

`+` delimited dual index barcodes, and arbitrary raw barcodes #62

theJasonFan commented Feb 28, 2024 •

edited

Loading

theJasonFan left a comment •

edited

Loading