mesa icon indicating copy to clipboard operation
mesa copied to clipboard

Prevent TDC with predictive mixing

Open Debraheem opened this issue 10 months ago • 5 comments

First stab at addressing gh-issue-607.

I added an error that gets printed if predictive mixing is called while TDC is on. We could probably flag it earlier so you can't set both controls at the same time on start up? Open to any opinions here, should we disallow users from using these controls together, or just give them a big warning?

Debraheem avatar Mar 29 '24 17:03 Debraheem

There is possibly a serious issue in do_predictive_mixing() in the stopping criterion for its search_loop. Note that in this loop we are increasing k_bot_mz = k_bot_mz + 1 when outward=false, and the stopping criterion is in line 491 when k_bot_mz == s%nz-1. But the search will fail if we start the search at k_bot_mz =s%nz-1 because the value is incremented by 1 and then the stopping criterion cannot be hit. This is what is happening in the user issue. What should the search_loop return in this case? set k_bot_mz = s%nz-1 and exit the search loop?

pmocz avatar Jul 03 '24 01:07 pmocz

What should the search_loop return in this case? set k_bot_mz = s%nz-1 and exit the search loop?

That seems like it could work! Would it also make sense to change k_bot_mz == s%nz-1 on line 491 to k_bot_mz >= s%nz-1?

Debraheem avatar Jul 03 '24 02:07 Debraheem

What should the search_loop return in this case? set k_bot_mz = s%nz-1 and exit the search loop?

That seems like it could work! Would it also make sense to change k_bot_mz == s%nz-1 on line 491 to k_bot_mz >= s%nz-1?

It wouldn't quite fix it because eval_mixing_coefficients() operates on the index first, and goes out-of-bounds.

It seems to me that the intention of this function is to return a search result within [1, s%nz-1]. Would @evbauer know more about do_predictive_mixing()?

It seems to me there is also an issue at the other end of the search. If k_top_mz is initially 1 (and outward=true) then the search_loop in do_predictive_mixing() would decrease the index to 0 and not be able to exit at the stopping criterion in line 490

pmocz avatar Jul 03 '24 11:07 pmocz

my recollection from mesa VI is TDC and predictive mixing should not be used together, although i can't seem to find the email thread on this at the moment.

rich townsend, as the primary author of predictive mixing, should weigh in on this issue as well the do loop limits.

fxt44 avatar Jul 03 '24 12:07 fxt44

Sounds like we should make TDC and predictive mixing a run mode that is not allowed, as @Debraheem has nicely done!

@rhdtownsend what are your thoughts on how to fix the out-of-bounds issue in the search_loop in do_predictice_mixing()?

pmocz avatar Jul 03 '24 13:07 pmocz

From a conceptual standpoint, is there a reason that predictive mixing and TDC are mutually exclusive? I use both in novae since the time steps are necessarily short during the ramp-up of the convective runaway, so convective pre-mixing introduces unnaturally fast mixing in boundary regions. Predictive mixing does not instantaneously mix material, but it helps prevent the convective shell from shattering into tons of tiny sub-shells.

I get that there is a numerical issue here, but if the two are not logically inconsistent with each other, I'd rather put up some guardrails rather than forbid the combination altogether. I agree; it would be good to get @rhdtownsend's opinion.

wmwolf avatar Jul 17 '24 19:07 wmwolf

Nice tweak @pmocz! @rhdtownsend can you give it a quick look over before we merge?

Debraheem avatar Aug 14 '24 00:08 Debraheem