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

Make parallel test2_global, local and power #11

Open
astamm opened this issue Apr 11, 2022 · 4 comments
Open

Make parallel test2_global, local and power #11

astamm opened this issue Apr 11, 2022 · 4 comments

Comments

@astamm
Copy link
Collaborator

astamm commented Apr 11, 2022

No description provided.

@danielemule
Copy link

I have parallelized power2() via futures (future.apply and progressr packages).
Moreover, I have parallelized the distance matrix computation in dist_nvd() and, consequently, in test2_global() via futures (only for the "match-frobenius" distance).

You can find my version of these functions in my graphmatch branch of nevada (link).

@astamm
Copy link
Collaborator Author

astamm commented May 16, 2022

I took a look at what you did. You learned the futureverse quite well but you have written code as an end-user would, not as a package developer. You should not have any call to future::plan() inside the code of the package. Furthermore, you chose to use the future.apply package which parallelises replicate() which is not optimal. Using furrr you parallelise purrr::map() which is already a speed improvement of replicate().

You should create a new branch on your fork specific to this issue and initiate a pull request. That way, I will be able to take over. Thanks for starting this!

@danielemule
Copy link

OK, thanks for your advice. As I understand it, it is not necessary to put any function argument related to parallelization tasks and so any call to future::plan() should be specified by the end-user before calling nevada functions, shouldn't it?

@astamm
Copy link
Collaborator Author

astamm commented May 17, 2022

Correct. All you need to do in the code is calling furrr::future_map() or future.apply::replicate() instead of lapply() or replicate() and let the end-user decide whether (s)he wants to turn on parallelisation.

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

No branches or pull requests

2 participants