Skip to content

Feature/superad_reduction: opt-in turnover-time limiter with v_c floor and smooth relaxation#1006

Open
matteocantiello wants to merge 9 commits into
fix/superad-reduction-jermyn23-consistencyfrom
feature/superad-turnover-time-limiter
Open

Feature/superad_reduction: opt-in turnover-time limiter with v_c floor and smooth relaxation#1006
matteocantiello wants to merge 9 commits into
fix/superad-reduction-jermyn23-consistencyfrom
feature/superad-turnover-time-limiter

Conversation

@matteocantiello
Copy link
Copy Markdown
Member

@matteocantiello matteocantiello commented May 21, 2026

Summary

This PR adds an opt-in physical refinement to the existing
use_superad_reduction machinery (Jermyn+23), making the radiative-gradient
throttle aware of both the local convective response time and its
previous-step state.

The PR introduces one new feature, together with two coupled improvements that
were identified during validation:

  1. a turnover-time limiter,
  2. a velocity floor for slow-convection cells, and
  3. a smooth-relaxation formula anchored to the previous converged throttle.

All new behavior is opt-in. With defaults, the existing test_suite output is
bit-for-bit unchanged.

What this PR does

1. Turnover-time limiter

New control:

superad_reduction_use_turnover_limit

Type/default:

logical, default .false.

When enabled, the throttle excess

Gamma_fac - 1

is multiplied by

f_tau = 1 - exp(-dt / tau_conv)

where

tau_conv = scale_height(k) / mlt_vc_old(k)

This leaves the throttle essentially unchanged when

dt >> tau_conv

but smoothly attenuates it when

dt < tau_conv

that is, when convection cannot respond as quickly as the implicit step is
asking it to.

2. Velocity floor for slow-convection cells

New companion control:

superad_reduction_turnover_vc_floor_frac

Type/default:

real, default 0d0

Before computing tau_conv, this floors mlt_vc_old(k) at

frac * csound_face(k)

This is needed in radiation-pressure-dominated layers, especially in the
iron-bump surface zone, where MLT can significantly under-predict the relevant
response speed. Even when MLT predicts very small convective velocities,
perturbations can still communicate through the layer on an acoustic timescale.

Without this floor, mlt_vc_old(k) can become so small that

f_tau -> 0

which fully suppresses the throttle in exactly the cells where radiation-pressure
inversions make the Newton solve fragile.

3. Smooth-relaxation formula

The original limiter formula was

Gamma_fac_new = 1 + f_tau * (Gamma_fac_inst - 1)

This relaxes Gamma_fac toward 1, i.e. toward no throttle, when
dt << tau_conv.

That is not the desired behavior when the throttle is already active. In that
case, the throttle should also not be released faster than convection can
respond. Abruptly relaxing toward Gamma_fac = 1 can produce a discontinuous
structural rearrangement, visible as a small HRD jump near the end of the
to_cc phase when the timestep collapses during iron-core formation.

This PR replaces that expression with a first-order relaxation anchored to the
previous step's converged value:

Gamma_fac_new = Gamma_fac_old + f_tau * (Gamma_fac_inst - Gamma_fac_old)

or equivalently,

Gamma_fac_new = (1 - f_tau) * Gamma_fac_old + f_tau * Gamma_fac_inst

This is the discrete form of

dGamma_fac/dt = (Gamma_fac_inst - Gamma_fac) / tau_conv

The limiting behavior is then:

  • dt >> tau_conv: f_tau -> 1, so Gamma_fac -> Gamma_fac_inst
  • dt << tau_conv: f_tau -> 0, so Gamma_fac -> Gamma_fac_old

No additional control is needed. On the first step, when Gamma_fac_old is not
yet available, the guard falls through and the limiter reduces to the original
first-step behavior. On subsequent steps, the smooth-relaxation form is used.

Implementation

Main implementation changes:

