Repository navigation
Fix issues #241, #240: remove unused Diffusion members; scale only the delta_rhs source by rho_half - #262
Merged
Conversation
…n members; scale only the delta_rhs source by rho_half Issue AMReX-Fluids#241: none of these Diffusion members has a caller anywhere in Source/, Exec/, Tutorials/ or Util/, so delete them: - diffuse_Ssync: a second, independent copy of the scalar sync solve, with its own density handling. The live sync path is diffuse_scalar with add_old_time_divFlux=false, called from NavierStokes::mac_sync. The last call site was already commented out before c55efb1 (2019). - FillBoundary(BndryRegister&, ...): left over from the pre-MLMG boundary-register solvers. - viscFluxReg(), maxOrder(), tensorMaxOrder(): the flux register is reached through NavierStokesBase::getViscFluxReg() instead, and the max_order / tensor_max_order statics are used directly. The member variables and the diffuse.max_order / diffuse.tensor_max_order ParmParse knobs are unchanged. The comment in diffuse_tensor_velocity that cited diffuse_Ssync as the reference for RHS scaling now cites only diffuse_scalar. computeAlpha keeps its alpha_in branch, since diffuse_scalar both forwards alpha_in here and consumes it in its own source loop, but its assert now checks alpha_in->nComp() instead of the freshly defined one-component destination, which only ever admitted alpha_in_comp == 0. Issue AMReX-Fluids#240: for rho_flag==1, the delta_rhs block multiplied the whole RHS by rho_half, including the old-time diffusion term that mgn.apply had already deposited there when add_old_time_divFlux is set. Scale only the source. No current caller passes both delta_rhs and add_old_time_divFlux=true, so this is latent. Verified: Exec/run2d and Exec/eb_run3d build clean with USE_ASSERTION, WARN_ALL and WARN_ERROR; regtest.2d.hotspot (3 levels, scalar diffusion and mac_sync active) run for 10 steps is bit-identical to development (fcompare: PLOTFILE AGREE). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jbbel
approved these changes
Oct 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #241. Closes #240.
#241 — remove
Diffusionmembers that are never exercisedNone of these has a caller anywhere in
Source/,Exec/,Tutorials/orUtil/:diffuse_Ssync(~170 lines). This is a second, independent copy of the scalar sync solve, with its own density logic (computeAlphaon current-timeS, an explicitrho_flag==2rescaling,rho_halfscaling of the RHS) that was never checked against the live path. The live sync diffusion isdiffuse_scalarwithadd_old_time_divFlux=false, called fromNavierStokes::mac_sync. Its last call site was already commented out beforec55efb16(2019).FillBoundary(BndryRegister&, ...)— left over from the pre-MLMG boundary-register solvers.viscFluxReg(),maxOrder(),tensorMaxOrder()— the flux register is reached throughNavierStokesBase::getViscFluxReg(), and themax_order/tensor_max_orderstatics are used directly. The member variables and thediffuse.max_order/diffuse.tensor_max_orderParmParse knobs are unchanged.The comment in
diffuse_tensor_velocitythat citeddiffuse_Ssyncas the reference for RHS scaling now cites onlydiffuse_scalar.computeAlphakeeps itsalpha_inbranch: oncediffuse_Ssyncis gone the remaining forwarder isdiffuse_scalar, which also consumesalpha_inin its own source loop, so it is live API surface rather than dead code. Its assert now checksalpha_in->nComp()instead of the freshly defined one-component destination, which only ever admittedalpha_in_comp == 0and never validated the source at all.#240 — scale only the
delta_rhssource byrho_halfFor
rho_flag==1the source block multiplied the whole RHS byrho_half, including the old-time diffusion term thatmgn.applyhad already deposited there whenadd_old_time_divFluxis set. Only the source should be scaled. Latent today: no caller passes bothdelta_rhsandadd_old_time_divFlux=true.Verification
Exec/run2dandExec/eb_run3dbuild clean withUSE_ASSERTION=TRUE WARN_ALL=TRUE WARN_ERROR=TRUEand the CI'sEXTRACXXFLAGS.grepfor all five removed symbols acrossSource/ Exec/ Tutorials/ Util/ Docs/returns nothing.regtest.2d.hotspot(3 levels, temperature conduction +scal_diff_coefs,mac_syncactive every step), 10 steps, compared against a binary built fromdevelopment:fcomparereports PLOTFILE AGREE — exact zeros on every variable at all three levels. Expected, since nothing removed was reachable and theDiffusion::diffuse_scalar: withrho_flag==1anddelta_rhs, the old-time diffusion term is also multiplied byrho_half#240 path is latent.🤖 Generated with Claude Code