Three component phase separation for ultramassive white dwarfs, part 2 - #863
Three component phase separation for ultramassive white dwarfs, part 2#863evbauer wants to merge 43 commits into
Conversation
Updated code in phase_separation.f90 and a new wd_o_ne_3_phase test suite.
|
@mcastrotapia I've pushed a few "housekeeping" fixes to get the static analysis script to pass, so you might want to merge this branch into your fork to stay up to date. Anyway, this PR will eventually be the one that merges the code into Thanks for submitting this code! |
|
Thanks for the fixes and for reviewing the PR! I will merge the branch into my fork, and I'll be attentive to comments/questions and updates. |
There was a problem hiding this comment.
Note to self for later: update run_star_extras, rn script, etc to make this a fully-fledged test case and integrate it into do1_test_source for the test suite.
|
We'll also need to add some documentation of the new options in controls.defaults. |
|
Converting the plain text |
The rate of phase separation heating is somewhat mesh and timestep dependent. It should get the integrated amount of energy right, but we are not resolving the spatial and time scales over which it occurs. This led to an undesirable behavior at small time steps where crystallizing a zone could cause enough mixing to generate a transient heating spike. When localized near the crystallization front, this could re-melt previously crystallized material in an unphysical way. The smoothing approach here redistributes this energy throughout the inner half of the star, which should prevent re-melting, so we at least get the right amount of energy into the stellar interior, while we might not be precisely locating it in the right place. This changes the default behavior somewhat, so I've included an inlist flag in case somebody wants to turn this off to reproduce previously published behavior.
|
I believe we're finally ready on this one! (assuming the tests pass) Perhaps we should add a changelog entry? |
VincentVanlaer
left a comment
VincentVanlaer
left a comment
There was a problem hiding this comment.
Left some small comments, which can be resolved easily or can be postponed. I think you should add a changelog entry, this feature certainly deserves it!
| write(*,*) 'invalid phase_separation_option' | ||
| stop | ||
| end if | ||
|
|
There was a problem hiding this comment.
You will need a check for ierr here, otherwise smooth_eps_phase_sep will hide the error. Since neither functions actually set ierr to anything else than zero, the ierr parameter could also be removed.
| if (components=='CONe') then | ||
| filename = trim(phase_sep_data_dir) // '/CONe_deltaO.h5' | ||
| else if (components=='NeOMg') then | ||
| filename = trim(phase_sep_data_dir) // '/NeOMg_deltaO.h5' | ||
| else if (components=='ONeNa') then | ||
| filename = trim(phase_sep_data_dir) // '/ONeNa_deltaO.h5' | ||
| else if (components=='COMg') then | ||
| filename = trim(phase_sep_data_dir) // '/COMg_deltaMg.h5' | ||
| end if |
There was a problem hiding this comment.
I don't know how often these data reads are called. If it is on the order of 1 to 10s of times per run its probably fine. Otherwise we might want to keep this around in memory. This isn't a blocker for me to merge in since it is a new feature anyway (and disabled by default?) and leave it for a rainy day.
There was a problem hiding this comment.
Good point! In principle you're definitely right that it would be better to keep these around in memory. I think they get called 100s of times per run. In practice reading these small files doesn't seem to be much of a bottleneck right now, especially after converting them to hdf5, so I think we'll leave it for a rainy day for now.
| ! if true, redistribute all heating associated with phase | ||
| ! separation evenly throughout the inner half of the white dwarf | ||
| ! (prevents unphysical localized spikes at small time steps) |
There was a problem hiding this comment.
Would be good to document the choice for the inner half (could be arbitrary, but I think it would nice for the user to know why).
|
Thanks for the suggestions @VincentVanlaer! I've addressed them (other than the loading into memory suggestion, which I'd like to leave for later), and I added a changelog entry. I think this should be ready to merge at last. I've had a bit of trouble getting the new test case to be robust to small changes in starting model generation when running all inlists, which is why there are several [ci optional] commits here while I was working that out. I've added a few things that will hopefully help it be more robust, but I'll keep an eye on the testing for a while too. |
4 participants
Updated code in phase_separation.f90 and a new wd_o_ne_3_phase test suite.