File Change
star_data/public/star_data_step_work.inc Adds superad_reduction_factor_old(:) and have_superad_reduction_factor, following the existing paired _old array pattern.
star_data/private/star_controls.inc Declares the two new namelist controls.
star/private/ctrls_io.f90 Wires the controls into &controls, including namelist, store, and write support.
star/defaults/controls.defaults Adds defaults and documentation. Both controls default to off.
star/private/alloc.f90 Allocates superad_reduction_factor_old in star_info_old_arrays, avoiding null-pointer issues in check_sizes.
star/private/evolve_support.f90 Copies superad_reduction_factor into superad_reduction_factor_old at the end of each successful step, mirroring the mlt_vc -> mlt_vc_old pattern.
star/private/init.f90 and star/private/remove_shells.f90 Initialize/reset the have_superad_reduction_factor flag.
star/private/turb_support.f90 Adds the limiter block inside set_superad_reduction, including the velocity floor, smooth-relaxation formula, and guard logic.

Validation

A. 60 Msun A/B/C benchmark

Restart from a photo shortly before the He-exhaustion timestep collapse, then
run through xa_central_lower_limit.

Run End condition Steps Retries
limiter off, baseline xa_central_lower_limit 1940 14
limiter on, no floor max_model_number 6000 780
limiter on, floor = 1d-3 * cs xa_central_lower_limit 1910 15

The bare limiter, without the velocity floor, suppresses the throttle in roughly
5-17 surface iron-bump cells. Radiation-pressure inversions then reappear,
leading to many retries and failure to reach central He exhaustion.

With the 1d-3 * cs floor, those cells remain regularized and the run returns
to baseline-quality numerics while keeping the physical limiter active.

B. Smooth relaxation removes the late-phase HRD jump

For the 60 Msun to_cc phase, restarted from after_core_c_burn:

Formula Endpoint log T_eff Endpoint log L fe_core_infall_limit?
old: Gamma_fac = 1 + f_tau * (Gamma_fac_inst - 1) 3.58 5.58 yes
new: Gamma_fac = Gamma_fac_old + f_tau * (Gamma_fac_inst - Gamma_fac_old) 3.70 6.11 yes

Both runs reach fe_core_infall_limit at the same central T_c / rho_c
state. The smooth-relaxation formula keeps the surface evolution continuous to
the endpoint and removes the visible HRD jump.

C. Full pipelines to core collapse at solar metallicity

With the full feature stack enabled:

superad_reduction_use_turnover_limit = .true.
superad_reduction_turnover_vc_floor_frac = 1d-3

and using the smooth-relaxation formula:

Mass Outcome
30 Msun fe_core_infall_limit reached
40 Msun fe_core_infall_limit reached
60 Msun fe_core_infall_limit reached

Matched controls without the limiter stalled during He burning or earlier.

Backwards compatibility

Both new controls default to off.

With

superad_reduction_use_turnover_limit = .false.

the new limiter code path is skipped, and test_suite output is bit-for-bit
identical to the parent branch.

On the first step with the limiter enabled, have_superad_reduction_factor is
false, so the smooth-relaxation formula falls through and the limiter produces
the same first-step output as the original two-term form. From the second step
onward, the smooth-relaxation form is active.

matteocantiello and others added 4 commits May 20, 2026 15:40
When the new control superad_reduction_use_turnover_limit is .true.,
multiply the throttle excess (Gamma_factor - 1) by

  f_tau = 1 - exp(-dt / tau_conv),  tau_conv = scale_height / mlt_vc_old

in each convective cell. This is the natural first-order response
fraction of a relaxing system with characteristic time tau_conv: the
fraction of the steady-state correction realized in time dt. The
limiter recovers the current behavior in the dt >> tau_conv limit and
smoothly suppresses the throttle in the dt << tau_conv limit -- where
the implicit-step approximation is asking convection to respond faster
than it can.

Implementation:
- new logical control with default .false. (opt-in)
- applied AFTER the existing logistic cap superad_reduction_limit, so
  the cap remains an upper bound on Gamma_factor and the limiter just
  linearly interpolates between 1 and the capped value
- uses mlt_vc_old (real(dp), no autoDiff partials) so tau_conv is a
  frozen exogenous parameter w.r.t. the inner Newton solve --
  avoiding the bistable feedback loop that broke our inner-Picard
  experiments
