MESAHub / mesa

Modules for Experiments in Stellar Astrophysics
http://mesastar.org
GNU Lesser General Public License v2.1
142 stars 38 forks source link

Predictive mixing is broken since #226 #286

Closed annethoul closed 3 years ago

annethoul commented 3 years ago

After Bill's commit 226 (April 2021) that changed 101 files, Predictive mixing does not work anymore. Whether it is used or not leads to the same wrong evolution on the CHeB when using the Ledoux criterion.

rjfarmer commented 3 years ago

Commit c6592ca

Is this a: it no longer does anything whether on/off? Or it does the wrong thing when on?

evbauer commented 3 years ago

Anne's plot from slack seems to show that it just isn't doing anything after #226. image

adamjermyn commented 3 years ago

Is it supposed to look like the red/yellow curves? Is 226 just showing nothing at all? (a bit hard to tell on the plot)

-Adam

On Tue, Jun 22, 2021 at 4:03 PM, Evan Bauer @.***> wrote:

Anne's plot from slack seems to show that it just isn't doing anything after #226 https://github.com/MESAHub/mesa/pull/226. [image: image] https://user-images.githubusercontent.com/18405113/122991667-4ce97480-d373-11eb-8308-a87d585e29ae.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MESAHub/mesa/issues/286#issuecomment-866294106, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPR5H5QSJYBMJKMRWJQRODTUDUABANCNFSM47ELN3WA .

evbauer commented 3 years ago

226 is the black curve showing a roughly constant Mcore around 0.10. It's supposed to grow like red or yellow.

adamjermyn commented 3 years ago

If I want to look into this is hb_2M the case to look at?

Related 1: Is convective premixing still working, or is this purely an issue with predictive mixing? Related 2: If the former is working, do we want to keep predictive mixing around at all? (My understanding was that convective premixing is a strict upgrade relative to predictive mixing...)

fxt44 commented 3 years ago

keep predictive mixing around at all?

yes! it is currently the only formalism that prevents (random) breathing pulses. this is mission critical if one attempts to say something precise about, say, the mass fraction profiles of white dwarfs.

adamjermyn commented 3 years ago

Ok, so we do genuinely need to fix this one. Just making sure :-)

annethoul commented 3 years ago

Yes, this needs to be fixed! I will look at convective premixing to see if it is also broken or not and will let you know. But it is not a strict upgrade of predictive mixing, as Frank says.

wmwolf commented 3 years ago

My understanding is that predictive mixing is also best for simulations that take short timesteps. Convective premixing does unphysical things at the peak of nova bursts (and likely other similar scenarios) since it advances the convective boundary and mixes far faster than is physically possible.

annethoul commented 3 years ago

Agree with Bill. Convective premixing assumes instantaneous mixing. Predictive mixing does not.

adamjermyn commented 3 years ago

I can reproduce this on current head of main. Looking into it.

adamjermyn commented 3 years ago

It's definitely on. I turned on debugging output and it certainly prints things:

 Predictive mixing: i, k_a, k_b, q_a, q_b, superad_min=           1        2512        2476   5.0477468908206591E-002   5.0556394414585926E-002  0.62840621392012275     
 add_predictive_mixing; model, n_conv_bdy=         140           3
 Predictive mixing at convective boundary: i, j=           1           1
   s%predictive_zone_type=any
   s%predictive_zone_loc=core
   s%predictive_bdy_loc=any
 Predictive mixing: i, j, q_top, q_bot:           1           1   5.0477468908206757E-002   0.0000000000000000     
 Exiting predictive search due to abundance reversal
 Predictive mixing: i, k_a, k_b, q_a, q_b, superad_min=           1        2530        2494   5.0477468908206757E-002   5.0556394414586092E-002  0.62840621392336260     
 add_predictive_mixing; model, n_conv_bdy=         141           3
 Predictive mixing at convective boundary: i, j=           1           1
   s%predictive_zone_type=any
   s%predictive_zone_loc=core
   s%predictive_bdy_loc=any

Does anyone know what the debug output means? :P

evbauer commented 3 years ago

I think I've found the issue. Explanation incoming...

evbauer commented 3 years ago

Predictive mixing needs to have gradL_composition_term set to 0 in its calls to MLT. The merge in #226 seems to have inadvertently undone that, and so predictive mix doesn't work. Should be an easy fix. I'll commit soon.

adamjermyn commented 3 years ago

Why does it need that?

evbauer commented 3 years ago

I think uniform composition is a fundamental assumption of the predictive mixing iterations. See section 2.1 in MESA IV.

adamjermyn commented 3 years ago

Oh I see! The assumption is that the composition must be uniform in the convection zone. And that means that calls to MLT have to assume this when predicting the mixing (because otherwise they'll say 'wait this is stable, vc=0'. Right?

-Adam

On Wed, Jun 23, 2021 at 4:54 PM, Evan Bauer @.***> wrote:

I think uniform composition is a fundamental assumption of the predictive mixing iterations. See section 2.1 in MESA IV.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MESAHub/mesa/issues/286#issuecomment-867150711, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPR5H4B4W62OF3IVWL7OJ3TUJCXVANCNFSM47ELN3WA .

evbauer commented 3 years ago

Right. I'm reading the old version of the code now to make sure I can get things restored properly.

adamjermyn commented 3 years ago

I love your commit (hb_2m should know that it's failing).

evbauer commented 3 years ago

Hopefully it will be shamed into compliance. (And thanks to Josiah for doing all the hard work setting up that infrastructure so that hopefully I just needed to put reasonable numbers in the inlist.)

evbauer commented 3 years ago

Fixed in 226cc95

For completeness, I should say that there are two MLT calls in predictive_mix.f90, but the one in the restore_cell_loop actually is meant to allow gradL_composition_term to be set. So that one was functioning properly, and only one of the MLT calls needed to be changed.