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

Reduces memory usage and improves speed #48

Open
wants to merge 1 commit into
base: bloody
Choose a base branch
from

Conversation

courcelm
Copy link
Contributor

Current slicing/join was concatenating the whole chromosome sequence when only a slice was required. That required 2-3 GB of memory in some case and it was slow in my use case:

Before
plot_old

I suggest reverting back to original pyGeno code that was changed by @ericloud .

After

plot_new

@ericloud
Is this a problem with your bug reported here:
2fd3fbe#diff-9a0352f44c9b0e9b00e4e2df44eae54b

Current slicing was concatenating the whole chrosome sequence when only a slice was required. That required 2-3 GB of memory in some case and it was slow.
@ericloud
Copy link
Contributor

ericloud commented May 29, 2018

If my memories are correct, I change this part of code to return the correct sequence when insertion, deletion or indel are present.

example:
_ _ _ _ D _ _ _ _ _ S _ _ _ _ _ _ _ _ _ _
_: 1 nucleotide without change
D: a deletion on 1 nuc
S: a SNP on 1 nuc
And we want to return X nuc on the left and on the right of the SNP. This case is very common when you try to load dbSNP.

If we apply the slice on data (old version), the function will return only (X-1) nuc on the left, because one of them is deleted.

But as you mentioned, apply the slice on sequence (new version 3bb0518 and 2fd3fbe) involve to load all the chromosome and increase memory usage and time.

There are certainly a more efficient way to do it, but go back to the old version, just change the problem to somewhere else.

@ericloud
Copy link
Contributor

ericloud commented May 30, 2018

Also have you try to scan your mutations by Chromosome?
In that way the sequence should stay in cache and be loaded only one time.

Something like:

for chr in genome.iterGet(Chromosome):
    for snp in chr.get(dbSNPSNP):
        ...

@courcelm
Copy link
Contributor Author

I'm already processing by chromosome and it reloads the sequence for every transcript.

@ericloud
Copy link
Contributor

ericloud commented May 30, 2018

Even with something like :

for chr in genome.iterGet(Chromosome):
    chr_seq = chrom.getSequenceData()
    for snp in chr.get(dbSNPSNP):
        bin_seq = NucBinarySequence(chr_seq[slice(snp.start,snp.end,1))])
        ...

It's my last suggestion to help you.

I work on other project for the moment, sorry but I don't have the time to improve this specific case.

@courcelm
Copy link
Contributor Author

I can't apply this since I don't want to load the whole chromosome sequence to memory. Anyway I have solve my problem. Thanks for your help.

@tariqdaouda
Copy link
Owner

What is your solution?

@courcelm
Copy link
Contributor Author

courcelm commented May 30, 2018

I don't have a solution that fixes both the high memory usage and @ericloud bug. I found a way in my script to skip the call to _getSequence

@tariqdaouda
Copy link
Owner

I also have noticed this drop in performances. This is something that would need to be improved in the future.

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.

3 participants