SWIFTsim merge requestshttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests2023-07-04T19:23:06Zhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1736Mhd canvas2023-07-04T19:23:06ZFederico Andrés StasyszynMhd canvasThe goal if this merge is to test the gravitational collapseThe goal if this merge is to test the gravitational collapseFederico Andrés StasyszynFederico Andrés Stasyszynhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1735Draft: Add host weights to METIS partitioning2023-09-11T10:47:06ZPeter W. DraperDraft: Add host weights to METIS partitioningSee #856 first implementation of the basics. Largely untested!See #856 first implementation of the basics. Largely untested!Peter W. DraperPeter W. Draperhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1734added some links and hints where to get dependency libraries2023-06-28T15:08:48ZMladen Ivkovicadded some links and hints where to get dependency librariesIt was brought to my attention that this would be an improvement. So here it is.It was brought to my attention that this would be an improvement. So here it is.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1733Fix GitHub Issue #342023-06-28T15:09:29ZJosh BorrowFix GitHub Issue #34Fixes https://github.com/SWIFTSIM/SWIFT/issues/34Fixes https://github.com/SWIFTSIM/SWIFT/issues/34Joey BraspenningJoey Braspenninghttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1732install clang-format using pip, don't expect users to have the correct one2023-10-07T09:14:25ZMladen Ivkovicinstall clang-format using pip, don't expect users to have the correct oneTurns out one can install clang-format through pip, just like the python formatter black. This is surely a better way, as it doesn't require users to provide the correct version themselves, and is portable. I modified `format.sh` to ins...Turns out one can install clang-format through pip, just like the python formatter black. This is surely a better way, as it doesn't require users to provide the correct version themselves, and is portable. I modified `format.sh` to install a specific version of clang-format now, and cleaned up and added additional checks for both format.sh and format_python.sh.
Usage of the scripts remains as-is. The option to override which clang-format exec to use via CLANG_FORMAT_CMD variable remains.
I successfully ran both formatting scripts on cosma after loading the python/3.10.1 module. It'll likely work with other python3 modules as well, but I didn't check.
I also switched to clang-format 16, because I can. It introduces very minimal changes (for commented out macros, it adds a space between the comment and the beginning of the macro). There's like 10-20 instances in 4-5 files. You might wanna run the formatting scripts immediately after merging this MR.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1731Deadlock detector2023-09-11T13:04:10ZMladen IvkovicDeadlock detectorI've been working on this as part of trying to solve issues with !1719 .
The idea is to allow swift to detect deadlocks. The scheduler keeps track of the time the last task was successfully fetched from a queue. If a user specified amou...I've been working on this as part of trying to solve issues with !1719 .
The idea is to allow swift to detect deadlocks. The scheduler keeps track of the time the last task was successfully fetched from a queue. If a user specified amount of time passes since the last successful fetch (tens of seconds, minutes...), swift assumes we're in a deadlock and aborts the mission.
I'm trying to use it to debug the issues in !1719. I thought this might be useful for others too, so here we are.
Marked as draft since it needs more testing on bigger examples, but it should be ready for a review.
I've prepared a second git branch with a ready-to-go example, where I manually tweaked cell_unskip.c to produce a deadlock.
How to run the example:
```
$ git checkout scheduler_timeout-proof-of-concept # note that this is not the same branch as in the MR
$ ./configure --with-rt=debug --with-stars=basic --with-feedback=none --enable-debugging-checks
$ make -j
$ cd examples/RadiativeTransferTests/UniformBox_3D
$ ./run.sh -m # -m flag executes with MPI
```
The deadlock should occur on step 3, after 10s of inactivity. The output should look something like this:
```
[0000] [00000.0] main: Running on 64000 gas particles, 0 sink particles, 8000 stars particles 0 black hole particles, 0 neutrino particles, and 0 DM particles (72000 gravity particles)
[0000] [00000.0] main: from t=0.000e+00 until t=9.537e-07 with 2 ranks, 4 threads / rank and 4 task queues / rank (dt_min=1.000e-12, dt_max=5.000e-08)...
[0000] [00000.1] engine_init_particles: Setting particles to a valid state...
[0000] [00000.1] engine_init_particles: Computing initial gas densities and approximate gravity.
[0000] [00000.1] space_rebuild: (re)building space
[0000] [00000.2] engine_init_particles: Converting internal energy variable.
[0000] [00000.2] engine_init_particles: Running initial fake time-step.
[0000] [00000.2] space_rebuild: (re)building space
0 cycle 0 (during regular tasks) dt= 7.450580e-09 min/max active bin= 1/56 rt_updates= 64000
0 cycle 1 time= 7.450580e-09 dt= 7.450580e-09 min/max active bin=49/49 rt_updates= 64000
0 cycle 2 time= 1.490116e-08 dt= 7.450580e-09 min/max active bin=49/50 rt_updates= 64000
0 cycle 3 time= 2.235174e-08 dt= 7.450580e-09 min/max active bin=49/49 rt_updates= 64000
[0000] [00000.6] engine_dump_snapshot: Dumping snapshot at t=0.000000e+00
[0000] [00000.6] engine_print_stats: Saving statistics at t=0.000000e+00
# Step Time Scale-factor Redshift Time-step Time-bins Updates g-Updates s-Updates sink-Updates b-Updates Wall-clock time [ms] Props Dead time [ms]
0 0.000000e+00 1.0000000 0.0000000 0.000000e+00 1 56 64000 72000 8000 0 0 526.688 27 34.313
[0000] [00000.6] space_rebuild: (re)building space
1 cycle 0 (during regular tasks) dt= 7.450580e-09 min/max active bin=49/51 rt_updates= 64000
1 cycle 1 time= 3.725290e-08 dt= 7.450580e-09 min/max active bin=49/49 rt_updates= 64000
1 cycle 2 time= 4.470348e-08 dt= 7.450580e-09 min/max active bin=49/50 rt_updates= 64000
1 cycle 3 time= 5.215406e-08 dt= 7.450580e-09 min/max active bin=49/49 rt_updates= 64000
[0000] [00001.0] engine_print_stats: Saving statistics at t=2.980232e-08
[0000] [00001.0] engine_dump_snapshot: Dumping snapshot at t=2.980232e-08
1 2.980232e-08 1.0000000 0.0000000 2.980232e-08 51 51 64000 72000 8000 0 0 414.871 25 33.973
2 cycle 0 (during regular tasks) dt= 7.450580e-09 min/max active bin=49/52 rt_updates= 64000
2 cycle 1 time= 6.705522e-08 dt= 7.450580e-09 min/max active bin=49/49 rt_updates= 64000
2 cycle 2 time= 7.450580e-08 dt= 7.450580e-09 min/max active bin=49/50 rt_updates= 64000
2 cycle 3 time= 8.195638e-08 dt= 7.450580e-09 min/max active bin=49/49 rt_updates= 64000
[0000] [00001.4] engine_print_stats: Saving statistics at t=5.960464e-08
[0000] [00001.4] engine_dump_snapshot: Dumping snapshot at t=5.960464e-08
2 5.960464e-08 1.0000000 0.0000000 2.980232e-08 51 52 64000 72000 8000 0 0 384.540 24 31.060
[0000] [00001.4] engine_repartition: repartitioning space
[0000] [00001.4] check_weights: checking vertex weight consistency
[0000] [00001.4] check_weights: checking edge weight consistency
[0000] [00001.4] check_weights: partition weights checked successfully
[0000] [00001.4] space_rebuild: (re)building space
3 cycle 0 (during regular tasks) dt= 7.450580e-09 min/max active bin=49/51 rt_updates= 64000
[0001] [00011.7] scheduler_abort_deadlock: Detected what looks like a deadlock after 10000 ms of no new task being fetched from queues. Dumping queues.
[0001] [00011.7] scheduler_abort_deadlock: Detected what looks like a deadlock after 10000 ms of no new task being fetched from queues. Dumping queues.
[0001] [00011.7] scheduler.c:scheduler_abort_deadlock():2901: Aborting now.
[0001] [00011.7] scheduler.c:scheduler_abort_deadlock():2901: Aborting now.
Abort(-1) on node 1 (rank 1 in comm 0): application called MPI_Abort(MPI_COMM_WORLD, -1) - process 1
Abort(-1) on node 1 (rank 1 in comm 0): application called MPI_Abort(MPI_COMM_WORLD, -1) - process 1
[0001] [00011.7] scheduler_abort_deadlock: Detected what looks like a deadlock after 10000.2 ms of no new task being fetched from queues. Dumping queues.
```Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1730Master2023-06-06T19:17:26ZFederico Andrés StasyszynMasterMove to version 1.0Move to version 1.0Federico Andrés StasyszynFederico Andrés Stasyszynhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1729ICs close to complete and reasonable running parameters for MHD cloud...2023-06-06T19:14:47ZFederico Andrés StasyszynICs close to complete and reasonable running parameters for MHD cloud...ICs close to complete and reasonable running parameters for MHD cloud collapse, does not seem to crashICs close to complete and reasonable running parameters for MHD cloud collapse, does not seem to crashFederico Andrés StasyszynFederico Andrés Stasyszynhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1728Remove unused code in src/feedback/EAGLE-kinetic2023-05-22T21:19:16ZEvgenii ChaikinRemove unused code in src/feedback/EAGLE-kineticThis PR removes the ability to choose a neighbor selection scheme in `EAGLE_kinetic` because the feedback in `EAGLE_kinetic` is always isotropic and the variable `feedback_model` defined in the file with feedback properties is never used...This PR removes the ability to choose a neighbor selection scheme in `EAGLE_kinetic` because the feedback in `EAGLE_kinetic` is always isotropic and the variable `feedback_model` defined in the file with feedback properties is never used.
I am not sure whether the appearance of `feedback_model` in `EAGLE_kinetic` was a mistake during copy-paste of the code from `EAGLE_thermal`. Or whether the intention was to make the interface in `EAGLE_thermal` and `EAGLE_kinetic` as close as possible, and since `feedback_model` is used `EAGLE_thermal`, it was copied over to `EAGLE_kinetic` on purpose.
In any case, I think it is confusing to keep something that is never used, hence I suggest removing `feedback_model` from `EAGLE_kinetic`.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1727Update spin/jet AGN model2023-09-25T07:31:22ZFilip HuskoUpdate spin/jet AGN modelThis implements lots of small updates that have accumulated in the last few months. I will try to group these into separate categories for clarity. I know that this is quite a lot, sorry. I probably shouldn't have waited this long with a...This implements lots of small updates that have accumulated in the last few months. I will try to group these into separate categories for clarity. I know that this is quite a lot, sorry. I probably shouldn't have waited this long with all the changes, I didn't quite realize how many there were until I started making this MR.
**Improvements to kicking scheme for jets:**
1. There was an unnecessary loop for sorting particles in `black_holes_iact.h` (old lines 370-376). This loop effectively meant that the same particle occupied the entire ray. The end result was that whenever the code wanted to kick more than one pair of particles in a time-step, it would just give the extra energy to the same two particles. The loop has been removed.
2. One-sided kicks (where only one kick happens, from one side of the BH) are now prevented from happening. This is accomplished by lines 1224-1249 in the new version of `black_holes.h`, where the condition `(bp->rays_jet[i].id_min_length != -1) && (bp->rays_jet_pos[i].id_min_length != -1)` makes sure that both hemispheres have to have two particles to kick.
3. Kicks are now perfectly antisymmetrical, as suggested by Joop recently. They are still random, but whenever a pair is kicked, their added momentum is along a line separated by 180 degrees. This, however, requires that the direction for the kicks is randomly drawn at some point earlier than in `runner_iact_nonsym_bh_gas_feedback()` within `black_holes_iact.h`. The way I implemented it now is that I define a new variable (`bp->jet_direction[3]`) for each BH that is randomly drawn every time the code decides a BH will hand out some number of kicks (this is done in `black_holes.h`, new lines 1269-1274). Since this is now done at the level of the BH and not individual particles, I had to also change the function `random_direction_in_cone()` to accept one fewer argument, since the particle ID (of the gas particle) is now unnecessary. This is why `random.h` and `tests/testRandomCone.c` change.
4. The `ray_jet_correction` term, which is added to quantities when sorting particles in the jet rays to make sure that already kicked particles end up at the latter end of the rays, are now included less liberally. Previously, this term was added (it was non-zero) when the relative velocity between the BH and the gas particle was a relatively small fraction of the target jet velocity, i.e. we used it only if `relative_velocity > 0.3 * bi->v_jet`. However, in realistic cases, `bi->v_jet` can be only a few thousand km/s, meaning that particles with velocities of order 500 km/s relative to the BH were being excluded from being considered for kicks. Now we use `relative_velocity > 0.8 * bi->v_jet`, which should still make sure that already kicked particles don't get kicked again, but it should not be as restrictive.
**Changes to the super-Eddington regime:**
1. We now assume that the slim disc modelling of super-Eddington accretion is active once the disc becomes super-Eddington, but using a spin-dependant Eddington ratio, rather than the usual one (that uses 0.1 as the radiative efficiency in the definition of the Eddington accretion rate): new lines 515-520 in `black_holes_spin.h`. Previously this was being done in a more complicated way by using the parameter `TD_SD_eps_r_threshold`, which is now removed.
2. The aspect ratio H/R of the slim disc is now assumed to be the same as that of the thick disc at very low accretion rates (new lines 561-563 in `black_holes_spin.h`). This means that jet efficiencies and spindown are the same in the two regimes. Previously there was a small difference in the two aspect ratios, but for simplicity and because of theoretical uncertainties they are now the same.
3. The slim disc modelling is now on by default in the model; `include_slim_disk` is set to 1 in the example parameter file and example parameter block in `doc`.
**Changes to the variable velocity schemes:**
1. A maximum jet velocity has been added for the variable velocity cases. This is currently set to 1e5 km/s, i.e. c/3. This has been added to the parameter example file and code block in `doc`.
2. The 'Local' and 'SoundSpeed' velocity schemes have been merged into a single one, called just 'Local'. This implements both of the conditions that we discussed when I was in Leiden: the one based on the replenishment time-scale of the BH kernel, and the one based on the crossing time-scale. Both are implemented as minima to the jet velocity. The replenishment part of this scheme uses a hot gas sound speed, which is calculated using only particles in the BH kernel whose temperature is above some minimum `temperature_hot_gas_min_K` (1e4 at the moment, matching the ISM) that is set in the parameter files. The velocity dispersion of gas is also included in this calculation of the replenishment time-scale.
3. Added the last jet velocity `last_jet_kick_velocity` as one of the tracer variables, for these variable schemes.
**Additional changes to parameters:**
1. The `mdot_crit_ADAF` parameter, which is the critical Eddington ratio at which modelling of the accretion disc changes from thick to thin (and feedback from jets to thermal) has now been correctly set in the parameter files (it was missing previously, but it was in the code).
2. The fiducial velocity scheme is now 'Constant', and the value is now set to 3160 km/s (the fiducial value for low-res COLIBRE at the moment).
3. The subgrid accretion disc viscosity parameter `alpha_acc` has been set to 0.2 instead of 0.1.
With regards to tracers, it seems that the jet-related ones weren't properly included at all for the EAGLE scheme, so I have now done that. This is why the tracer files change (other than the new tracer `last_jet_kick_velocity`).
Finally, I added an accretion efficiency parameter. This is applied only if the BH is in the thick disc regime, i.e. if the Eddington ratio is below the already-mentioned `mdot_crit_ADAF`. I have removed all the 'ADIOS'-related parameters; these were just a more complicated (and realistic, but we don't need it) way of accounting for an accretion efficiency.
Edit: I added **some further changes**:
1. In the previous version, before spin was updated in `black_holes.h`, some other quantities (like the jet/radiative efficiencies and the increase to the jet/thermal reservoirs) were already computed before it was updated. The code now updates spin first, and only later computes the other quantities.
2. I added another updating of the jet and radiative efficiencies right after the line `decide_mode(bp, props)` near the end of `black_holes_prepare_feedback` in `black_holes.h`. This is a very important change. The `decide_mode(bp, props)` function can make the BH change feedback modes, but that is not reflected properly until the jet and radiative efficiency fields of the BHs are updated, so it must be done straight after `decide_mode(bp, props)`. If this is not done (like in the previous version), the BH may have, for example, a high jet efficiency and a zero radiative efficiency, when it should have a zero jet efficiency and a non-zero radiative one. The effect is that the BH timesteps were being computed incorrectly, since `black_holes_compute_timestep` uses the fields `bp->jet_efficiency` and `bp->radiative_efficiency` to estimate when the next feedback event will be.
3. I have added an additional spin-related time-step. This one makes sure that the spin vector (and therefore jet direction) don't flip by a huge amount over a single time step.
I have tested the above three changes and things seem stable. It is possible that the last one slows the simulations down somewhat (it doesn't seem to at low-res and in small boxes, but that may change).
Edit 2: I have modified the calculation of the orbital angular momentum of two merging black holes. Previously this was being calculated in the frame of one of the BHs, but I think that's incorrect. Now, the centre of mass is calculated and the coordinates and velocities of each BHs calculated to be in the centre-of-mass frame. The total orbital angular momentum is then simply the sum of each of those for the 2 BHs.
Also, when calculating the total angular momentum (including the orbital contribution and the two spins), I wasn't correctly calculating the angular momentum of each BH (the one coming from spin), which has now been corrected.
Edit 3: The parameter `fix_jet_efficiency` used to not only do what it says, but it also fixed the jet directions to be along the z-axis. This has now been changed, a new parameter, `fix_jet_direction`, is used to do that.
Edit 4: I updated the idealized cluster example to do the following: 1) Have the necessary parameters in the parameter files to also run with jets, 2) modified the README to let the user know how to configure the subgrid model, either EAGLE or SPIN_JET_EAGLE.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1726fix NaNs in plot_task_dependencies.py2023-05-16T11:40:30ZMladen Ivkovicfix NaNs in plot_task_dependencies.pyApparently newer versions of pandas read in empty csv fields as "NaN"s, not as "None"s any longer. So I manually replace them so the script works again.Apparently newer versions of pandas read in empty csv fields as "NaN"s, not as "None"s any longer. So I manually replace them so the script works again.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1725ignore oneAPI optreport outputs2023-05-16T11:40:09ZMladen Ivkovicignore oneAPI optreport outputsMatthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1724Rt gear unphysical check fix2023-05-16T11:27:55ZMladen IvkovicRt gear unphysical check fixFor now, catch either mass or rho being zero. At the moment, they are not
necessarily both zero. For example, an unphysical check may zero out both
mass and density when it becomes negative in a hydro step. Once that
happens, particles m...For now, catch either mass or rho being zero. At the moment, they are not
necessarily both zero. For example, an unphysical check may zero out both
mass and density when it becomes negative in a hydro step. Once that
happens, particles may gain mass through flux exchanges with other active
particles, while they themselves remain inactive. The density of such
inactive particles however remains zero until the particle is active
again. See issue #833.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1723Use correct fluid velocity in time extrapolation (Gizmo).2023-05-31T15:41:42ZYolan UyttenhoveUse correct fluid velocity in time extrapolation (Gizmo).(cherry picked from commit 3970804bf91240c5e102db63ab5014f9444ea845)
I think there is a mistake in the time extrapolation of the primitive quantities in the Gimzo and moving mesh hydro schemes.
Just using the fluid velocity to calculate...(cherry picked from commit 3970804bf91240c5e102db63ab5014f9444ea845)
I think there is a mistake in the time extrapolation of the primitive quantities in the Gimzo and moving mesh hydro schemes.
Just using the fluid velocity to calculate the time extrapolations from the spatial gradient violates Galilean invariance (pretty dramatically for very high bulk velocities).
I think we should use the fluid velocity in the rest frame of the particle instead.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1722Merge branch 'MHD_FS' into 'MHD_canvas'2023-05-05T13:06:49ZFederico Andrés StasyszynMerge branch 'MHD_FS' into 'MHD_canvas'update to understad Monopole behavior
See merge request swift/swiftsim!1721update to understad Monopole behavior
See merge request swift/swiftsim!1721Federico Andrés StasyszynFederico Andrés Stasyszynhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1721update to understad Monopole behavior2023-05-05T13:06:30ZFederico Andrés Stasyszynupdate to understad Monopole behaviorFederico Andrés StasyszynFederico Andrés Stasyszynhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1720Add cmdline option to dump restart files after step 1 (and continue with the ...2023-05-22T18:24:16ZMladen IvkovicAdd cmdline option to dump restart files after step 1 (and continue with the run)This oughta be helpful for debugging and profiling.This oughta be helpful for debugging and profiling.Peter W. DraperPeter W. Draperhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1719Fixed issues with missing unlocks/dependencies/debug checks for top level cel...2023-09-04T20:10:45ZMladen IvkovicFixed issues with missing unlocks/dependencies/debug checks for top level cells which aren't also super cellsNot sure how we didn't notice this one before. Should've been an issue earlier. Especially since I now detected it on EAGLE_12, whereas I've been running EAGLE_12 examples for RT checks for ages.
Anyway, this should do the trick.
This ...Not sure how we didn't notice this one before. Should've been an issue earlier. Especially since I now detected it on EAGLE_12, whereas I've been running EAGLE_12 examples for RT checks for ages.
Anyway, this should do the trick.
This fixed a handful of related bugs.Matthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1718Fix partition costs and checks2023-04-27T19:04:34ZMladen IvkovicFix partition costs and checks- make the list of task types in `partition_gather_weights` and `check_weights` in `partition.c` identical
- clean up these lists. Remove duplicates like `kick1` and `timestep`, add all tasks I found in the `enum task_type` that should h...- make the list of task types in `partition_gather_weights` and `check_weights` in `partition.c` identical
- clean up these lists. Remove duplicates like `kick1` and `timestep`, add all tasks I found in the `enum task_type` that should have some work.
- re-order the lists of task types to have the same order as the type definitions in the enum
- document that new tasks should also be added to these listsMatthieu SchallerMatthieu Schallerhttps://gitlab.cosma.dur.ac.uk/swift/swiftsim/-/merge_requests/1717Add a new potential MWPotential20142023-09-27T10:14:08ZDarwinAdd a new potential MWPotential2014This branch implements MWPotential2014, a reference Milky-Way potential from Bovy's paper,
galpy: A Python Library for Galactic Dynamics, Jo Bovy (2015), Astrophys. J. Supp., 216, 29 (<arXiv/1412.3451>).This branch implements MWPotential2014, a reference Milky-Way potential from Bovy's paper,
galpy: A Python Library for Galactic Dynamics, Jo Bovy (2015), Astrophys. J. Supp., 216, 29 (<arXiv/1412.3451>).Matthieu SchallerMatthieu Schaller