- scale_height is auto_diff in the calling context; its partials
  propagate cleanly through tau_conv and f_turnover so the outer
  Newton solver sees an analytically-correct linearization

Files:
- star_data/private/star_controls.inc:   new logical field
- star/private/ctrls_io.f90:             namelist entry + read/write
- star/private/turb_support.f90:         limiter block + locals
- star/defaults/controls.defaults:       default + docstring
- docs/source/changelog.rst:             new-feature entry

Default disabled, so every existing test_suite case is bit-for-bit
unchanged. Design document: report/turnover_time_limiter.{tex,pdf}.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_old

set_superad_reduction is called during the relax / model-construction phase,
before s% mlt_vc_old has been allocated and before the first dt is set. The
new turnover-time limiter dereferenced s% mlt_vc_old(k) unconditionally,
which segfaulted whenever superad_reduction_use_turnover_limit was .true.

Gate the limiter on s% have_mlt_vc, associated(s% mlt_vc_old) and
s% dt > 0d0 so the block only runs once a previous-step convective
velocity exists. Pre-evolution callers fall through with Gamma_factor
unchanged (i.e. the unmodified Jermyn23 reduction is applied).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New real control `superad_reduction_turnover_vc_floor_frac` floors
mlt_vc_old at frac * csound_face(k) before computing tau_conv =
scale_height/mlt_vc_old. Slow-convection iron-bump cells otherwise have
tiny mlt_vc_old, hence huge tau_conv, hence f_turnover -> 0 and the
throttle is fully suppressed there even when the rest of the star is
evolving normally. That leaves radiation-pressure inversions unsmoothed
and made the Newton solve grind through hundreds of retries.

Validated on the 60M envelope-issues benchmark: with frac=1d-3 the model
restarted from photos/x00001000 terminates cleanly at xa_central_lower_limit
in 1910 steps with 15 retries — matching the limiter-off baseline (1940
steps, 14 retries) and avoiding the 780-retry / 6000-step grind seen with
the unfloored limiter on.

