Skip to content
Snippets Groups Projects

Rebuild criteria

Merged Rebuild criteria
rebuild_criteria into master
Merged Pedro Gonnet requested to merge rebuild_criteria into master

Instead of rebuilding whenever the cell constraints or sort indices are violated, just re-sort in case of the latter.

This lead to some other changes:

  • Init tasks are gone, particle initialization is now handled in the drift tasks,
  • Drift tasks no longer act on the super-cell, but exist at every level similar to the sort tasks,
  • Fixed several dependency issues with sort and drift tasks,
  • Only drift/sort on-demand in the sub-cell tasks.

Marked as [WIP] as this has not yet been tested for MPI runs, or with gravity.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
src/cell.c
926 930 }
927 931 }
928 932
929 /* Check whether there was too much particle motion */
933 /* Only interested in pair interactions as of here. */
930 934 if (t->type == task_type_pair || t->type == task_type_sub_pair) {
935
936 /* Check whether there was too much particle motion, i.e. the
937 cell neighbour conditions were violated. */
931 938 if (t->tight &&
932 (max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin ||
933 ci->dx_max > space_maxreldx * ci->h_max ||
934 cj->dx_max > space_maxreldx * cj->h_max))
939 max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin)
  • Matthieu Schaller
    Matthieu Schaller @matthieu
    Owner

    Isn't this condition too extreme ? Or at least shouldn't we consider the min of ci->dmin and cj->dmin on the R.H.S. ? Wasn't space_stretch designed for this ?

    Also, t->tight is only set for the pairs/sub-pairs and is always set for them. Any reason to keep it as a separate property ?

  • Pedro Gonnet
    Pedro Gonnet @nnrw56 started a thread on commit 473d5f9a
    • src/cell.c
    926 930 }
    927 931 }
    928 932
    929 /* Check whether there was too much particle motion */
    933 /* Only interested in pair interactions as of here. */
    930 934 if (t->type == task_type_pair || t->type == task_type_sub_pair) {
    935
    936 /* Check whether there was too much particle motion, i.e. the
    937 cell neighbour conditions were violated. */
    931 938 if (t->tight &&
    932 (max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin ||
    933 ci->dx_max > space_maxreldx * ci->h_max ||
    934 cj->dx_max > space_maxreldx * cj->h_max))
    939 max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin)
    • Pedro Gonnet
      Pedro Gonnet @nnrw56
      Author Developer

      In what sense too extreme? This just checks that for the two largest particles in the cell, the distance constraints are not violated. space_stretch just sets the minimum fraction for the sum of the maximum particle movement in both cells, i.e. the amount of elasticity to allow before this constraint is violated.

      Regarding t->tight, you're right. Can you file an issue for that?

  • Pedro Gonnet
    Pedro Gonnet @nnrw56 started a thread on commit 473d5f9a
    • src/cell.c
    926 930 }
    927 931 }
    928 932
    929 /* Check whether there was too much particle motion */
    933 /* Only interested in pair interactions as of here. */
    930 934 if (t->type == task_type_pair || t->type == task_type_sub_pair) {
    935
    936 /* Check whether there was too much particle motion, i.e. the
    937 cell neighbour conditions were violated. */
    931 938 if (t->tight &&
    932 (max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin ||
    933 ci->dx_max > space_maxreldx * ci->h_max ||
    934 cj->dx_max > space_maxreldx * cj->h_max))
    939 max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin)
  • Matthieu Schaller
    Matthieu Schaller @matthieu started a thread on commit 473d5f9a
    • src/cell.c
    926 930 }
    927 931 }
    928 932
    929 /* Check whether there was too much particle motion */
    933 /* Only interested in pair interactions as of here. */
    930 934 if (t->type == task_type_pair || t->type == task_type_sub_pair) {
    935
    936 /* Check whether there was too much particle motion, i.e. the
    937 cell neighbour conditions were violated. */
    931 938 if (t->tight &&
    932 (max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin ||
    933 ci->dx_max > space_maxreldx * ci->h_max ||
    934 cj->dx_max > space_maxreldx * cj->h_max))
    939 max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin)
    • Matthieu Schaller
      Matthieu Schaller @matthieu
      Owner

      Maybe that was poorly formulated on my side. Shouldn't the condition be symmetric in i<->j ? Something like min(ci->dmin, cj->dmin) on the right ?

  • Pedro Gonnet
    Pedro Gonnet @nnrw56 started a thread on commit 473d5f9a
    • src/cell.c
    926 930 }
    927 931 }
    928 932
    929 /* Check whether there was too much particle motion */
    933 /* Only interested in pair interactions as of here. */
    930 934 if (t->type == task_type_pair || t->type == task_type_sub_pair) {
    935
    936 /* Check whether there was too much particle motion, i.e. the
    937 cell neighbour conditions were violated. */
    931 938 if (t->tight &&
    932 (max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin ||
    933 ci->dx_max > space_maxreldx * ci->h_max ||
    934 cj->dx_max > space_maxreldx * cj->h_max))
    939 max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin)
    • Pedro Gonnet
      Pedro Gonnet @nnrw56
      Author Developer

      Ah, OK, so ci->dmin and cj->dmin should be equal since we only ever interact two cells that have the same size. No idea why I chose cj in this statement, it shouldn't really matter :smiley:

  • Matthieu Schaller
    Matthieu Schaller @matthieu started a thread on commit 473d5f9a
    • src/cell.c
    926 930 }
    927 931 }
    928 932
    929 /* Check whether there was too much particle motion */
    933 /* Only interested in pair interactions as of here. */
    930 934 if (t->type == task_type_pair || t->type == task_type_sub_pair) {
    935
    936 /* Check whether there was too much particle motion, i.e. the
    937 cell neighbour conditions were violated. */
    931 938 if (t->tight &&
    932 (max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin ||
    933 ci->dx_max > space_maxreldx * ci->h_max ||
    934 cj->dx_max > space_maxreldx * cj->h_max))
    939 max(ci->h_max, cj->h_max) + ci->dx_max + cj->dx_max > cj->dmin)
  • Matthieu Schaller Added 166 commits:

    Added 166 commits:

  • Matthieu Schaller
    Matthieu Schaller @matthieu
    Owner

    I have merged in the latest master to allow me to do a clean diff. If you disagree, feel free to revert the last commit.

  • Matthieu Schaller
  • Matthieu Schaller
    Matthieu Schaller @matthieu started a thread on the diff
    • src/engine.c
    1886 1882 struct scheduler *sched, struct task *gravity, struct cell *c) {
    1887 1883
    1888 1884 /* init --> gravity --> grav_down --> kick */
    1889 scheduler_addunlock(sched, c->super->init, gravity);
    1885 scheduler_addunlock(sched, c->ghost, gravity);
    1890 1886 scheduler_addunlock(sched, gravity, c->super->grav_down);
  • Matthieu Schaller
    Matthieu Schaller @matthieu started a thread on the diff
    • src/engine.c
    1998 1994 struct scheduler *sched, struct task *density, struct task *gradient,
    1999 1995 struct task *force, struct cell *c, int with_cooling) {
    2000 1996
    2001 /* init --> density loop --> ghost --> gradient loop --> extra_ghost */
    1997 /* density loop --> ghost --> gradient loop --> extra_ghost */
    2002 1998 /* extra_ghost --> force loop */
    2003 scheduler_addunlock(sched, c->super->init, density);
    2004 1999 scheduler_addunlock(sched, density, c->super->ghost);
  • Matthieu Schaller
  • Matthieu Schaller
    Matthieu Schaller @matthieu started a thread on the diff
    • src/space.h
    45 45 #define space_maxcount_default 10000
    46 46 #define space_max_top_level_cells_default 12
    47 47 #define space_stretch 1.10f
    48 #define space_maxreldx 0.25f
    48 #define space_maxreldx 0.1f
    49 49
  • Matthieu Schaller mentioned in issue #280 (closed)

    mentioned in issue #280 (closed)

  • Matthieu Schaller Added 13 commits:

    Added 13 commits:

  • Matthieu Schaller Added 1 commit:

    Added 1 commit:

    • 407d24be - Removed last occurence of the init task
  • James Willis Added 3 commits:

    Added 3 commits:

    • c1f9d59c - Script to plot the scaling breakdown of functions.
    • b9cf373a - Updated scaling script.
    • 3693bad2 - Merge branch 'rebuild_criteria' of gitlab.cosma.dur.ac.uk:swift/swiftsim into rebuild_criteria
  • Matthieu Schaller Added 3 commits:

    Added 3 commits:

  • Matthieu Schaller
    Matthieu Schaller @matthieu
    Owner

    @nnrw56 would you like to have this properly tested ?

  • Pedro Gonnet
    Pedro Gonnet @nnrw56
    Author Developer

    @matthieu yes, that would be great! but it's probably best if i address the first round of comments first though...

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
    • Please register or sign in to reply