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

WIP: Refactor Load Balancing #541

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

Conversation

squarefk
Copy link

The goal of this pull request is to generalize SparseDimPartitioner into more scenarios, to support:

  1. Both 2D and 3D
  2. SparseMap, DenseMap, ParticlesView
  3. Performance comparison with ALL load balancing

@streeve
Copy link
Member

streeve commented Jun 14, 2022

Jenkins is failing because of the benchmark CMakeLists - I can start the other tests after that fix.

This is a great start! I think we still want some separation of the sparse partitioning from the particle version, maybe a derived class. Particularly because the tile concept does not exist in the dense grid

I like the DynamicPartitioner naming - we could eventually rename the existing as StaticPartitioner and ALLDynamicPartitioner

@squarefk squarefk changed the title Refactor Load Balancing WIP: Refactor Load Balancing Jun 14, 2022
@squarefk
Copy link
Author

Jenkins is failing because of the benchmark CMakeLists - I can start the other tests after that fix.

This is a great start! I think we still want some separation of the sparse partitioning from the particle version, maybe a derived class. Particularly because the tile concept does not exist in the dense grid

I like the DynamicPartitioner naming - we could eventually rename the existing as StaticPartitioner and ALLDynamicPartitioner

@streeve That makes sense. Since particles and sparse map have quite different parameters to pass in the function, I am not sure how the derived class works here to address. Neither constructor does the job I guess.

cajita/unit_test/tstDynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/unit_test/tstDynamicPartitioner.hpp Outdated Show resolved Hide resolved
benchmark/cajita/Cajita_DynamicPartitionerPerformance.cpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Show resolved Hide resolved
Copy link
Member

@streeve streeve left a comment

Choose a reason for hiding this comment

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

Looks like you'll need to merge/rebase master to fix the CI (error from the merged SparseLocalGrid

cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_DynamicPartitioner.hpp Outdated Show resolved Hide resolved
cajita/src/Cajita_ParticleDynamicPartitioner.hpp Outdated Show resolved Hide resolved
Copy link
Member

@streeve streeve left a comment

Choose a reason for hiding this comment

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

You can run make doxygen from your build to check for missing docs

@codecov
Copy link

codecov bot commented Aug 8, 2022

Codecov Report

Merging #541 (4698e0c) into master (b47a695) will increase coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head 4698e0c differs from pull request most recent head 7433ca3. Consider uploading reports for the commit 7433ca3 to get more accurate results

@@          Coverage Diff           @@
##           master    #541   +/-   ##
======================================
  Coverage    94.8%   94.8%           
======================================
  Files          47      49    +2     
  Lines        5633    5636    +3     
======================================
+ Hits         5341    5345    +4     
+ Misses        292     291    -1     
Impacted Files Coverage Δ
cajita/src/Cajita_DynamicPartitioner.hpp 98.4% <100.0%> (ø)
cajita/src/Cajita_ParticleDynamicPartitioner.hpp 100.0% <100.0%> (ø)
cajita/src/Cajita_SparseMapDynamicPartitioner.hpp 100.0% <100.0%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

This was referenced Aug 25, 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.

3 participants