Default frac=0d0 disables the floor (current behavior preserved bit-for-bit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a paragraph noting that superad_reduction_turnover_vc_floor_frac
exists, what it does (floor mlt_vc_old at frac * csound_face), and
why it matters (without the floor, slow-convection iron-bump cells
zero out the throttle and the Newton solver bogs down). Records the
60 M_sun validation: 15 retries with floor=1d-3 vs 780 retries
without the floor, matching the limiter-off baseline (14 retries).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Debraheem
Copy link
Copy Markdown
Member

Debraheem commented May 21, 2026

Amazing! I think we should refactor this a bit but this is great. I am however a little unconvinced that we should be using an exponential to limit the enhancement. This is one of those cases where I think linear might be more self consistent, and stable, but maybe I can be proven wrong.

@Debraheem Debraheem requested a review from matthiasfabry May 21, 2026 03:52
@aurimontem
Copy link
Copy Markdown
Contributor

Adding
superad_reduction_use_turnover_limit = .true.
superad_reduction_turnover_vc_floor_frac = 1d-3

works great (no crash, no excursions, no spurious oscillations which arise with MLT++ and superad in hydro mode without the limiter) for our hackathon test case with O-burning 25Msun that crashes with excursions w/ and w/o hydro.

Excellent stuff!!!

matteocantiello and others added 3 commits May 21, 2026 19:43
…ep value

The previous turnover-limiter formula

   Gamma_fac_new = 1 + f_tau * (Gamma_fac_inst - 1)

relaxes Gamma_fac toward 1 (no throttle) when dt << tau_conv. That is
physically wrong: it implies convection can instantaneously release the
throttle. The model state in the previous step was adapted to some
throttle value Gamma_fac_old; abruptly resetting Gamma_fac to 1 because
the new dt is short produces a discontinuous structural rearrangement.
In practice this manifests as a small but visible HRD jump at the end of
the to_cc phase, when dt collapses by orders of magnitude during
iron-core formation.

Correct first-order relaxation: anchor at last-step Gamma_fac_old and
relax toward the instantaneous value with characteristic time tau_conv,

   Gamma_fac_new = Gamma_fac_old + f_tau * (Gamma_fac_inst - Gamma_fac_old)
                 = (1 - f_tau) * Gamma_fac_old + f_tau * Gamma_fac_inst

When dt >> tau_conv (steady evolution): f_tau -> 1, Gamma_fac -> Gamma_fac_inst.
Identical to the previous behaviour.
When dt << tau_conv: f_tau -> 0, Gamma_fac -> Gamma_fac_old. Convection
had no time to adapt, so the throttle is held at the previous-step
value.

This is the discrete form of dGamma_fac/dt = (Gamma_fac_inst - Gamma_fac)/tau_conv,
the natural relaxation equation for a quantity with response time tau_conv.

Plumbing
--------
- star_data/public/star_data_step_work.inc: declare
  superad_reduction_factor_old(:) and have_superad_reduction_factor
  alongside the other paired _old arrays.
- star/private/alloc.f90: allocate superad_reduction_factor_old in
  star_info_old_arrays so check_sizes does not trip on a null pointer.
- star/private/evolve_support.f90: copy_to_old(superad_reduction_factor,
  superad_reduction_factor_old) at the end of each successful step. Done
  unconditionally so check_sizes is happy; have_superad_reduction_factor
  gates whether the limiter actually consumes the snapshot.
- star/private/{init,remove_shells}.f90: initialise the flag to false at
  startup and on model reload.
- star/private/turb_support.f90: the formula change. Guard adds
  s%have_superad_reduction_factor and associated(...) to the existing
  5-way guard. Clamp Gamma_fac_old at >= 1 to avoid relaxing from a
  non-physical anchor.

Default-on: the new formula reproduces the old one exactly on the first
step (when have_superad_reduction_factor is false) and is strictly more
correct on every subsequent step. No new control needed.

Validation: 60 M_sun benchmark, to_cc phase, restart from
after_core_c_burn. Both old and new formulae reach fe_core_infall_limit
and end with identical central T_c / rho_c (Si-burning corner). The old
formula's HRD endpoint sits at log Teff=3.58 / log L=5.58 -- the visible
jump. The new formula's endpoint sits at log Teff=3.70 / log L=6.11, on
a smooth continuation of the track. See report/smooth_vs_old_limiter.pdf.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI's sphinx-lint flagged 'density-' at end of line as a dangling hyphen.
Move 'density-inversion' onto the next line as a single compound word.
Pure docstring fix, no semantic change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@matteocantiello matteocantiello changed the title Feature/superad turnover time limiter Feature/superad_reduction: opt-in turnover-time limiter with v_c floor and smooth relaxation May 22, 2026
@aurimontem aurimontem self-requested a review May 22, 2026 00:53
…h-relax also on throttle-release; protect snapshot

Three correctness bugs in the smooth-relaxation limiter, all flagged in
review:

1) Mesh remap. The previous-step Gamma_factor was kept in
   s%superad_reduction_factor_old and read by the limiter as
   Gamma_factor_old(k). But s%superad_reduction_factor was not threaded
   through do_mesh_adjust / do_prune_mesh_surface / prev_mesh restoration
   alongside mlt_vc. After a split/merge/revised mesh, k on the new mesh
   no longer mapped to the same mass coordinate as Gamma_factor_old(k) --
   the limiter was reading a stale-mesh value. Fixed by mirroring the
   full mlt_vc plumbing for superad_reduction_factor:

   - Added prev_mesh_superad_reduction_factor(:) to
     star_data/public/star_data_step_work.inc.
   - Allocation in alloc.f90 alongside prev_mesh_mlt_vc.
   - Save (line 1866-style) and retry-restoration (line 2200-style) in
     evolve.f90.
   - Added to do_mesh_adjust signature; interpolated onto the new mesh
     via do_interp_pt_val (mesh_adjust.f90:~250) with a neutral fill
     value of 1d0 (= no throttle) for cells without overlap.
   - Added to do_prune_mesh_surface signature; pruned via prune1.
   - prev_mesh struct restoration in adjust_mesh.f90:~431.
   - Threaded through the callers in adjust_mesh.f90 and
     remove_shells.f90.

