1. The Enzo Project
  2. Untitled project
  3. enzo-dev
  4. Pull requests

Pull requests

#119 Merged
Repository
cluster-dev
Branch
week-of-code
Repository
enzo-dev
Branch
week-of-code

Add ClusterSMBHFeedback etc

Author
  1. yl2501
Reviewers
Description

We add Super Massive Black Hole feedback in the center of a cooling flow galaxy cluster (=PointSourceGravityPosition in our simulations). We add mass, momentum and energy to the Jet Cells to simulate mechanical and thermal feedback. The cold gas from around the Black Hole is removed at the same rate as the outflow rate. The changes do not seem to affect other parts of enzo.

Comments (6)

  1. Greg Bryan

    Yuan -- I think this looks very good, thanks! A few minor things and then it should be ready to go!

    1. Can you remove the changes to DetermineSubgridSizeExtrema.C from this pull request and put them in another pull-request? I think those changes deserve their own pull-request (as they really have little to do directly with the SMBH stuff).

    2. The same thing goes for the changes in Grid_SetFlaggingFieldStaticRegions.C - can you move those changes to their own pull request? I know this change is required for this problem, but I think those changes are more wide ranging and deserve a fuller discussion.

    3. There are some changes to Make.mach.trestles which don't really belong in this pull request. Remember you can always put a local enzo makefile in ~/.enzo.

    4. Can you also remove the changes to OutputFromEvolveLevel.C? That could cause a problem if somebody really wanted to use a very small timestep.

    5. Finally, can you put the bug fix in solve_cool.F in it's own pull request, just so people can see this, and it's not buried in this pull request? If you want, I can do it.

    Thanks, Greg

  2. Greg Bryan

    Thanks Yuan! The update looks great. The only problem I see in the changes is that DetermineSubgridSizeExtrema.C is still showing changes from the current tip (see the diffs).

    The other thing that I realized I missed is documentation. Can you add a brief description of the new problem and it's associated parameters in this file: doc/manual/source/parameters/problemtypes.rst (following the similar format as with the others)?

    Thanks, Greg

  3. Greg Bryan

    OK, almost there I think! The code looks good to me, so I ran the test-suite and found a problem, which is that EvolveLevel calls ClusterSMBHSumGasMass even if the ClusterSMBH test problem is not being used. I think you want to add a line in ClusterSMBHSumGasMass which immediately returns if ProblemType != 108.

  4. Matt Turk

    Hi Yuan, I have a comment which doesn't need to hold up accepting this PR. And that's that in the SMBH feedback routine, there are a lot of combinations of the macro GRIDINDEX_NOGHOST with the same input parameters. My suggestion would be that since you're using it many, many times in the routine, to assign the value once to a temporary variable (say, gridind_ijk) and use that in its place. But again, I don't think this needs to be a blocker for acceptance, just something to consider the next time you update/modify the routine.

  5. yl2501 author

    Thank you, Matt! That's a good suggestion. I will use that when I clean up the code and issue another pull-request in the future.