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

Compare: move get_similarities_at_index into compare_parallel function to help with memory #1509

Closed
wants to merge 9 commits into from
Closed

Compare: move get_similarities_at_index into compare_parallel function to help with memory #1509

wants to merge 9 commits into from

Conversation

olgabot
Copy link
Collaborator

@olgabot olgabot commented May 8, 2021

When running compare on large (~30,000 signature) datasets, even on beefy machines, the memory can lock up (ref: #1486). @pranathivemuri added this fix, which moves the function get_similarities_at_index into the inside of compare_parallel, which reduces the amount of serialization passed between threads. I'm not 100% sure on how this works, @pranathivemuri is the expert here!

@codecov
Copy link

codecov bot commented May 8, 2021

Codecov Report

Merging #1509 (2a5a684) into latest (03f28cd) will increase coverage by 7.47%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           latest    #1509      +/-   ##
==========================================
+ Coverage   82.61%   90.09%   +7.47%     
==========================================
  Files         113       86      -27     
  Lines       11995     8195    -3800     
  Branches     1513     1513              
==========================================
- Hits         9910     7383    -2527     
+ Misses       1830      557    -1273     
  Partials      255      255              
Flag Coverage Δ
python 90.09% <100.00%> (-0.02%) ⬇️
rust ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/sourmash/compare.py 100.00% <100.00%> (ø)
src/sourmash/signature.py 90.00% <0.00%> (-0.48%) ⬇️
src/core/src/encodings.rs
src/core/tests/test.rs
src/core/src/index/linear.rs
src/core/src/sketch/hyperloglog/estimators.rs
src/core/src/index/search.rs
src/core/src/index/mod.rs
src/core/src/ffi/mod.rs
src/core/src/sketch/minhash.rs
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03f28cd...2a5a684. Read the comment docs.

@olgabot
Copy link
Collaborator Author

olgabot commented May 8, 2021

For some reason the version appears as 0.0.0 -- any ideas why?

(nf-core--kmermaid-1.1.0dev)
 Sat  8 May - 10:31  ~/code/sourmash   origin ☊ olgabot/pranthi-fxn-in-fxn ✔ 
 olga@hulk  sourmash info

== This is sourmash version 0.0.0. ==
== Please cite Brown and Irber (2016), doi:10.21105/joss.00027. ==

sourmash version 0.0.0
- loaded from path: /data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/site-packages/sourmash-0.0.0-py3.7-linux-ppc64le.egg/sourmash/cli

@ctb
Copy link
Contributor

ctb commented May 8, 2021 via email

@olgabot
Copy link
Collaborator Author

olgabot commented May 9, 2021

@pranathivemuri Hmm there's still issues with pickling - do you know what would be the best way around this?

(nf-core--kmermaid-1.1.0dev)
 ✘  Sat  8 May - 10:35  /data_lg/olga/tmp 
 olga@hulk  sourmash compare --processes 120 --csv /home/olga/data_sm/immune-evolution/kmer-signatures/5--sourmash-compare/alphabet-DNA__ksize-21__scaled-10 --dna --ksize 21 /home/olga/data_sm/immune-evolution/kmer-signatures/2--test-human/1--single-cell-sigs/alphabet-DNA__ksize-21__scaled-10 /home/olga/data_sm/immune-evolution/kmer-signatures/3--test-bat/1--single-cell-sigs/alphabet-DNA__ksize-21__scaled-10 /home/olga/data_sm/immune-evolution/kmer-signatures/1--train-mouse/1--single-cell-sigs/alphabet-DNA__ksize-21__scaled-10

== This is sourmash version 0.0.0. ==
== Please cite Brown and Irber (2016), doi:10.21105/joss.00027. ==

<<<natures/2--test-human/1--single-cell-sigs/alphabet-DNA__ksize-21__scaled-10'
<<<ignatures/3--test-bat/1--single-cell-sigs/alphabet-DNA__ksize-21__scaled-10'
<<<atures/1--train-mouse/1--single-cell-sigs/alphabet-DNA__ksize-21__scaled-10'
loaded 27812 signatures total.

Created memmapped siglist
Initialized memmapped similarities matrix
Calculated chunk size for multiprocessing
Initialized multiprocessing pool.imap
Traceback (most recent call last):
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/bin/sourmash", line 11, in <module>
    load_entry_point('sourmash==0.0.0', 'console_scripts', 'sourmash')()
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/site-packages/sourmash-0.0.0-py3.7-linux-ppc64le.egg/sourmash/__main__.py", line 13, in main
    return mainmethod(args)
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/site-packages/sourmash-0.0.0-py3.7-linux-ppc64le.egg/sourmash/cli/compare.py", line 54, in main
    return sourmash.commands.compare(args)
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/site-packages/sourmash-0.0.0-py3.7-linux-ppc64le.egg/sourmash/commands.py", line 142, in compare
    n_jobs=args.processes)
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/site-packages/sourmash-0.0.0-py3.7-linux-ppc64le.egg/sourmash/compare.py", line 217, in compare_all_pairs
    similarities = compare_parallel(siglist, ignore_abundance, downsample, n_jobs)
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/site-packages/sourmash-0.0.0-py3.7-linux-ppc64le.egg/sourmash/compare.py", line 178, in compare_parallel
    for index, l in enumerate(result):
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/multiprocessing/pool.py", line 325, in <genexpr>
    return (item for chunk in result for item in chunk)
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/multiprocessing/pool.py", line 748, in next
    raise value
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/multiprocessing/pool.py", line 431, in _handle_tasks
    put(task)
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/multiprocessing/connection.py", line 206, in send
    self._send_bytes(_ForkingPickler.dumps(obj))
  File "/data_sm/home/olga_ibm/miniconda3/envs/nf-core--kmermaid-1.1.0dev/lib/python3.7/multiprocessing/reduction.py", line 51, in dumps
    cls(buf, protocol).dump(obj)
AttributeError: Can't pickle local object 'compare_parallel.<locals>.get_similarities_at_index'

@ctb
Copy link
Contributor

ctb commented May 9, 2021

the version stuff can be fixed with pip install setuptools_scm, I believe. Here's the link to when it first popped up in release 4.0.0rc1: #1337 (comment)

@pranathivemuri
Copy link
Contributor

pranathivemuri commented May 9, 2021

@olgabot could you please try now? By using nested function and the shared variables in the outer function I am trying to avoid serialization of siglist that seems to take up a lot of memory as per your issue #1486

@olgabot
Copy link
Collaborator Author

olgabot commented May 12, 2021

This version is working for me!

@pranathivemuri
Copy link
Contributor

@ctb @olgabot should we rebase and get a review?

@ctb
Copy link
Contributor

ctb commented May 26, 2021

@ctb @olgabot should we rebase and get a review?

yes, please!

@ctb
Copy link
Contributor

ctb commented Sep 6, 2022

I might pick this up now that I grok some of the code more b/c of #2265. See also expert commentary here.

@czbiohub-sf czbiohub-sf closed this by deleting the head repository Mar 17, 2023
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.

4 participants