2) Throttle-release. The limiter guard was

       if (... .and. Gamma_factor > 1d0 .and. ...) then

   meaning the relaxation block only fired when the new step needed
   throttling instantaneously. If the previous step was throttled
   (Gamma_factor_old > 1) and the new step had no instantaneous
   trigger (Gamma_factor = 1), the limiter was skipped and the
   throttle snapped to 1 in a single step -- exactly the abrupt
   release the smooth-relaxation form was supposed to prevent.

   The fix:
   - Moved the limiter block out of the enclosing
     `if (Gamma_term > 0d0)` block so it executes regardless of
     whether the instantaneous trigger fired.
   - Replaced the Gamma_factor > 1d0 guard with
     `Gamma_factor > 1d0 .or. Gamma_factor_old > 1d0`, so the
     relaxation fires whenever there is throttling to evolve --
     either tightening it OR releasing it.

3) Start-of-step overwrite. set_vars_if_needed is called at line 1891
   of evolve.f90, before new_generation snapshots the previous step's
   superad_reduction_factor into superad_reduction_factor_old. That
   path eventually calls Get_results -> set_superad_reduction, which
   writes the new step's value into s%superad_reduction_factor. So
   without protection, the snapshot taken at line 1895 would already
   be the new step's value, not the previous step's converged value.

   Fix: added s%okay_to_set_superad_reduction_factor flag (mirror of
   s%okay_to_set_mlt_vc) that gates the write to
   s%superad_reduction_factor(k) at the end of set_superad_reduction.
   The flag is set false at all the same sites that set
   okay_to_set_mlt_vc false (init.f90, evolve.f90:321, evolve.f90:700,
   remove_shells.f90 when removing shells); set true at the same sites
   that flip okay_to_set_mlt_vc true (evolve_support.f90:209 right
   after copy_to_old, read_model.f90:158 after model load).

Validation: 60 M_sun to_cc phase restarted from after_core_c_burn
completes cleanly to fe_core_infall_limit, no SIGSEGVs on the new
arrays, no star_info_old_arrays size-check failures across mesh
adjustments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@matteocantiello
Copy link
Copy Markdown
Member Author

Latest updates

This PR now includes three additional fixes needed to make the smooth
superad-reduction limiter robust across mesh changes, retries, and throttle
release.

1. Mesh remapping of the previous-step superad factor

The previous-step superad-reduction factor is now carried through mesh remaps,
mirroring the existing mlt_vc / mlt_vc_old handling.

This adds the prev_mesh_superad_reduction_factor array and the associated
plumbing through:

  • alloc.f90
  • evolve.f90
  • mesh_adjust.f90
  • adjust_mesh.f90
  • remove_shells.f90

The remap path includes the needed save/restore, interpolation, pruning, and
caller updates. This is required because the smooth limiter uses the previous
step's converged superad-reduction factor; after a mesh change, that quantity
must remain defined on the current mesh.

2. Correct throttle-release behavior

The limiter block has been moved outside the

if (Gamma_term > 0d0)

block.

The guard has also been changed from requiring

Gamma_factor > 1d0

to allowing the limiter to operate when either the new step or the previous step
is throttled:

Gamma_factor > 1d0 .or. Gamma_factor_old > 1d0

This matters because the limiter must control both directions:

  • turning the throttle on when the current step needs it, and
  • releasing the throttle only on the relevant response timescale when the
    previous step was throttled.

Without this, the code could still release the throttle too abruptly when the
instantaneous new value no longer required superad reduction.

3. Snapshot protection

This PR now adds an okay_to_set_superad_reduction_factor flag, mirroring the
existing okay_to_set_mlt_vc pattern.

The flag is set to false during:

  • initialization,
  • start of step,
  • retries,
  • shell removal,

and is set to true only after new_generation snapshots are valid.

The write at the end of set_superad_reduction is gated on this flag.

This prevents invalid or intermediate values of the superad-reduction factor
from being saved and later used as the previous-step reference value. In other
words, the smooth limiter now only relaxes against a clean, converged snapshot.

… dead okay_to_set gate

