Skip to content

Conversation

@ceriottm
Copy link
Contributor

@ceriottm ceriottm commented May 2, 2021

Let's open - at least as a playground - this can of worms. Is it possible with manageable effort to get openmp parallelization of the loop over centers that appear in all of the time-consuming step of the calculation?

Rationale and detailed description of the changes:

  • added the minimal amount of infrastructure to Iterators to get parallel for loops to work
  • added placeholders for the loops that could be parallelized
  • at the moment, any actual parallelization breaks all the tests

Tasks before review:

  • Add tests, examples, and complete documentation of any added functionality
    • Make sure the autogenerated documentation appears in Sphinx and is
      formatted correctly (ask @max-veit if you need help with this task).
    • Write additional documentation in the Sphinx (not docstrings!) to
      explain the feature and its usage in plain English
    • Make sure the examples run (!) and produce the expected output
    • For bugfix pull requests, list here which tests have been added or re-enabled
      to verify the fix and catch future regressions as well as similar bugs
      elsewhere in the code.
  • Run make lint on the project, ensure it passes
    • Run make pretty-cpp and make pretty-python, check that the
      auto-formatting is sensible
    • Review variable names, make sure they're descriptive (ask a friend)
    • Review all TODOs, convert to issues wherever possible
    • Make sure draft, in-progress, and commented-out code is moved to its
      own branch
  • If committing any code in Jupyter/IPython notebooks, install and run nbstripout
  • Merge with master and resolve any conflicts
  • (anything else that's still in progress)

Remaining tasks, out of scope of this pull request:

Parallelization across nodes, best left to the calling code's domain decomposition

ceriottm added 2 commits May 1, 2021 23:38
this is not a superficial restructure, because loops over centers are not thread-safe
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.

2 participants