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

add tracer particles #490

Merged
merged 48 commits into from
Jan 18, 2024
Merged

add tracer particles #490

merged 48 commits into from
Jan 18, 2024

Conversation

BenWibking
Copy link
Collaborator

@BenWibking BenWibking commented Jan 9, 2024

Description

This adds tracer particles. Particles are created one-per-cell in the initial conditions, then they advect with the fluid using the face-centered velocities returned from the Riemann solver. Particles are included in plotfiles and checkpoints. (Particles will be added to openPMD outputs in a subsequent PR.)

The method adopted here for particle advection with face-centered velocities is the "marker-and-cell method" (Harlow & Welch 1965).

Tracer particles are turned on for the HydroBlast3D and SphericalCollapse problems, so tracer particle functionality is included in the test suite.

Significant code infrastructure changes:

  • This removes the old AMR level subcycling code taken from Chombo and replaces it with the cleaner version from the AMReX code examples. However, it should behave identically as before. This is necessary for particle redistribution to work with subcycling.
  • This adds the face-centered velocities to state_new_fc_ and state_old_fc_. Some changes to the machinery for face-centered variables was necessary to get this to work, such as implementing AMR interpolation for face-centered variables using the amrex::FaceLinear interpolator (caution: this interpolator does NOT preserve the divergence of the field!). This is necessary because ghost faces are needed in order to handle particles that cross coarse-fine boundaries during a timestep. (In the future, this machinery can be modified so that it creates alias MultiFabs for separate face-centered components and then uses a different AMR interpolator for each component.)
  • In order to avoid modifying the initial conditions for every problem when tracer particles are enabled, this separates the initial conditions set-up function into setInitialConditionsOnGrid (cell-centered variables) and setInitialConditionsOnGridFaceVars (face-centered variables). This only affects the FCQuantities test problem.

Related issues

Closes #489 and #498.

Checklist

Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an x inside the square brackets [ ] in the Markdown source below:

  • I have added a description (see above).
  • I have added a link to any related issues see (see above).
  • I have read the Contributing Guide.
  • I have added tests for any new physics that this PR adds to the code.
  • I have tested this PR on my local computer and all tests pass.
  • I have manually triggered the GPU tests with the magic comment /azp run.
  • I have requested a reviewer for this PR.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 13. Check the log or trigger a new build to see more.

src/simulation.hpp Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Show resolved Hide resolved
src/simulation.hpp Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
src/simulation.hpp Outdated Show resolved Hide resolved
@BenWibking
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@BenWibking
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

src/RadhydroSimulation.hpp Outdated Show resolved Hide resolved
@BenWibking
Copy link
Collaborator Author

BenWibking commented Jan 10, 2024

There is a weird ROCm build issue:

In file included from /home/runner/work/quokka/quokka/src/RadMatterCouplingRSLA/test_radiation_matter_coupling_rsla.cpp:10:
In file included from /home/runner/work/quokka/quokka/src/RadMatterCouplingRSLA/test_radiation_matter_coupling_rsla.hpp:21:
In file included from /home/runner/work/quokka/quokka/src/radiation_system.hpp:29:
In file included from /home/runner/work/quokka/quokka/src/hyperbolic_system.hpp:35:
In file included from /home/runner/work/quokka/quokka/src/simulation.hpp:76:
In file included from /home/runner/work/quokka/quokka/extern/amrex/Src/AmrCore/AMReX_AmrParticles.H:5:
In file included from /home/runner/work/quokka/quokka/extern/amrex/Src/Particle/AMReX_Particles.H:5:
In file included from /home/runner/work/quokka/quokka/extern/amrex/Src/Particle/AMReX_ParticleContainer.H:1406:
In file included from /home/runner/work/quokka/quokka/extern/amrex/Src/Particle/AMReX_ParticleIO.H:5:
/home/runner/work/quokka/quokka/extern/amrex/Src/Particle/AMReX_WriteBinaryParticleData.H:161:14: error: reference to __host__ function 'memcpy' in __host__ __device__ function
        std::memcpy(&yi, &yu, sizeof(yu));
             ^
2 errors generated when compiling for gfx908.

@BenWibking
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@BenWibking
Copy link
Collaborator Author

unrelated macOS CI failure.

@BenWibking
Copy link
Collaborator Author

BenWibking commented Jan 10, 2024

reliably crashes when AMR is enabled. need to debug

Fixed. Needs at least 2 ghost cells for the face velocity.

@BenWibking
Copy link
Collaborator Author

BenWibking commented Jan 10, 2024

On uniform grids, it seems to work fine:

kh_tracer

@BenWibking
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@BenWibking BenWibking marked this pull request as ready for review January 15, 2024 21:18
@BenWibking BenWibking changed the title [WIP] add tracer particles add tracer particles Jan 15, 2024
@BenWibking
Copy link
Collaborator Author

@markkrumholz There are a lot of code infrastructure changes in this PR, unfortunately. Let me know whether I should split this up into multiple PRs, and if so, what splitting would be easiest to review.

Copy link
Collaborator

@markkrumholz markkrumholz left a comment

Choose a reason for hiding this comment

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

I've left a few comments on individual files, but overall this looks fine. Once those comments are addressed I can approve.

@BenWibking
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@BenWibking
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@BenWibking
Copy link
Collaborator Author

@markkrumholz Can you re-review?

Copy link
Collaborator

@markkrumholz markkrumholz left a comment

Choose a reason for hiding this comment

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

OK, looks good now.

@markkrumholz markkrumholz added this pull request to the merge queue Jan 17, 2024
Merged via the queue into development with commit bf37694 Jan 18, 2024
13 checks passed
@BenWibking BenWibking deleted the BenWibking/tracer-particles branch January 18, 2024 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add tracer particles
2 participants