The previous commit attempted to protect s%superad_reduction_factor from
being overwritten by the start-of-step set_vars_if_needed by mirroring
mlt_vc's okay_to_set_mlt_vc gate. That approach failed in practice: the
flag is set to .true. only in read_model and in the retry path
(set_current_to_old), never in the normal step flow. After step 1, the
gate is permanently false and the gated write
"s%superad_reduction_factor(k) = Gamma_factor%val" never fires.
For mlt_vc this is harmless because s%mlt_vc_ad carries the autoDiff
truth and s%mlt_vc is just a plain-dp snapshot; there is no autoDiff
back-channel for Gamma_factor, so gating its only write killed it.
s%superad_reduction_factor_old ended up frozen at its initial value,
the smooth-relaxation anchor (max(_old, 1d0)) collapsed to 1d0, and the
formula degenerated back to the old 1+f_tau*(inst-1) behaviour. The
HRD endpoint jump returned.

Refactored approach: skip the gate entirely and instead snapshot
s%superad_reduction_factor -> s%superad_reduction_factor_old at a
deterministic point in prepare_for_new_step -- AFTER do_mesh has
remapped s%superad_reduction_factor onto the new mesh (so _old is on
the correct mesh), and BEFORE set_vars_if_needed could overwrite it
with the new step's first-pass value. set s%have_superad_reduction_factor
true at the same point.

Effect:
- The write in set_superad_reduction is now unconditional (matches the
  original behaviour pre-PR), so s%superad_reduction_factor stays live
  across Newton iterations.
- copy_to_old in new_generation no longer manages the snapshot; it
  remains in place to keep s%superad_reduction_factor_old allocated
  (so the existing check_sizes path is happy).
- All okay_to_set_superad_reduction_factor sites removed: declaration,
  init.f90, evolve.f90:321, evolve.f90:702, evolve_support.f90:210,
  read_model.f90:160, remove_shells.f90:1239.

Validation: 60 M_sun full pipeline (60M_cc_smooth_fixed) reaches
fe_core_infall_limit cleanly with the relaxation anchored at the real
previous-step value. Track on the HRD now differs measurably from the
old-formula 60M_cc track, as expected -- the smooth-relaxation is
actually doing work this time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread star/private/turb_support.f90 Outdated
vc_old_floor = s% superad_reduction_turnover_vc_floor_frac &
* s% csound_face(k)
vc_old_local = max(s% mlt_vc_old(k), vc_old_floor, 1d-30)
tau_conv = scale_height / vc_old_local
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to mimic the behavior of public star_utils tau_conv functions for this:

real(dp) function conv_time_scale(s,k_in) result(tau_conv)
type (star_info), pointer :: s
integer, intent(in) :: k_in
integer :: k
real(dp) :: brunt_B, alfa, beta, rho_face, Peos_face, chiT_face, chiRho_face, &
f, dlnP, dlnT, grada_face, gradT_actual, brunt_N2
if (.not. s% calculate_Brunt_B) then
tau_conv = 0d0
return
end if
k = max(2,k_in)
brunt_B = s% brunt_B(k)
call get_face_weights(s, k, alfa, beta)
rho_face = alfa*s% rho(k) + beta*s% rho(k-1)
Peos_face = alfa*s% Peos(k) + beta*s% Peos(k-1)
chiT_face = alfa*s% chiT(k) + beta*s% chiT(k-1)
chiRho_face = alfa*s% chiRho(k) + beta*s% chiRho(k-1)
f = pow2(s% grav(k))*rho_face/Peos_face*chiT_face/chiRho_face
dlnP = s% lnPeos(k-1) - s% lnPeos(k)
dlnT = s% lnT(k-1) - s% lnT(k)
grada_face = alfa*s% grada(k) + beta*s% grada(k-1)
gradT_actual = safe_div_val(s, dlnT, dlnP) ! mlt has not been called yet when doing this
brunt_N2 = f*(brunt_B - (gradT_actual - grada_face))
if(abs(brunt_B) > 0d0) then
tau_conv = 1d0/sqrt(abs(brunt_N2))
else
tau_conv = 0d0
end if
end function conv_time_scale
subroutine set_conv_time_scales(s)
type (star_info), pointer :: s
integer :: k
real(dp) :: tau_conv
include 'formats'
s% min_conv_time_scale = 1d99
s% max_conv_time_scale = 0d0
do k=1,s%nz
if (s% X(k) > s% max_X_for_conv_timescale) cycle
if (s% X(k) < s% min_X_for_conv_timescale) cycle
if (s% q(k) > s% max_q_for_conv_timescale) cycle
if (s% q(k) < s% min_q_for_conv_timescale) exit
tau_conv = conv_time_scale(s,k)
if (tau_conv < s% min_conv_time_scale) &
s% min_conv_time_scale = tau_conv
if (tau_conv > s% max_conv_time_scale) &
s% max_conv_time_scale = tau_conv
end do
if (s% max_conv_time_scale == 0d0) s% max_conv_time_scale = 1d99
if (s% min_conv_time_scale == 1d99) s% min_conv_time_scale = 0d0
end subroutine set_conv_time_scales

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous comment and the following comment only apply if we decide we'd like to make this an explicit quantity that is fixed over solver iterations. Right now you're mixing an implicit scale height with an explicit start of step velocity, and so tau_conv is being recalculated every solver iteration and changing. We should think about if that is desirable or if tau_conv should just be a fixed value over solver iterations.

