Skip to content

VCF benchmarks #944

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

Closed
wants to merge 2 commits into from
Closed

VCF benchmarks #944

wants to merge 2 commits into from

Conversation

tomwhite
Copy link
Collaborator

This is not ready for merging, but shows the current status of VCF read and write speed (#936 and #924).

sgkit/tests/io/vcf/test_vcf_benchmark.py::test_vcf_read_speed bytes read: 78683380
duration: 2.69 s
speed: 29.3 MB/s
PASSED
sgkit/tests/io/vcf/test_vcf_benchmark.py::test_vcf_write_speed [W::vcf_parse_format] Extreme FORMAT/AD value encountered and set to missing at 20:10000054
bytes written: 79602637
duration: 99.27 s
speed: 0.8 MB/s
PASSED

It should be possible to get both to ~100MB/s.

@tomwhite tomwhite mentioned this pull request Nov 15, 2022
@tomwhite
Copy link
Collaborator Author

tomwhite commented Dec 5, 2022

Re-running with the new code from #953, I get:

sgkit/tests/io/vcf/test_vcf_benchmark.py::test_vcf_write_speed bytes written: 81811189
duration: 0.84 s
speed: 97.7 MB/s
PASSED

This could be merged given that it only takes a few seconds to run now. It could be added to benchmark tests later (#938).

@tomwhite tomwhite marked this pull request as ready for review December 5, 2022 12:51
@jeromekelleher
Copy link
Collaborator

Do we want to put this in as a unit test? Maybe simpler to use have a vcf_benchmarks.py file in the root or something that you can run if you like? I don't really see the advantage of running this under pytest, since we also need benchmarks that definitely wouldn't run in seconds.

@tomwhite
Copy link
Collaborator Author

tomwhite commented Dec 5, 2022

Yes, that's probably the best thing to do.

@tomwhite
Copy link
Collaborator Author

I've opened #976 as a replacement for this.

@tomwhite
Copy link
Collaborator Author

Closed in favour of #976

@tomwhite tomwhite closed this Dec 13, 2022
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