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

Distributed FMM with complete tree structure on each rank #8

Closed

Conversation

gaohao95
Copy link
Contributor

@gaohao95 gaohao95 commented Oct 22, 2019

TODO List:

Must do:

  • Integrate pytest with MPI for CI.
  • Remove exception tag of this branch from CI jobs.
  • Merge constantone test case into test_distributed.py.
  • Add a documentation webpage.
  • Adapt calculate_pot for Sumpy.

Maybe:

  • Make all public API inside __init__.py.
  • Think about whether we can decrease device memory usage for box_mpole_is_used.
  • Unified array with both device and host pointer.

.gitlab-ci.yml Show resolved Hide resolved
boxtree/fmm.py Outdated Show resolved Hide resolved
boxtree/distributed/__init__.py Show resolved Hide resolved
boxtree/distributed/__init__.py Outdated Show resolved Hide resolved
boxtree/distributed/__init__.py Show resolved Hide resolved
@inducer inducer force-pushed the no-tree-in-wrangler branch 2 times, most recently from ddb2065 to 5d13e04 Compare July 1, 2021 03:36
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Hao, thanks for working on this! Here's a bit more progress on review.

FYI, the "tree-dependent data" PRs:

are done and should be landing soon.

boxtree/distributed/__init__.py Outdated Show resolved Hide resolved
boxtree/distributed/partition.py Show resolved Hide resolved
boxtree/distributed/partition.py Outdated Show resolved Hide resolved
boxtree/distributed/partition.py Outdated Show resolved Hide resolved
boxtree/distributed/partition.py Outdated Show resolved Hide resolved
boxtree/tools.py Outdated Show resolved Hide resolved
boxtree/traversal.py Outdated Show resolved Hide resolved
boxtree/traversal.py Outdated Show resolved Hide resolved
boxtree/tools.py Show resolved Hide resolved
boxtree/fmm.py Outdated Show resolved Hide resolved
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as discussed during meeting)

boxtree/distributed/calculation.py Outdated Show resolved Hide resolved
calibration_params = \
FMMCostModel.get_unit_calibration_params()

# We need to construct a wrangler in order to access `level_nterms`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ wrangler being created just for that purpose

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unbelievable. I actually remembered to submit comments this time.

boxtree/tools.py Outdated Show resolved Hide resolved
boxtree/tools.py Show resolved Hide resolved
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! A few more comments below, largely minor. In terms of review progress, I've now read everything except

  • local_traversal
  • local_tree
  • partition
  • traversal

doc/distributed.rst Outdated Show resolved Hide resolved
boxtree/fmm.py Outdated Show resolved Hide resolved
boxtree/fmm.py Show resolved Hide resolved
boxtree/fmm.py Outdated Show resolved Hide resolved
boxtree/fmm.py Outdated Show resolved Hide resolved
boxtree/distributed/__init__.py Show resolved Hide resolved
boxtree/distributed/__init__.py Show resolved Hide resolved
boxtree/distributed/calculation.py Outdated Show resolved Hide resolved
boxtree/distributed/calculation.py Outdated Show resolved Hide resolved
boxtree/distributed/partition.py Outdated Show resolved Hide resolved
@inducer
Copy link
Owner

inducer commented Dec 14, 2021

FYI I just rebased inducer/sumpy#70 to bring it up to date with respect to the latest sumpy (actually just redid the patch, the merge didn't really work).

@inducer inducer deleted the branch inducer:no-tree-in-wrangler December 14, 2021 17:37
@inducer inducer closed this Dec 14, 2021
@inducer
Copy link
Owner

inducer commented Dec 14, 2021

Whoops! I did not mean to close this. I think what happened here is that I merged the wrangler refactor (#29), which served as the base branch of this, and because that branch got deleted, it closed this PR. @gaohao95, could you open a new PR off of your branch that targets main and links to this? Sorry about the fuss.

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