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.

Sign up for GitHub

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

Jump to bottom

DummyParticles: Main: Mark particles as deleted on deletion. #474

Merged
merged 73 commits into from
Jul 28, 2020
Merged

DummyParticles: Main: Mark particles as deleted on deletion. #474

merged 73 commits into from
Jul 28, 2020

Conversation

SteffenSeckler
Copy link
Member

@SteffenSeckler SteffenSeckler commented May 26, 2020 •
edited

Description

  • removed RMM because why not...
  • don't actually delete particles, but instead, mark them as dummy/deleted.
  • [Jenkinsfile] remove threadsanitizer because it is fully covered by archer
  • dummy particles need to be removed on a rebuild of the container.
  • Fixes incorrect handling of masks in LJFunctorAVX::SoAKernel(). upot was used instead of the masked version (only for non-cell-wise-owned-state)
  • Adapt functors to ignore dummy particles:
    • LJFunctor
    • LJFunctorAVX
    • LJFunctorCuda
    • LJFunctorCudaGlobals
    • FlopCounterFunctor <-- do we need that? I don't really care about it...
    • SPHDensityFunctor
    • SPHForceFunctor
  • Deleting particles using AutoPas::deleteParticle no longer invalidates the neighbor lists.

Related Pull Requests

Resolved Issues

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

  • Test if updateContainer() removes dummy particles.
  • Test if newly deleted particles are not still used with SoA <-- This is tested through the test TraversalComparison. While the particles are still inside of the SoA, they are explicitly skipped inside of the SoAFunctors.
  • Test with dummies to LJFunctorAVXTest

@SteffenSeckler SteffenSeckler changed the title Make particles marked as deleted on deletion. Mark particles as deleted on deletion. May 29, 2020
@lgtm-com
Copy link

lgtm-com bot commented May 29, 2020

This pull request introduces 1 alert when merging edb14d2 into a2a71c7 - view on LGTM.com

new alerts:

  • 1 for Unsigned comparison to zero

@lgtm-com
Copy link

lgtm-com bot commented Jun 5, 2020

This pull request introduces 1 alert when merging 3ad1e4b into cb22dd6 - view on LGTM.com

new alerts:

  • 1 for Unsigned comparison to zero

@SteffenSeckler
fixes underflow in Cluster.h
a1407d7
@SteffenSeckler
dummies :)
c700165 
* introduces HaloOwnedAndDummy iterator behavior
* use proper dummy state in SoA

disclaimer:
AVXFunctor and sph functors not yet touched.
@lunaticcoding
Copy link

@FG-TUM @SteffenSeckler Wie schauts mit dem branch aus? :S I bin von dem geblockt :S isch des soweit, dass i n in meinen rein mergen kann? :)

@SteffenSeckler
Copy link
Member Author

awkward language you are speaking 😋
There isn't missing much. All functors except for the LJFunctor need adaption for the new things, so it will currently only compile if you disable AVX. You can manually set the vectorization either to off or to SSE, then it should work.
So you can merge it to your branch, but the tests will most likely fail.

Or you can wait until @FG-TUM adapts the LJFunctorAVX...

Cuda stuff is of course also not adapted, but well...

@lunaticcoding
Copy link

@SteffenSeckler Sorry ;D My bad. I think I'll just merge it now and implement my stuff and then wait for @FG-TUM to wrap it up before diving into testing :) thanks :)

@SteffenSeckler
fix avx test
a9540c3
@SteffenSeckler
compiling avx version, tests are still failing
f3cad20
@SteffenSeckler
fixes wrong blend operation
8bbb235
@SteffenSeckler
some of the formats
003b3e7
@SteffenSeckler
Merge remote-tracking branch 'origin/master' into feature/mark_deleted
96a4892 
# Conflicts:
#	src/autopas/containers/linkedCells/LinkedCells.h
#	src/autopas/containers/verletClusterLists/ClusterTower.h
#	src/autopas/containers/verletClusterLists/VerletClusterListsRebuilder.h
#	tests/testAutopas/tests/containers/AllContainersTests.h
@SteffenSeckler
all the formats
2f1816a
@SteffenSeckler
two merge errors
79318be
@SteffenSeckler SteffenSeckler force-pushed the feature/mark_deleted branch 2 times, most recently from 4252b72 to f13f3c2 Compare June 16, 2020 12:56
@SteffenSeckler
Copy link
Member Author

@FG-TUM do we still need the FlopCounterFunctor? And if yes, should this be adapted or not?

@SteffenSeckler
Copy link
Member Author

@lunaticcoding this PR now includes the necessary changes for cuda. Only SPH is missing at this point (which can be found in #494 )

@SteffenSeckler SteffenSeckler added test-performance BETA: Attaching label runs performance measurements for all commits up to that point and removed test-performance BETA: Attaching label runs performance measurements for all commits up to that point labels Jun 23, 2020
@SteffenSeckler SteffenSeckler added test-performance BETA: Attaching label runs performance measurements for all commits up to that point and removed test-performance BETA: Attaching label runs performance measurements for all commits up to that point labels Jul 21, 2020
@SteffenSeckler SteffenSeckler added test-performance BETA: Attaching label runs performance measurements for all commits up to that point and removed test-performance BETA: Attaching label runs performance measurements for all commits up to that point labels Jul 22, 2020
FG-TUM
FG-TUM approved these changes Jul 27, 2020
View reviewed changes
Copy link
Member

@FG-TUM FG-TUM left a comment

Choose a reason for hiding this comment

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

Mostly cleanup of debugging leftovers

docs/Doxyfile.in Outdated Show resolved Hide resolved
src/autopas/containers/verletClusterLists/Cluster.h Outdated Show resolved Hide resolved
src/autopas/containers/verletClusterLists/Cluster.h Outdated Show resolved Hide resolved
src/autopas/containers/verletClusterLists/Cluster.h Outdated Show resolved Hide resolved
src/autopas/molecularDynamics/LJFunctor.h Outdated Show resolved Hide resolved
src/autopas/sph/SPHCalcHydroForceFunctor.h Outdated Show resolved Hide resolved
src/autopas/sph/SPHCalcHydroForceFunctor.h Outdated Show resolved Hide resolved
src/autopas/sph/SPHCalcHydroForceFunctor.h Outdated Show resolved Hide resolved
tests/testAutopas/tests/molecularDynamics/LJFunctorAVXTest.cpp Outdated Show resolved Hide resolved
tools/autopasTools/generators/RandomGenerator.h Outdated Show resolved Hide resolved
@SteffenSeckler SteffenSeckler merged commit 637c2e2 into master Jul 28, 2020
@SteffenSeckler SteffenSeckler deleted the feature/mark_deleted branch July 28, 2020 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FG-TUM FG-TUM FG-TUM approved these changes

Assignees
No one assigned
Labels
epic refactoring test-performance BETA: Attaching label runs performance measurements for all commits up to that point
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SteffenSeckler @lunaticcoding @FG-TUM