GATB / bcalm

compacted de Bruijn graph construction in low memory
MIT License
99 stars 20 forks source link

bcalm 2.2.1 on conda is still producing incorrect cDBG #44

Closed ctb closed 5 years ago

ctb commented 5 years ago

Per #37, I (and others in my group) are finding that bcalm 2.2.1 in bioconda continues to produce an incorrect cDBG. To reproduce, use the same procedure and you should see the same complaint in the output.

Here is what conda reports as far as what is installed.

(sgc) ctbrown@farm:~/spacegraphcats-twofoo-example$ conda list | grep -i bcalm
bcalm                     2.2.1                hdbcaa40_1    bioconda

cc @natir @rchikhi

Let me know if there's anything I can do!

rchikhi commented 5 years ago

hmm yeah. On my computer: master branch is fine, binary v2.2.1 on github is fine, but I confirm that conda's version still has the bug.

Conda's binaries v2.2.0 and v2.2.1 oddly have the same size but they're not bit-equal:

-rwxrwxr-x 1 rayan rayan 30628741 Oct 28  2018 /home/rayan/miniconda3/pkgs/bcalm-2.2.0-hd28b015_3/bin/bcalm
-rwxrwxr-x 1 rayan rayan 30628741 Dec 26  2018 /home/rayan/miniconda3/pkgs/bcalm-2.2.1-hd28b015_0/bin/bcalm
$ diff /home/rayan/miniconda3/pkgs/bcalm-2.2.0-hd28b015_3/bin/bcalm /home/rayan/miniconda3/pkgs/bcalm-2.2.1-hd28b015_0/bin/bcalm 
Binary files /home/rayan/miniconda3/pkgs/bcalm-2.2.0-hd28b015_3/bin/bcalm and /home/rayan/miniconda3/pkgs/bcalm-2.2.1-hd28b015_0/bin/bcalm differ

I figured the culprit: https://github.com/bioconda/bioconda-recipes/blob/master/recipes/bcalm/build.sh#L8

That conda's bcalm v2.2.1 is built using an ooold gatb-core, one that still has the 2018 bug.

@natir, a fix would be to do a git clone https://github.com/GATB/gatb-core.git && git checkout f25f57bb65db0870ec3c5b14af5e694c7941f567 there.

I keep forgetting that conda distributes a binary that it compiles, not the binary that's from github release.

Apologies for the zombie bug @ctb!

ctb commented 5 years ago

Braiiiiiiiiiiins!

-- Titus Brown, ctbrown@ucdavis.edumailto:ctbrown@ucdavis.edu

On Jul 4, 2019, at 3:32 PM, Rayan Chikhi notifications@github.com<mailto:notifications@github.com> wrote:

hmm yeah. On my computer: master branch is fine, binary v2.2.1 on github is fine, but I confirm that conda's version still has the bug.

Conda's binaries v2.2.0 and v2.2.1 oddly have the same size but they're not bit-equal:

-rwxrwxr-x 1 rayan rayan 30628741 Oct 28 2018 /home/rayan/miniconda3/pkgs/bcalm-2.2.0-hd28b015_3/bin/bcalm -rwxrwxr-x 1 rayan rayan 30628741 Dec 26 2018 /home/rayan/miniconda3/pkgs/bcalm-2.2.1-hd28b015_0/bin/bcalm $ diff /home/rayan/miniconda3/pkgs/bcalm-2.2.0-hd28b015_3/bin/bcalm /home/rayan/miniconda3/pkgs/bcalm-2.2.1-hd28b015_0/bin/bcalm Binary files /home/rayan/miniconda3/pkgs/bcalm-2.2.0-hd28b015_3/bin/bcalm and /home/rayan/miniconda3/pkgs/bcalm-2.2.1-hd28b015_0/bin/bcalm differ

I figured the culprit: https://github.com/bioconda/bioconda-recipes/blob/master/recipes/bcalm/build.sh#L8

That conda's bcalm v2.2.1 is built using an ooold gatb-core, one that still has the 2018 bug.

@natirhttps://github.com/natir, a fix would be to do a git clone https://github.com/GATB/gatb-core.git && git checkout f25f57bb65db0870ec3c5b14af5e694c7941f567 there.

I keep forgetting that conda distributes a binary that it compiles, not the binary that's from github release.

Apologies for the zombie bug @ctbhttps://github.com/ctb!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/GATB/bcalm/issues/44?email_source=notifications&email_token=AAAMOSEWD7HQTVE44E5HFQTP5Z3AHA5CNFSM4H52YX42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZIGOAA#issuecomment-508585728, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAAMOSE6XQRJWABQ6L2SSMDP5Z3AHANCNFSM4H52YX4Q.

natir commented 5 years ago

Very nice work @rchikhi I create a pull request to update bcalm build in bioconda and merge it when all check was passed.

Follow pull request bioconda/bioconda-recipes#16213 to be notify when it's merged in bioconda

natir commented 5 years ago

With not yet merge bioconda version of bcalm installable with this command:

conda install -c https://63640-42372094-gh.circle-artifacts.com/0/tmp/artifacts/packages bcalm

I didn't reproduce bug

ctb commented 5 years ago

that seems to fix it - thank you!!

natir commented 5 years ago

This version of bcalm's build was merge in bioconda I hope the trouble was fix. If you have another trouble please feel free to tag me in the issue.

ctb commented 5 years ago

thank you!!

ctb commented 5 years ago

I have verified that with a regular conda install, it is now fixed. Thank you again!