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

Feature/duplex #324

Merged
merged 33 commits into from
Apr 24, 2024
Merged

Feature/duplex #324

merged 33 commits into from
Apr 24, 2024

Conversation

Adoni5
Copy link
Contributor

@Adoni5 Adoni5 commented Jan 24, 2024

Two implementations of simple duplex targeting

  • Initial implementation of the "chill" accept if the previous read was deliberately sequenced, and this one otherwise wouldn't be, which has been included to account for sequencing duplex with no_map as the decision.

  • The slightly more complex previous alignment was sequenced, and this read aligns to the opposite strand on the same contig.

Both of these modes break the chain of stop_receiving if the previous read was only sequenced if it was potentially duplex, by checking the new duplex_override decision, on the duplex_tracker.

Other things of note:

  1. Version is included in the printed output at the top of the logs
  2. Small change to an error in mappy.py index extension checking, which not includes the extension that is incorrect for clearer error messaging
  3. Added a new decision of when we override a read at the start of a readfish run, if the translocated portion is of unknown length

Copy link
Contributor

@alexomics alexomics left a comment

Choose a reason for hiding this comment

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

Overall good, needs to be certain about strand types at specific places!

src/readfish/_cli_args.py Outdated Show resolved Hide resolved
src/readfish/entry_points/targets.py Show resolved Hide resolved
src/readfish/entry_points/targets.py Outdated Show resolved Hide resolved
src/readfish/entry_points/targets.py Outdated Show resolved Hide resolved
src/readfish/entry_points/targets.py Outdated Show resolved Hide resolved
src/readfish/plugins/utils.py Outdated Show resolved Hide resolved
src/readfish/plugins/utils.py Outdated Show resolved Hide resolved
src/readfish/plugins/utils.py Outdated Show resolved Hide resolved
src/readfish/entry_points/targets.py Show resolved Hide resolved
src/readfish/plugins/utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@alexomics alexomics left a comment

Choose a reason for hiding this comment

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

I think that the logic is clearer, I've made a couple of comments but I think that this is there!

Has it been run against a live/simulated device?

src/readfish/entry_points/targets.py Outdated Show resolved Hide resolved
src/readfish/entry_points/targets.py Outdated Show resolved Hide resolved
@mattloose
Copy link
Contributor

It has been tested live.... waiting to interpret those tests.

@Adoni5
Copy link
Contributor Author

Adoni5 commented Feb 12, 2024

Checked the simple duplex

Of 220,000 Duplex reads after 4 days of running - we sent unblocks to 42,886 to the one or both of these reads parents, (29051 reads had an unblock sent to one of the parents, 13835 to both of the parents).

However looking at this by read lengths it's probably fine -
image

And over the whole width of read lengths:
image

Note - 10 base bin width for both plots.

It's roughly 0.7206% of total duplex bases, so I think this is working!

To avoid this languishing, I'm suggesting we merge this in and mark it as highly experimental.

Move storing duplex tracker alignments/decisions into it's own conditional block
@Adoni5 Adoni5 force-pushed the feature/duplex branch 2 times, most recently from 3bfe556 to 159e49e Compare February 12, 2024 17:38
@Adoni5 Adoni5 mentioned this pull request Apr 17, 2024
@Adoni5
Copy link
Contributor Author

Adoni5 commented Apr 17, 2024

This works - @alexomics and @mattloose I'd like to merge so we can address #347 and avoid this languishing so long it no longer works

@mattloose
Copy link
Contributor

Yes - we know that this works now. I'm happy to merge it.

@Adoni5
Copy link
Contributor Author

Adoni5 commented Apr 17, 2024

@alexomics blocked by your requested change for being certain about Strand which has been addressed!

@Adoni5 Adoni5 merged commit 778d260 into main Apr 24, 2024
9 checks passed
@Adoni5 Adoni5 mentioned this pull request May 14, 2024
@Adoni5 Adoni5 deleted the feature/duplex branch August 26, 2024 17:05
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.

3 participants