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

[Important] COBS binary format changed; the 661k HQ indexes must be recomputed #32

Closed
leoisl opened this issue Jul 5, 2022 · 9 comments · Fixed by #40
Closed

[Important] COBS binary format changed; the 661k HQ indexes must be recomputed #32

leoisl opened this issue Jul 5, 2022 · 9 comments · Fixed by #40
Assignees
Labels
bug Something isn't working high-priority

Comments

@leoisl
Copy link
Collaborator

leoisl commented Jul 5, 2022

When running mof-search on the 661k HQ indexes downloaded automatically from zenodo through this pipeline, on the EBI cluster (linux), COBS errors out with a segfault on all indexes, e.g.:

$ cobs query -t 0.33 -T 2 -i  intermediate/00_cobs/staphylococcus_aureus__10.cobs -f queries/gc01_1kl.fa
Segmentation fault (core dumped)

This COBS executable is the one that we will distribute, i.e. the one in https://github.com/leoisl/cobs/tree/mac being PRed to the main repo (iqbal-lab-org/cobs#9). When testing with an index generated with an updated COBS, cobs query works fine. Thus users won't be able to run any query because our previously generated COBS indexes is not compatible with the COBS we will distribute (IDK why). We have then to update all COBS indexes in Zenodo

@karel-brinda
Copy link
Owner

Hi Leandro, I'm not sure if I understand this problem. Did the binary COBS format change in the end? Is the underlying issue that the previously built COBS indexes are made in the old format that is no longer supported by the updated COBS?

@leoisl
Copy link
Collaborator Author

leoisl commented Jul 6, 2022

Did the binary COBS format change in the end?

It seems yes, otherwise we wouldn't have this segfault, although it was working on a previous OS X version you were using. It could be due to the latest commits, or due to fixes done by Zhicheng and Giang on COBS (see their commits here: https://github.com/iqbal-lab-org/cobs/commits/master ). Their fixes to cobs/construction/classic_index.cpp surely makes us build different indexes, so I can confirm the binary COBS format did change, but I thought it would still be compatible. Sorry that when I started to work on this COBS version for OS X, I forked from https://github.com/bingmann/cobs instead of https://github.com/iqbal-lab-org/cobs , where the latter is more updated and had these bug fixes...

Is the underlying issue that the previously built COBS indexes are made in the old format that is no longer supported by the updated COBS?

Yes. Just last week when I was PRing my branch I realised https://github.com/iqbal-lab-org/cobs existed and I should PR to this repo instead. Then I merged the new changes/fixes of this repo, had to further work a bit to make them OS X compatible. But now we have incompatible indexes...

@karel-brinda karel-brinda changed the title [Important] COBS 661k HQ indexes must be updated in Zenodo [Important] COBS binary format changed; the 661k HQ indexes must be recomputed Jul 6, 2022
@karel-brinda karel-brinda added the bug Something isn't working label Jul 6, 2022
@leoisl
Copy link
Collaborator Author

leoisl commented Jul 15, 2022

This will be done today I hope!

@leoisl
Copy link
Collaborator Author

leoisl commented Jul 15, 2022

I couldn't do it today trying to fix #35, maybe tomorrow

@leoisl
Copy link
Collaborator Author

leoisl commented Jul 15, 2022

Indices are reproducible, downloading now and uploading to Zenodo, but this will take some time

@karel-brinda
Copy link
Owner

Cool; can I now update the indexes in the paper?

@leoisl
Copy link
Collaborator Author

leoisl commented Jul 17, 2022

Just a moment, for some reason 2 salmonella indexes didn't go through in my bulk upload. I've added them but needed to publish a new version of the part 2 indexes. I am running the pipeline once to be sure everything is working

@leoisl
Copy link
Collaborator Author

leoisl commented Jul 18, 2022

@leoisl leoisl closed this as completed Jul 18, 2022
@karel-brinda
Copy link
Owner

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants