Parallelize Sampling of LDT

[kareef928]

Reference Issues/PRs

Fixes #520

What does this implement/fix? Briefly explain your changes.

Parallelizes the sampling in the latent distribution test

Any other comments?

Notebook showing runtime comparison: https://nbviewer.jupyter.org/github/kareef928/graspologic-personal/blob/main/notebooks/ldt-parallel-sim.ipynb

[netlify]

Deploy Preview for graspologic failed.

Built with commit bcc4466

https://app.netlify.com/sites/graspologic/deploys/60a5729eba61610007da2e30

[asaadeldin11]

@kareef928 if there's randomness associated with the parallelization, add a random_state parameter for reproducibility. See how GraphMatch deals with this for reference. If you have any questions let me know.

[kareef928]

@asaadeldin11 something like this right? (I tested the _sample_modified_ase method and got reproducibility from it)

[asaadeldin11]

Good job Kareef. Make these small changes and you'll be good to go.

[bdpedigo]

@asaadeldin11 not sure about the n_jobs change - i do want to standardize (see #755, and https://github.com/microsoft/graspologic/pull/710/files#r580257172) but would have to do for both, so this would be an API-breaking change.

[bdpedigo]

just talked to @dwaynepryce, if we do want it to be n_jobs for both then we just have to bump the major version in setup.cfg

[asaadeldin11]

just talked to @dwaynepryce, if we do want it to be n_jobs for both then we just have to bump the major version in setup.cfg

I don't have a preference, we can also just change the parameter name in GraphMatch to workers if that's easier?

[daxpryce]

yep!

[bdpedigo]

n_jobs is my preference overall if using joblib just because that is actually what joblib calls it and we can steal their docs

[daxpryce]

conversely I'd prefer workers because I can parallelize leiden and I'm probably going to use threads or workers, not n_jobs

[daxpryce]

So really if I have to choose right now, my preferential order is:

1. threads
2. workers
3. n_jobs

[bdpedigo]

in that case, for the purposes of this PR, let's leave as workers, not change anything for LPT, and we can bring this discussion up (one of many) in #755

[bdpedigo]

@kareef928 for some reason there are conflicts here