-
Notifications
You must be signed in to change notification settings - Fork 2
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
Maintain sanity (or madness) #165
Conversation
auto &sc = pmb->meshblock_data.Get(stage_name[stage]); | ||
|
||
auto p2c = tl.AddTask(none, fluid::PrimitiveToConserved, sc.get()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new set of regions is good motivation for getting the rest of the hydro to be per-meshdata I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Some comments below. Nothing really blocking, but I think we shouldn't demand that num_partitions=1
across the whole code if we can help it, as that limits what we can do with load balancing (i.e., with monte carlo) and with AMR (when we get there)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, agree with @Yurlungur's comments. It would also be good to start documenting runtime options, but that's for another day..
…n so CI failures are readable
… brryan/maintain_sanity
@@ -32,10 +32,17 @@ Real inline Min(const Real &x) { | |||
return xmin; | |||
} | |||
|
|||
Real inline Sum(const Real &x) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not actually used now but I think we can leave this in in case the need arises in a problem generator file at some point
OK this is ready for re-review @Yurlungur. The function used to paint on B field lines is still unnecessarily re-evaluated every so often but this cost should always be small so I didn't bother to rewrite everything to work with restarts for this pretty specific feature. If you think this is too ugly or problematic let me know and I can revisit. Otherwise this feature seems to do what it's supposed to. |
PR Summary
Long-duration simulations of Fishbone torii tend to accumulate large amounts of magnetic flux on the black hole (MAD state). While this may suggest that MADs are ubiquitous in nature, it would be nice to be able to simulate accretion onto weakly magnetized black holes (SANEs) for long times. This PR adds a runtime option for setting a specific
phi = Phi / sqrt(Mdot)
amd some timescale on which the code should maintain that value. Phi is maintained by adding a (hopefully small) amount of divergence-free nearly vertical field lines to the black hole every timestep.This PR also fixes a bug in the history flux evaluators, where the block index wasn't being passed in to the geometry calls. I think this should have been caught in the geometry infrastructure (it is caught with analytic but not cached geometry) but that is a separate issue (#171). The geometry worked correctly for single-meshblock-per-MPI-rank calculations.
Here is an example 256^2 2D torus run with no sanity maintenance (
torus.hst
) and the sanitized version with a targetphi = 1
(indicated by the dashed line in the phi plot) and a characteristic time for changing the net fieldtau = 500M
:The early hiccup in phi with the sanitized run is probably due to mdot being all over the place there. I also included a timer so that the phi changing code can be disabled for some initial coordinate time.
Restarts also look fine (here, a bit after 500M):
PR Checklist
scripts/bash/format.sh
.