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

Release KHARMA 2023.12 #42

Merged
merged 251 commits into from
Dec 8, 2023
Merged

Release KHARMA 2023.12 #42

merged 251 commits into from
Dec 8, 2023

Conversation

bprather
Copy link
Contributor

@bprather bprather commented Nov 28, 2023

This PR will track things as we prep "KHARMA Next" to just be "KHARMA."

Issues with this version should be opened as Github issues, and we'll track the blockers here.

Depends on (not in importance order):

  1. Land Bump Parthenon & Fix SMR #39 with SMR fixes

  2. MPI Hang at first file output under SMR #43

  3. NCSA Delta: Running on multiple GPUs on the 4xA100 nodes #44

  4. NCSA Delta: CPU build #45

  5. NCSA Delta: trace build #47

Vedant Dhruv and others added 30 commits November 30, 2022 17:17
(i) Wasn't allocating the right amount of scratch memory for the imex solve.
(ii) Viscous bondi needs 5 zones in the horizon. Was my bane for most of the day.
… sector `<emhd/feedback>`.

This allows integrating viscous Bondi problem into this branch.
…inesearch failed, ie. when even manual backtracking was not sufficient as well.

TODO:
1. Provide the option to average over neighboring good zones when a particular zone fails to converge to physically meaningful primitives.
2. Provide a runtime option to save the components of the residual. Useful for debugging.

I've made some edits towards initializing an EMHD torus but it doesn't work (ctop is NaN within a few timesteps).
…tart_kharma problem, and some more updates
1. Non-ideal contributions to the stress-energy tensor weren't being considered when computing source terms for u, uvec. We now have a `Flux::AddSource` that computes the said source terms, and is called in the ImEx driver, replacing `GRMHD::AddSource`. This also meant I had to define a global `Flux::calc_tensor` that can accept data over MeshblockPacks.
2. Boundary sync at end of sub-step in the ImEx driver is now carried out over the entire domain and is done after fixing UtoP failures (similar to the `dev` commit).
3. No clue as to how the conducting atmosphere problem worked previosuly without the feedback from the non-ideal sector onto the source terms, but that is fixed now. Have included a conditional statement in `EMHD::convert_prims_to_q_dP` to deal with `kappa_eta` closure type. This is necessary because `conduction_alpha` and `viscosity_alpha` are no longer constant but rather depend on `kappa` and `eta` respectively, and rho as well. The problem now converges at the expected order.
4. Included a PtoU call at the end of drift frame floors. Also, floored bsq calculation to SMALL. This is necessary because `ApplyFloors` is called prior to B-field initialization during problem initialization.
5. Removed the option to print residual during the implicit solve since it clearly wouldn't work on GPUs.
6. Included problem init and .par files for ImEx and EMHD torus. This is not to say they run.
7. Have included a comment in problem.cpp to let users know that during problem initialization, fluid frame floors are used since the fluid conserved vars outside the torus are NaNs. However, this is done in a rather underhanded manner and should be more explicit.
8. Edits made to conducting atmosphere initialization and .par file so it is up-to-date with the latest version of `kharmaim`.
9. Updated `run.sh` and `check.py` scripts for conducting atmosphere so that it is similar to the rest of the test problems. This is not to say it will pass CI. As it stands, the problem can be run ONLY ON CPUs.
…vant scripts. For some reason though it seems like B fields are not copied over correctly, or being overriden by some other functions (maybe SyncAllBounds?)
…t was due to ReflectX2). The only issue left is big DivB at the patching boundary. This is confusing because all the primitive and conservative values are all copied over properly
1. Capability to average over bad/unconverged zones in implicit solver (similar to fixup for UtoP)
2. Some updates from #02cf70b8509d77fad7d5a2d386f9dcf2cf711779 and #9ffbfe1493ddbd38cbd5464cd83a8d856a469540
3. PRINTTILE functionality in types.hpp for debugging purposes. Allows printing a 6x6 tile about selected zone
4. Reduced default uflr_geom
5. Modified default floors in sane_emhd.par

NOTE: The code compiles. The avergaing functionality is being tested.
… input parameters to '\Params' group, and some TRACE-enabled output in `apply_instability_limits`.
Ben Prather and others added 22 commits November 15, 2023 15:19
1. Zero EMFs on boundaries in boundaries.cpp.
   Supercedes B and EMF fixes in b_ct.cpp
2. Remove check_inflow_flux_X because they were confusing,
   and should always match check_inflow_X
@bprather
Copy link
Contributor Author

bprather commented Dec 4, 2023

I've removed nvhpc build/run issue as a blocker since GCC is preferred already on Delta. It seems like it could be related to not being able to use device-side MPI buffers on some Nvidia machines, so it's an issue to fix in Parthenon as we can figure it out.

Since the Parthenon-bump branch now fixes outstanding issues with this branch for at least @vedantdhruv96 and my production contexts, once it passes regression tests I'll be merging that here. So, any other users of mainline KHARMA should pull this dev branch in a couple of hours, run git submodule update --recursive to pull the new Parthenon & Kokkos versions, and ensure it works for their case!

Then, pending sign-offs about this code working for folks, I then plan to release it, and never ever write a PR >5k lines again.

Copy link
Contributor

@vedantdhruv96 vedantdhruv96 left a comment

Choose a reason for hiding this comment

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

Looks good! I'm able to run this version of the code for various torii problems- ideal, extended, electrons. On both CPUs and GPUs (nvidia nodes on Delta) with multiple ranks. It also restarts fine.

@bprather bprather merged commit 4936147 into stable Dec 8, 2023
2 checks passed
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.

2 participants