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

AGN Triggering #2

Merged
merged 108 commits into from
May 18, 2023
Merged

AGN Triggering #2

merged 108 commits into from
May 18, 2023

Conversation

forrestglines
Copy link
Contributor

Adds multiphysics modules necessary for evolving galaxy clusters with triggered AGN feedback. It implements the three triggering mechanisms from Meece 2017, cold gas triggering, boosted Bondi accretion, and Booth Schaye accretion. It both calculates the AGN feedback to inject from these mechanisms and removes mass from the simulation at a fixed velocity and temperature (so that momentum and specific gas energy is changed). I've also modified the thermal AGN feedback to inject mass at a fixed velocity (so that momentum is changed) and magnetic AGN feedback mass injection at a fixed velocity and temperature (so that momentum and specific gas energy is changed). These implementation details/choices are intended to follow Meece 2017 as closely as possible, although they could be revisited.

The separate components added are:

  • Calculation of AGN triggering from Cold Gas
  • Calculation of AGN triggering from Boosted Bondi
  • Calculation of AGN triggering from Booth-Schaye
  • Gas removal (ie. accretion) of triggering gas at fixed velocity and temperature
  • Mass injection at fixed velocity with Thermal AGN feedback
  • Mass injection at fixed velocity, fixed temperature mass injection with Magnetic AGN feedback

Each of these separate components have been tested individually in regression tests to function as expected (not to say they could still have bugs). I've also ran a larger test with cold gas accretion and all three kinds of AGN feedback (thermal, kinetic jet, and magnetic feedback) working at once across 4 gpus on a node with multiple restarts -- everything seems to work as of commit 8b78940. This test is probably sufficient for preliminary science runs at larger scales, that would be my next step if I had the time.

I think this MR is ready for review and to be merged. There are a few outstanding questions about implementation choices for either reviewers or future merge requests. However, since the changes in this MR are exclusive to running galaxy clusters, I'd argue for merging these changes in after review.

My outstanding questions and things for reviewers to double check are:

  • Calculation of AGN triggering is implementing via hacking the Parthenon history dump feature, which allows doing reductions across the simulation. These happen at the end of an evolution step whereas it might be more correct to calculate triggering before the step. Hooks for such reductions might have already been added to Parthenon.
  • Are the primitive variables used for calculating AGN triggering at the correct time stamp or are they from the previous timestep?
  • Double check that the AGN triggering calculated/used after a restart is the same if the simulation had continued through that step without stopping and restarting.
  • Is the implementation of boosted Bondi/Booth-Schaye correct as to how density, velocity, and sound-speed averaged over the accretion volume?
  • Is the mass injection at fixed velocity with thermal feedback and at fixed velocity and temperature with magnetic feedback both implemented correctly and appropriate for the AGN-physics model we want?

On a related note for reviewers, if anything does seem broken/not working, I'd suggest checking whether the input file is correct -- that things are named correctly, that they match the inputs read by the code, and that the inputs are saved correctly in the code.

There are a three things left to implement for galaxy cluster simulations but we could move forward with preliminary simulations. These features are:

  • Initial sinusoidal perturbations to density/velocity/magnetic fields
  • Supernova feedback following Prasad 2022 / Voit 2015
  • AMR (adaptive, not just static) on density or cooling rate

I have a repo on gitlab for creating initial conditions for galaxy clusters, making movies, and plotting statistics. magnetized-clusters. You need Python 3 and Jupyter notebook with both unyt and the Parthenon-enabled yt from my parthenon-frontend branch on my github repo installed.

A few useful files and notebooks from my magnetized-clusters repo are:

Generate AthenaPK Input.ipynb: Generate input files for galaxy clusters. Since I've designed the input files to use purely code units (except to define code units), it's helpful to use unyt to manage the code units that go into the input file.

Slice Movies.ipynb: Make movies from galaxy cluster simulations. Does the heavy lifting for automatically generating boundaries for the color/field axes on plots so that the axes aren't constantly changing throughout the movie but the axis bounds also don't have to be explicitly set.

Statistics.ipynb: WIP notebook to read statistics over time from a simulation. E.g. total mass/cooling over time, mins and maxes, cold gas mass, etc. Data reading is fleshed out, plots/figures are WIP

medium.precessing_all_mech_jet.input: A working, with restarts, half-handcrafted, half-generated input file including a cold-gas triggered precessing AGN jet with thermal, kinetic, and magnetic AGN feedback. Would be a good starting point for a full simulation.

@pgrete pgrete self-requested a review October 19, 2022 17:05
@forrestglines
Copy link
Contributor Author

@pgrete This PR was ready for review but after merging with main many of the cluster tests are failing due to negative densities in tests that should be running with a static hydro, source terms only. It seems that riemann = none is no longer skipping the hydro step like it used to. Can you take a look at it?

Also, I don't seem to have any write access to this repo, I can't push anything up

@pgrete
Copy link
Contributor

pgrete commented Oct 20, 2022

@pgrete This PR was ready for review but after merging with main many of the cluster tests are failing due to negative densities in tests that should be running with a static hydro, source terms only. It seems that riemann = none is no longer skipping the hydro step like it used to. Can you take a look at it?

Also, I don't seem to have any write access to this repo, I can't push anything up

I'll have a look.

@forrestglines
Copy link
Contributor Author

@par-hermes format

@pgrete pgrete force-pushed the forrestglines/cluster_agn_triggering branch from b38c4a6 to 15cc60d Compare May 5, 2023 12:03
@forrestglines
Copy link
Contributor Author

@par-hermes format

@forrestglines
Copy link
Contributor Author

In the current state of the revisions, we have one failing test: cluster_hydro_agn_feedback.

The test explores five test cases of thermal only, kinetic only, and combined thermal_kinetic feedback with a tilted and non-tilted kinetic jet. (Only one test for thermal only since the jet tilt is irrelevant). The tests with kinetic feedback are failing in various ways.

1. The change in energy density for all tests with kinetic feedback is off by ~0.15%. This could be related to the question above about injected momentum and change in kinetic energy density.

2. The change in momentum density for the tests with tilted kinetic jets is off by >100%. I suspect there's an issue in the coordinates of precessed jets in either the test or how they are invoked in kinetic jet feedback. The tilted precessed jet coordinates seemed to work correctly in the magnetic tower regression test.

I'd since fixed the precessing jet. It was due to incorrect application of rotations from a small detail in the Sympy functions I use to figure out the coordinate rotation and transformation math. It's working now.

@forrestglines
Copy link
Contributor Author

@par-hermes format

@forrestglines
Copy link
Contributor Author

@pgrete I've addressed all the comments I can. I've also made issues #46, #61, and #62 to address further changes.

Please take a look at the changes again. Merge if you approve and the CI is passing.

Copy link
Contributor

@pgrete pgrete left a comment

Choose a reason for hiding this comment

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

Final changes look good! Great job, @forrestglines !
Test pass (though I'm not sure why the results are not reported back).
Pulling the trigger.

@BenWibking
Copy link
Contributor

Did the CI machine die? I don't think it usually takes this long to run.

@pgrete pgrete merged commit 2b7c87e into main May 18, 2023
@pgrete pgrete deleted the forrestglines/cluster_agn_triggering branch March 18, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants