Black Holes: Port puncture tracking and enable tagging around the punctures #89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also move initialization of particles to set_initial_punctures.
* pre checkpoint * post checkpoint * post init * post restart Also add some problem specific hooks too.
This example builds fine but there are still problems with some failed assertions in the puncture tracking. [skip ci]
We know we want to switch to tagging cells based on the puncture locations but for now this is making debugging the punctures annoying.
MPI_AllGather doesn't like the send and receive buffers being the same and we need to redistribute particles before writing to a checkpoint as they might have moved to a different process.
This seems to work unlike invoking linear_interpolate_to_particle directly.
CUDA doesn't allow device lambdas in class member functions that are private.
This means we only have to do MPI collectives at the end of execute_tracking rather than in redistribute to figure out which process has the punctures.
This removes the need to convert between a 2-index object and a linear object in the PunctureTracker. Also remove the tracking of which process has each puncture. We don't need this info as we set the puncture coords to 0 by default and then sum reduce over all procs.
This is in an effort to make member functions single-purpose and remove redundant variables.
These changes include * Making PunctureTracker a template class with the template parameter being the number of punctures. This allows us to make m_puncture_coords a fixed size array. * Moving the setting of initial_puncture_coords to start_from_initial_punctures() rather than initialize() as it's irrelevant in the restart case. * Making PunctureTracker obtain the restart_time from it's GRAMR pointer rather than it being needed to be passed to track(). * Moving the updating of m_puncture_coords from the particle locations to a separate function which we now also call after restarting from the checkpoint file. * Other minor changes
Also get the filename and output path from the ParmParse table directly (in a more AMReX-like way) rather than requiring it to be passed in the constructor to PunctureTracker. Move the initialization of PunctureTracker to the BHAMR constructor.
Also make m_puncture_tracker a private member of BHAMR.
Following AMReX-Codes/amrex#4098 we no longer need to make an alias MultiFab and use cic_interpolate.
This requires moving around some of the initialization of the PunctureTracker class. Also, rather than having a combined ChiPunctureExtractionTaggingCriterion class, we switch to using the existing ChiExtractionTaggingCriterion class and then using a separate PunctureTracker class.
|
This PR modifies the following files which are ignored by .lint-ignore: Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted. |
Cpp-Linter Report
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This long-awaited PR ports the puncture tracking capabilities from GRChombo to GRTeclyn. Here, we leverage AMReX's particle capabilities and the particles are moved around the grid as particles. This simplifies working out what box/grid they reside in as we can instead just make AMReX redistribute them.
The main things introduced/changed by this PR are described below
Parameters
The parameters have been renamed to
puncture_tracking.enabled=false(default) ortruepuncture_tracking.level=0(default), ...,max_levelPunctureTrackerclassThis is now derived from
amrex::ParticleContainer<AMREX_SPACEDIM, 0>and is templated overnum_punctures(which will probably be2). The member functions have been renamed from the ones inPunctureTrackingin GRChombo.For the actual tracking, we essentially copy what AMReX's
TracerParticleContainer::AdvectWithUccdoes except we advect in the reverse direction to the shift. This uses the midpoint method with linear interpolation (which means we only need to fill 1 ghost cell).Note that we also store the coordinates in
PunctureTracker::m_puncture_coordsvariable which is set and broadcast to all MPI processes inPunctureTracker::update_puncture_coords.BHAMRchangesThis is now also templated over
num_puncturesso it can store a puncture tracker with the correct number of punctures! Its constructor initialises thePunctureTrackerifpuncture_tracking.enabled == true.New
GRAMRLevelhooksI have added some hooks for examples to do things post init, post restart, pre checkpoint and post checkpoint. I should make the formatting of these hooks more consistent...
PunctureTaggerclassRather than having a combined
ChiPunctureExtractionTaggingCriterionclass as before, we switch to using the existingChiExtractionTaggerclass and then using a separatePunctureTrackerclass which just does the tagging around the punctures.This resolves #15.
I'll open this as a draft for now as I still need to make a few minor changes and fix the linter errors but I think it should be close enough to finish for you to test, @KAClough.