Add fixmate -M option for base modifications#1990
Merged
Merged
Conversation
It no longer reads just 2 records at a time and instead fetches all reads sharing the same template name. It then scans these to find the primaries so it can match the same logic as before. This new structure however provides the opportunity for fixing secondaries and supplementaries based on their primary reads, which previously wasn't possible after the paired primary had been found.
Contributor
Author
|
Note: this will fail checks until samtools/htslib#1749 is merged. The code is still compatible with earlier htslibs, but it won't be correcting all of the possible type of mistakes without it as it cannot sanity check every type of base modification error. |
8afad7d to
4cd7ac5
Compare
4cd7ac5 to
030b3ee
Compare
030b3ee to
d04de4d
Compare
Contributor
Author
|
I just pushed a trivial commit message edit to trigger retests now the htslib PR has been merged. |
741ad08 to
4923539
Compare
daviesrob
reviewed
Feb 26, 2024
daviesrob
left a comment
Member
There was a problem hiding this comment.
Comments so far. I have yet to check through all the Methylation bits...
daviesrob
reviewed
Feb 27, 2024
4923539 to
d250bc8
Compare
If we have supplementary reads with trimmed SEQ fields and corresponding hard-clips, the MM/ML tag may be invalid if they don't have a corresponding MN that matches the trimmed SEQ length. However the primary non-supplementary alignment should contain the full sequence, allowing us to work out how to trim the supplementary MM/ML tags and we can set a revised MN accordingly. We also update the draft Mm and Ml tags to their official MM/ML tags. (Dropped for a while and updated early Feb 2024)
d250bc8 to
bc85434
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The specification has an MN tag to specify the length of the sequence at the time the MM/ML tags were produced. We validate this against the sequence length (or use the lack of it) as an indication that the aligner has not updated the base modification tags when doing hard-clipping of sequence during secondary alignment. Some aligners do, but most do not. This disparity makes it impossible to make assumptions, so
-Mis off by default and we should only use it when we know our aligner either does always output MN (it's then a useful sanity check) or if we know our aligner does not update MM/ML while hard clipping.This also refactors the main loop of fixmate to consume all reads for a single template before fixing up the data. The original implementation was written before secondary and supplementary reads were added, so it had implicit assumptions on data ordering and made it impossible to fix secondaries after the 2nd read in a read-pair.
Draft as I still need to verify this on proper aligner output rather than my hand-crafted examples.