-
High: hitting
max_nloopis treated as a warning and the run continues with a level that is still known to violate the Y CFL criterion. In _y_discover_nloop_per_level! the code warns, storesnloop[k] = max_nloop, and returns anyway; the caller then applies that invalid level in advect_y_massflux_subcycled!. TM5 does not do that: it aborts onmax_nloopin advecty__slopes.F90. For a debug path, continuing past a known-invalid timestep will generate misleading downstream failures. -
Medium: the new Y
nlooplogic is much closer to TM5 conceptually, but it is still implemented as a full CPU roundtrip for every Y sweep. _y_discover_nloop_per_level! copies the fullmandbmto host, does the search on CPU, then copies the scaledbmandnloopback in mass_flux_advection.jl. That is acceptable as an A100/F64 logic-debug step, but it will be expensive and highly synchronizing in the live Strang path, which calls Y twice per tracer in strang_split_massflux! and strang_split_massflux!. -
Medium: the workspace-backed Y path now follows TM5’s per-level refinement structure reasonably well, but the non-workspace fallback still uses the old static global subdivision. The new implementation is in advect_y_massflux_subcycled!, while the allocating fallback at mass_flux_advection.jl still does
n_sub = ceil(cfl/cfl_limit)and uniformbm/n_sub. Any tests or helper paths using the fallback will not exercise the new TM5-like logic and can give conflicting results. -
Low: the implementation claim is stronger than the actual state. The new Y path is closer to advecty__slopes.F90, but the overall LL path is still not TM5-faithful because prognostic slopes remain disabled in the live dispatch and X still uses the older global-doubling pilot rather than TM5-style local
nloop. So this is a good Y-side correction, not a full TM5 parity milestone yet.
My take: this is the right direction. The Y implementation is materially better than the previous global n_sub approach, and the core search logic does resemble TM5. The main thing I would change next is to make max_nloop failure fatal for the F64 debug path instead of warning-and-continue. After that, I would test whether Y local nloop alone removes the current pole failure before touching pole exclusion. If it does, great. If it does not, then it becomes worth matching TM5’s pole-row treatment next.
Assumptions: this review is of the current modified mass_flux_advection.jl in your worktree, not a committed revision. I did not run the model; this is a static code review.