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: Local particles #2400

Closed
wants to merge 62 commits into from
Closed

WIP: Local particles #2400

wants to merge 62 commits into from

Conversation

fweik
Copy link
Contributor

@fweik fweik commented Dec 4, 2018

Description of changes:

  • Replace the sparse array that was used for the local particle
    index by a hash map.
  • Adding particles when initializing a new cell system is now handled
    by the regular particle sorting code instead of duplicated code.

This finally makes Espresso linear in memory consumption.
If the performance with the std::unordered_map is too bad
we could replace it by a better one e.g. from abseil, but for
the reduced complexity in bookkeeping I really would like to
get this in.

@codecov
Copy link

codecov bot commented Dec 4, 2018

Codecov Report

Merging #2400 into python will decrease coverage by 1%.
The diff coverage is 90%.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #2400    +/-   ##
=======================================
- Coverage      77%     75%    -2%     
=======================================
  Files         498     494     -4     
  Lines       27891   27956    +65     
=======================================
- Hits        21492   21174   -318     
- Misses       6399    6782   +383
Impacted Files Coverage Δ
src/core/domain_decomposition.hpp 100% <ø> (ø) ⬆️
src/core/communication.hpp 100% <ø> (ø) ⬆️
src/core/cells.hpp 100% <ø> (ø) ⬆️
src/core/ParticleCache.hpp 100% <ø> (ø) ⬆️
src/core/io/mpiio/mpiio.cpp 0% <0%> (ø) ⬆️
src/core/virtual_sites.cpp 87% <100%> (ø) ⬆️
src/core/collision.cpp 79% <100%> (-1%) ⬇️
src/core/nsquare.cpp 99% <100%> (-1%) ⬇️
src/core/PartCfg.hpp 100% <100%> (ø) ⬆️
src/core/ghosts.cpp 87% <100%> (-1%) ⬇️
... and 67 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0438928...4e908b7. Read the comment docs.

@RudolfWeeber
Copy link
Contributor

  • for (auto c : local_cells) {
  • for (int i = 0; i < c->n; i++) {
  •  auto const &p = c->part[i];
    
  •  check_index_ghost_part(p);
    
    }
    }
    }
    Should that loop go over ghost cells?

@RudolfWeeber
Copy link
Contributor

Performance decreases by some 5% for a 1000 particle lj fluid at 0.1 volume fraction.
For 200 polymers of length 5 at 0.1 volume fraction, the slow down is some 15%.

@fweik fweik changed the title Local particles WIP: Local particles Jan 24, 2019
@fweik fweik added the Core label Jan 28, 2019
@espresso-ci
Copy link

Your pull request does not meet our code formatting rules. To fix this, please do one of the following:

  • You can download a patch with my suggested changes here, inspect it and make changes manually.
  • You can directly apply it to your repository by running curl https://gitlab.icp.uni-stuttgart.de/espressomd/espresso/-/jobs/85710/artifacts/raw/style.patch | git apply -.
  • You can run maintainer/CI/fix_style.sh to automatically fix your coding style. This is the same command that I have executed to generate the patch above, but it requires certain tools to be installed on your computer.

You can run gitlab-runner exec docker style afterwards to check if your changes worked out properly.

Please note that there are often multiple ways to correctly format code. As I am just a robot, I sometimes fail to identify the most aesthetically pleasing way. So please look over my suggested changes and adapt them where the style does not make sense.

@fweik fweik added this to the Espresso 4.1 milestone Mar 26, 2019
@fweik fweik closed this Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants