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

Alignment score #80

Merged
merged 33 commits into from
Nov 21, 2024
Merged

Alignment score #80

merged 33 commits into from
Nov 21, 2024

Conversation

rob-p
Copy link

@rob-p rob-p commented Aug 25, 2024

This PR adds a function map_with_name which allows resolving #75. Specifically, it passes the query name (which must be \0 terminated) to the map function, which allows the internal minimap2 function mm_map to resolve ties in the chain score.

Though the differences are subtle, this allows exactly replicating the behavior of command-line minimap2, in cases where this tie breaking can result in some alignments being marked supplementary, where they otherwise would not.

@mbhall88
Copy link
Contributor

Hi all,

Just wondering what the status is of this PR? What needs to be done to get it over the line? I'm happy to try and help if necessary.

@jguhlin
Copy link
Owner

jguhlin commented Oct 29, 2024

@rob-p @mbhall88 Hey. I'll work on getting this all moved today/tomorrow. Got off track with grant writing and some moving plans. :/

@mbhall88
Copy link
Contributor

Got off track with grant writing and some moving plans. :/

Ohhhhh I know the feeling! Thanks so much for this great library/bindings. I'm about to start a project using this library so I'll probably be harassing you a bit over the next couple of weeks 😅

@rob-p
Copy link
Author

rob-p commented Oct 30, 2024

@mbhall88,

If you want to take a stab at this (#71), I’m willing to help. Since I found a work around, it’s not top priority for us right now, but it seems important for the lib in general moving forward.

—Rob

p.s. I too know the feeling (not moving this time, but grants and conference deadlines out the wazoo this past month!)

@jguhlin
Copy link
Owner

jguhlin commented Nov 21, 2024

I'm going to merge this, and reconcile with with #85 and then see if I can hack out Arc and push a release

@jguhlin jguhlin merged commit adc1c1f into jguhlin:alignment-score Nov 21, 2024
0 of 3 checks passed
jguhlin added a commit that referenced this pull request Nov 21, 2024
jguhlin added a commit that referenced this pull request Nov 21, 2024
@rob-p Dunno if you can test it, or added the proper tests?

@mbhall88 Query name is passed in as AsRef<[u8]> and there is some not my favorite code in there to prevent a new allocation if the query name is already nul-allocated, and to  do an allocation if needed.

Also I've updated Mappings to return Arc<String>'s for query name and target name, which should decrease performance for very small jobs but increase for large jobs.... but I need to benchmark
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.

4 participants