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

Make use of faster preset and multithreaded minimap2 #26

Merged
merged 2 commits into from
Mar 17, 2024
Merged

Make use of faster preset and multithreaded minimap2 #26

merged 2 commits into from
Mar 17, 2024

Conversation

Ge0rges
Copy link
Contributor

@Ge0rges Ge0rges commented Mar 16, 2024

Switched to using the new version of minimap2 which has the new LrHq preset and multithreading.

Make use of that 4x faster preset.
Implement multithreading (I think it works?) for #18

@wdecoster
Copy link
Owner

Thank you @Ge0rges! Looks mostly okay, but the automated test reported two errors. Could you take a look at it? If necessary, I can help :)

@Ge0rges
Copy link
Contributor Author

Ge0rges commented Mar 17, 2024

Checking it out right now

@Ge0rges
Copy link
Contributor Author

Ge0rges commented Mar 17, 2024

@wdecoster should work now, I forgot to update the test functions.

@wdecoster
Copy link
Owner

Looks good! At some point, we should probably deal with the warning here:
https://github.com/wdecoster/chopper/actions/runs/8317985884/job/22759454860?pr=26#step:4:128

@wdecoster wdecoster merged commit c64acad into wdecoster:master Mar 17, 2024
1 check passed
@Ge0rges
Copy link
Contributor Author

Ge0rges commented Mar 17, 2024

Yeah I wasn't sure what that was about so I decided to leave it be for now.

@Ge0rges
Copy link
Contributor Author

Ge0rges commented Mar 17, 2024

It might be worth mentioning somewhere that we use LrHq in case someone is still using older chemistry.

@Ge0rges
Copy link
Contributor Author

Ge0rges commented Mar 23, 2024

@wdecoster do you think the PR merits a version bump and package?

@wdecoster
Copy link
Owner

Yes, I need to make a new release. Hmm, the older chemistry won't immediately go away. I am not yet sure what the best solution is.

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.

2 participants