if we go the explicit path:
We can just compute this quantity once at the start of step so it doesn't need recalculated. Then the simple way to do this is probably to just add a star pointer variable s% tau_conv and set s% tau_conv(k) = tau_conv inside of set_conv_time_scales which is already called with brunt at the start of step here

if (.not. skip_grads) then
if (dbg) write(*,*) 'call do_brunt_B'
call do_brunt_B(s, nzlo, nzhi, ierr) ! for unsmoothed_brunt_B
if (failed('do_brunt_B')) return
if (dbg) write(*,*) 'call set_grads'
call set_grads(s, ierr)
if (failed('set_grads')) return
call set_conv_time_scales(s) ! uses brunt_B
end if

Then you can just check for s%dt/s%tau_conv(k) here inside turb_support.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If convergence is unaffected, the explicit path should be faster, and avoids recalculating things that don't change over the iterations.

vc_old_floor = s% superad_reduction_turnover_vc_floor_frac &
* s% csound_face(k)
vc_old_local = max(s% mlt_vc_old(k), vc_old_floor, 1d-30)
tau_conv = scale_height / vc_old_local
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the zone begins the step as radiative, the limiter is not applying. This needs addressed somehow. I think the best solution is to use a different timescale that doesn't depend on mlt_vc at all. Wheteher it's the 1/N^2 or a sound_crossing/dynamical time.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we think of a better timescale, we won't need an axuillary control for flooring V. Either way, we might want the floor to a parameter rather than a control, since it's not really something that should be changing, rather it's just clipping bad values.

dq_old, q_old, j_rot_old, omega_old, mlt_vc_old
dq_old, q_old, j_rot_old, omega_old, mlt_vc_old, &
superad_reduction_factor_old
! superad_reduction_factor_old: previous step's converged
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these comments are superfluous here. Let's keep the docs in the contols, and keep a brief, concise but clear explanation of what is happening in the superadreduction scheme when it's called.

okay_to_set_mlt_vc, have_mlt_vc
okay_to_set_mlt_vc, have_mlt_vc, &
have_superad_reduction_factor
! true once a step has set
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also superfluous comments that should be removed.

