-
Notifications
You must be signed in to change notification settings - Fork 17
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
Kharma Next #33
Merged
Merged
Kharma Next #33
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
Same as kharmaim-stable but this segfaults. Committing to share code.
(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?)
…ture/multizone merge with the latest dev version
…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`.
… in emhd test problem par files
…ssue to EMHD specifically if it still exists
…essure anisotropy. Fixes: Done away with mesh decomposition for viscous Bondi along X1, since it messes with the evolution of dP (presumably by modifying B1 in some insidious manner).
…nzero flux across X1 bdry by serial operation. However it's likely we're not going to use these features
All MHD variables in bondi_viscous now converge as expected, and boundaries are applied to dP as expected. Source term seems to be much, much too large for some reason. Also Vbump Kokkos to fix a CUDA segfault (again?)
Supporting exchanging primitive vars only for ImEx driver in non-AMR had become a source of bugs, incl. last commit. Fix by simplifying. ImEx driver needs to be able to sync conserved variables anyway for AMR, so better to keep the same codepath even at the cost of the occasional UtoP/PtoU call.
These bugs would have appeared more inscrutably when we ran w/AMR anyway
This reverts commit d7d61e7.
A few commits back I removed sync_prims, reasoning conserved variables should be prolongated/restricted, and could always be recovered from each other. Both points were wrong: primitive vars can be prolongated and restricted on boundaries fine, though the latter is not ideal. And, in EMHD, it is not straightforward to recover P from U, as this is normally done inline with the computation of the next step's state. This commit switches to syncing primitive variables (sync_prims) anytime they're fundamental (ImEx and simple drivers, "prims_are_fundamental") Note the "conserved" B field is *always* what is sync'd, regardless of the other primitive or conserved variables. It also avoids loading the inverter package if GRMHD is implicitly evolved, and expands some computed domains related to B to work at the last prolongation operator bug before AMR.
Viscous Bondi: use outflow conditions, solve ODE from outer edge value (converges to fixed condition, but not at 2o, probably due to limited runtime) Conducting atmo: actually return check result, converge by fixing detection of higher-order terms.
WIP: Pass tests
…ow* rather than inflow.
Now that this doesn't have outstanding physics issues with core iharm3d functionality, I'm going to pull it to dev. Remaining issues will be opened on Github so we can track what's still to be fixed to make this run everywhere. |
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 branch pulls together:
kharma-next
: dependency updates/simplifications, better B field management, better boundary management, more modular with callbacks added based on Parthenon PEP1.multizone-dev
: forward-ports of most everything needed to run multi-zone chained simulations. Still buggy though.kharmaim-pivot
: implicit/extended MHD built on pivoted QR, with newer floors etc.Features
StateDescriptor
to provide extra callbacks, making it easier to write a KHARMA-specific package, and making the idea of a KHARMA package a bit more standard. Packages are loaded according to their dependencies in kharma.cpp.Flag()
usage is standardized a bit better andFlag
s now correspond to Kokkos profiling regions, meaning we can trace execution at runtime with the kokkos-tools kernel printerdevelop
branch. Thekokkos-kernels
submodule is replaced with a forked version, containing just those files necessary for a modified, pivoted form of the serial QR decomposition.Obviously there's also tremendous new functionality from the Multizone and kharmaim branches, covering new modes of running KHARMA. Details on those additions will come later as they start to be used.
Known Issues (rough order of importance):
Other TODOs before merge