Skip to content
Snippets Groups Projects

Do not collect and store ti_end_max since we never make use of it for anything.

Merged Matthieu Schaller requested to merge no_ti_end_max into master

We collect and store ti_end_max for each particle type but never make use of it. We used to but that is not the case anymore as there is no real gain to be had anywhere.

Do you agree that it's sensible to remove it and hence shave off on the memory of the cells and pcell communications?

Merge request reports

Merged by Matthieu SchallerMatthieu Schaller 4 years ago (Mar 28, 2021 9:55pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • No rush on this as it is largely orthogonal to other developments but just wondering if you had any thoughts.

  • Yes, let's do this. Good for code reduction if nothing else. I see an unrelated union slipped in. Are we sure that is safe?

  • Ah yes, that made it alongside the rest. This is innocent in principle. We only use next when storing the cells in a linked-list before usage. We can take this separately if you think that's safer.

  • I'll take your word for that. Trying to run this on a simple case and it is segfaulting here:

    (gdb) where
    #0  0x000055fdfe87837b in engine_addtasks_recv_hydro (e=0x7ffe05fc3ba0, c=0x55fe002d0540, t_xv=0x55fe029660e0, 
        t_rho=0x55fe02966140, t_gradient=0x55fe029661a0, t_ti=0x55fe02966200, t_prep1=0x0, t_limiter=0x0, with_feedback=0, 
        with_limiter=0, with_sync=0) at engine_maketasks.c:635
    #1  0x000055fdfe8828f6 in engine_addtasks_recv_mapper (map_data=0x55fe008a9e50, num_elements=219, extra_data=0x7ffe05fc3ba0)
        at engine_maketasks.c:3956
    #2  0x000055fdfe8f55b1 in threadpool_map (tp=0x7ffe05fc3cb0, map_function=0x55fdfe8827f0 <engine_addtasks_recv_mapper>, 
        map_data=0x55fe008a9e50, N=219, stride=24, chunk=0, extra_data=0x7ffe05fc3ba0) at threadpool.c:283
    #3  0x000055fdfe883e7a in engine_maketasks (e=0x7ffe05fc3ba0) at engine_maketasks.c:4328
    #4  0x000055fdfe870315 in engine_rebuild (e=0x7ffe05fc3ba0, repartitioned=0, clean_smoothing_length_values=0) at engine.c:1295
    #5  0x000055fdfe8714a4 in engine_init_particles (e=0x7ffe05fc3ba0, flag_entropy_ICs=0, clean_h_values=0) at engine.c:1743
    #6  0x000055fdfe84fe48 in main (argc=6, argv=0x7ffe05fc6348) at main.c:1496
    (gdb) list
    630         }
    631     #endif
    632         /* Make sure the part have been received before the BHs compute their
    633          * accretion rates (depends on particles' rho). */
    634         for (struct link *l = c->black_holes.density; l != NULL; l = l->next) {
    635           scheduler_addunlock(s, t_rho, l->t);
    636         }
    637       }
    638

    Odd as the command is:

    mpirun -np 4 ../../swift_mpi --hydro --threads=2 -n 16 eagle_6.yml

    and no blackholes are loaded, l is not NULL.

  • Checking master...

  • That is fine, so kicking this back to you.

  • BTW, please merge latest master, tried that and didn't help.

  • Thanks. Should I have caught that one. I'll investigate.

  • Matthieu Schaller added 80 commits

    added 80 commits

    • bb4117ef...5db55f88 - 48 commits from branch master
    • e7055a08 - Split the EAGLE feedback files into kinetic and thermal modes
    • 0dc0e43a - Rename the 'old' stellar task to 'star density ghost'
    • 061cc06d - Added empty functions to hold the content of the new star->gas interactions
    • 3316a287 - Added empty functions to hold the content of the new star->gas interactions
    • c9a3063a - Move the common enrichment-related feedback routines to a single file shared...
    • 967beb07 - Implement the double-kick mitigation measure
    • 88e73033 - change switch to enum and move to ray struct
    • 1a52b3ef - Split the EAGLE feedback files into kinetic and thermal modes
    • c85535b5 - Added a BAHAMAS master configuration and runtime mode
    • 531d3997 - Remove the calculation of subgrid properties in the COLIBRE cooling model
    • 95582215 - Remove the particle-carried metal masses from specific star channels
    • ff405dfd - Remove unused star particle-carried properties related to the injection in specific channels
    • 51880b52 - Do not attempt to write the subgrid properties to the snapshots or the...
    • 7627f23d - Do not throw an error message in the functions computing subgrid properties. Just return 0.
    • 7fd7dcdc - Do not update the injection per channels in the EAGLE enrichment model
    • a8ea9f66 - Make the BAHAMAS master-switch configure the code with the EAGLE-kinetic feedback flavour.
    • 8f1d5892 - In the EAGLE kinetic feedback, remove the variables related to the stellar channel injections
    • 03765040 - Add a template reference BAHAMAS parameter file
    • f8a6b498 - Do not ask for the feedback mode in the EAGLE kinetic feedback case. It's always isotropic.
    • 8961e327 - Remove the boost-alpha-only mode of the EAGLE-AGN model
    • 13bcf15a - Add an example output list
    • 0f6d6f93 - Update the .gitignore to not complain about the new example file
    • 1a238053 - Post-rebase fixes
    • 5619cc8f - Post-rebase fixes. The kinetic feedback is now in the master of the main code,...
    • 4fba6735 - Hide all the cell-carried RT variables into a union to save memory.
    • 1cdde0aa - Do not carry the pointers related to spawning new star particles. We never do this in this model
    • f8708779 - Remove the gpart-SF lock in the cell structure since we are not spawing particles
    • f663e759 - Move the common enrichment-related feedback routines to a single file shared...
    • fb74e372 - Applied code formatting tool
    • f4bb1bbb - Remove two more unnecessary variables from the cell structure
    • 6f59a360 - Make the code abort if a very large temperature is reached
    • 46e1109f - Do not collect and store ti_end_max since we never make use of it for anything.

    Compare with previous version

  • added 1 commit

    • bb4117ef - Do not collect and store ti_end_max since we never make use of it for anything.

    Compare with previous version

  • Matthieu Schaller added 49 commits

    added 49 commits

    • bb4117ef...5db55f88 - 48 commits from branch master
    • 9cfd6ca9 - Do not collect and store ti_end_max since we never make use of it for anything.

    Compare with previous version

  • I can reproduce the problem. It's actually not related to these changes but rather to the changes introduced earlie where we added hidden unions in the cell structure to save memory. That wasn't correctly protected in some code paths and did not crash solely because the ti_end_max part of the union could end up with life-saving sensible values.

    In other words, master is also in danger but somewhat lucky.

    Proper fix underway.

  • Matthieu Schaller added 32 commits

    added 32 commits

    • e7055a08 - Split the EAGLE feedback files into kinetic and thermal modes
    • 0dc0e43a - Rename the 'old' stellar task to 'star density ghost'
    • 061cc06d - Added empty functions to hold the content of the new star->gas interactions
    • 3316a287 - Added empty functions to hold the content of the new star->gas interactions
    • c9a3063a - Move the common enrichment-related feedback routines to a single file shared...
    • 967beb07 - Implement the double-kick mitigation measure
    • 88e73033 - change switch to enum and move to ray struct
    • 1a52b3ef - Split the EAGLE feedback files into kinetic and thermal modes
    • c85535b5 - Added a BAHAMAS master configuration and runtime mode
    • 531d3997 - Remove the calculation of subgrid properties in the COLIBRE cooling model
    • 95582215 - Remove the particle-carried metal masses from specific star channels
    • ff405dfd - Remove unused star particle-carried properties related to the injection in specific channels
    • 51880b52 - Do not attempt to write the subgrid properties to the snapshots or the...
    • 7627f23d - Do not throw an error message in the functions computing subgrid properties. Just return 0.
    • 7fd7dcdc - Do not update the injection per channels in the EAGLE enrichment model
    • a8ea9f66 - Make the BAHAMAS master-switch configure the code with the EAGLE-kinetic feedback flavour.
    • 8f1d5892 - In the EAGLE kinetic feedback, remove the variables related to the stellar channel injections
    • 03765040 - Add a template reference BAHAMAS parameter file
    • f8a6b498 - Do not ask for the feedback mode in the EAGLE kinetic feedback case. It's always isotropic.
    • 8961e327 - Remove the boost-alpha-only mode of the EAGLE-AGN model
    • 13bcf15a - Add an example output list
    • 0f6d6f93 - Update the .gitignore to not complain about the new example file
    • 1a238053 - Post-rebase fixes
    • 5619cc8f - Post-rebase fixes. The kinetic feedback is now in the master of the main code,...
    • 4fba6735 - Hide all the cell-carried RT variables into a union to save memory.
    • 1cdde0aa - Do not carry the pointers related to spawning new star particles. We never do this in this model
    • f8708779 - Remove the gpart-SF lock in the cell structure since we are not spawing particles
    • f663e759 - Move the common enrichment-related feedback routines to a single file shared...
    • fb74e372 - Applied code formatting tool
    • f4bb1bbb - Remove two more unnecessary variables from the cell structure
    • 6f59a360 - Make the code abort if a very large temperature is reached
    • 46e1109f - Do not collect and store ti_end_max since we never make use of it for anything.

    Compare with previous version

  • added 1 commit

    • 9cfd6ca9 - Do not collect and store ti_end_max since we never make use of it for anything.

    Compare with previous version

  • added 1 commit

    • 831d5487 - Only construct the BH-related communications when we are running with the BH policy.

    Compare with previous version

  • mentioned in commit f88438ad

Please register or sign in to reply
Loading