! been populated. Fires only when there is throttling to track --
! either an instantaneous Gamma_factor>1 OR a non-trivial old value
! that has to relax back to 1.
if (s% superad_reduction_use_turnover_limit .and. k > 0 .and. &
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this routine ever called with dt = 0? does mesa ever call mlt dt = 0? maybe during the pre-ms model builder?

associated(s% superad_reduction_factor_old) .and. &
s% dt > 0d0) then
! Clamp at >= 1 so we never relax from a non-physical sub-1 anchor.
Gamma_factor_old_local = max(s% superad_reduction_factor_old(k), 1d0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this statement "! Clamp at >= 1 so we never relax from a non-physical sub-1 anchor."

Shouldn't max(s% superad_reduction_factor_old(k), 1d0) actually be min(s% superad_reduction_factor_old(k), 1d0)? the factor should always be between 0 and 1. And remeshing wouldn't make superad_reduction_factor_old > 1, if it's never > 1 on the previous mesh.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps my confusion here is that we are using gamma_factor instead of grad_scale. I think we should be limiting gradscale not gamma_factor. gradscale is always between 0 and 1

grad_scale = (gradr-gradL)/(Gamma_factor*gradr) + gradL/gradr

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gamma factor can be large and positive

Copy link
Copy Markdown
Member

@Debraheem Debraheem May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or more specifically i think we should be limiting 1/gamma_factor.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay i think this is just a minor bug, here i explain below.

The code uses:

gradr_scaled = gradL + (gradr - gradL)/Gamma_factor

or equivalently:

gradr_scaled = grad_scale * gradr

where:

grad_scale = gradL/gradr + (1/Gamma_factor)*(1 - gradL/gradr)

So Gamma_factor is not the quantity that multiplies gradr going into MLT/TDC, rather Gamma enteres grad_scale in the denominator:

eta = 1/Gamma_factor

because these multiply the effective radiative gradient input.

Since our goal is to limit changes in gradr on a tunrover time, I think we should be limiting eta = 1/gamma_factor or grad_scale, not Gamma_factor itself.

Example:

Gamma_old = 1
Gamma_inst = 100 ! instantaneous gamma that superad wants with no limiter.
f_tau = 0.1

Current PR limiter:

Gamma_new = Gamma_old + f_tau*(Gamma_inst - Gamma_old)
          = 1 + 0.1*(100 - 1)
          = 10.9

eta_new = 1/Gamma_new = 0.092

That means about 91% of the reduction is applied immediately, even though f_tau is only 0.1.

If instead we limit the effective gradr scaling:

eta_old = 1/Gamma_old = 1
eta_inst = 1/Gamma_inst = 0.01

eta_new = eta_old + f_tau*(eta_inst - eta_old)
        = 1 + 0.1*(0.01 - 1)
        = 0.901

Gamma_new = 1/eta_new = 1.11

That applies only about 10% of the reduction, which matches the meaning of f_tau.

The easy soution is just to use use the limiter on eta = 1/Gamma_factor, e.g. using eta:

eta_inst = 1/Gamma_factor ! current iterate gamma_factor before limiting. 
eta_old  = 1/Gamma_factor_old

@aurimontem
Copy link
Copy Markdown
Contributor

So by way of an update on the adapted MESA VI S7.2 25M to O depletion model, without the limiter the model stalls whether or not hydro is on, and without hydro it’s worse. With MLT++ it does not stall but it does produce ratty behavior. With no superad/mlt++, the evolution itself is OK, and it gets to O depletion, though the physicality of MLT itself on the RSG branch is … dubious. With the initial version of the limiter, it jumps (almost instantaneously) from the superad HR track to the standard MLT track and the rattiness picks up at the end — BUT it gets to O depletion fairly easily. With the new version of the limiter where you don’t turn the reduction off, but rather keep it fixed for shorter dt, it gets to O depletion fairly easily, but the evolution in the HR gets very ratty.

image

Might push to core-collapse to diagnose further (and see how much the core evolution is affected)

@Debraheem
Copy link
Copy Markdown
Member

So by way of an update on the adapted MESA VI S7.2 25M to O depletion model, without the limiter the model stalls whether or not hydro is on, and without hydro it’s worse. With MLT++ it does not stall but it does produce ratty behavior. With no superad/mlt++, the evolution itself is OK, and it gets to O depletion, though the physicality of MLT itself on the RSG branch is … dubious. With the initial version of the limiter, it jumps (almost instantaneously) from the superad HR track to the standard MLT track and the rattiness picks up at the end — BUT it gets to O depletion fairly easily. With the new version of the limiter where you don’t turn the reduction off, but rather keep it fixed for shorter dt, it gets to O depletion fairly easily, but the evolution in the HR gets very ratty.

image Might push to core-collapse to diagnose further (and see how much the _core_ evolution is affected)

maybe some of the fixes i suggested will help remove this ratty hr behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants