Repository navigation
Conversation
…m_eos patch_ib%thermal_bc = 1 now also applies without chemistry, on the Fourier conduction path (fluid_pp%k_therm). The ghost keeps the image-point pressure; its temperature is mirrored about Twall and its density rebuilt with s_phase_temperature and the new s_phase_density, so the ghost-point kernel carries no EOS coefficients. s_blend_ghost_state is split into temperature and species halves; the temperature limiter takes its bounds as arguments (the NASA window with chemistry, positivity without) and is unchanged for chemistry. The validator requires k_therm > 0, num_fluids = 1 and Twall > 0 without chemistry.
|
Claude Code Review Head SHA: 4442ef8 The review produced no output (Claude step: failure). Run and transcript: https://github.com/MFlowCode/MFC/actions/runs/38328154585 |
…ess (#2043) * Review PRs without a shell The review prompt only reads prepared inputs -- the diff, the changed-file list, the bounded context directory and three docs -- and writes one file, .claude-review/output.md, which a later step publishes. It never needed to run commands, so grant Read/Write/Glob/Grep instead of Bash and drop --dangerously-skip-permissions. The two prompt steps that said "cat" now say "Read". This is worth having on its own, and it is a precondition for opening the review to authors without write access: shell access in a job holding the OAuth token is the payload a prompt injection in a PR diff would want. * Review every PR, including from authors without write access claude-code-action refuses to run when the triggering actor lacks write access ("Actor does not have write permissions to the repository"), so outside contributors' PRs got no review -- just the fallback comment saying the review produced no output. #2038 and #1926 are both in that state. Set allowed_non_write_users: "*", the action's supported way to lift that check. It only takes effect because github_token is already passed below it. allowed_bots is left at its default, so bots still cannot trigger a review. The action's docs call this bypass risky and ask that it be paired with a limited permission scope, which is why the preceding commit took the shell away: Claude now has Read/Write/Glob/Grep, cannot reach the token or the GitHub API, and only writes .claude-review/output.md for a later step to publish. The checkout stays on the base ref, so a PR's code is never executed here. Note this opens the OAuth token's budget to anyone who opens a PR.
Lines of Code
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2038 +/- ##
=======================================
Coverage 61.91% 61.91%
=======================================
Files 86 86
Lines 22848 22898 +50
Branches 3360 3369 +9
=======================================
+ Hits 14147 14178 +31
- Misses 6207 6221 +14
- Partials 2494 2499 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| ! merely above zero. Positivity alone is too weak there: a 300 K wall in 1500 K gas admits theta = 0.225, and the 30 K | ||
| ! ghost temperature that follows is positive but evaluates the NASA polynomials far below their T_low. Both ends are | ||
| ! constrained, since a hot wall extrapolates the other way. T_s itself can sit outside the window only if the case | ||
| ! prescribed a Twall there; theta = 0 then hands back exactly that value rather than quietly substituting a different |
There was a problem hiding this comment.
this is too much comment text
| # thermal_bc is acted on by the ghost-state reconstruction in s_ibm_correct_state, which an | ||
| # injecting surface bypasses. Without chemistry only thermal_bc = 1 has a branch there, and | ||
| # it rebuilds the fluid-1 ghost density through m_eos, so it needs the Fourier conduction | ||
| # path (stiffened/ideal gas only) to carry the wall flux and a single fluid. Left to |
| - `slip` applies a slip boundary to the surface of the patch if true and a no-slip boundary condition to the surface if false. | ||
|
|
||
| - `thermal_bc` selects the thermal immersed-boundary condition. A value of 0 applies a zero-normal-gradient temperature condition, 1 prescribes the wall temperature using `Twall`, and 2 solves the reacting-surface energy balance for the surface temperature. The `thermal_bc = 2` option requires `surface_reaction = 1`. A non-zero `thermal_bc` requires `chemistry = T` and cannot be combined with `inj_species > 0`, since the thermal condition is applied by the chemistry ghost-state reconstruction, which an injecting surface bypasses. | ||
| - `thermal_bc` selects the thermal immersed-boundary condition. A value of 0 applies a zero-normal-gradient temperature condition, 1 prescribes the wall temperature using `Twall`, and 2 solves the reacting-surface energy balance for the surface temperature. The `thermal_bc = 2` option requires `surface_reaction = 1`. A non-zero `thermal_bc` cannot be combined with `inj_species > 0`, since the thermal condition is applied by the ghost-state reconstruction, which an injecting surface bypasses. Without chemistry, `thermal_bc = 1` requires Fourier conduction (`fluid_pp(1)%%k_therm > 0`) and `num_fluids = 1`: the ghost keeps the image-point pressure and takes the density of the mirrored temperature through the equation of state. |
There was a problem hiding this comment.
can we rewrite this without AI
| - `thermal_bc` selects the thermal immersed-boundary condition. A value of 0 applies a zero-normal-gradient temperature condition, 1 prescribes the wall temperature using `Twall`, and 2 solves the reacting-surface energy balance for the surface temperature. The `thermal_bc = 2` option requires `surface_reaction = 1`. A non-zero `thermal_bc` cannot be combined with `inj_species > 0`, since the thermal condition is applied by the ghost-state reconstruction, which an injecting surface bypasses. Without chemistry, `thermal_bc = 1` requires Fourier conduction (`fluid_pp(1)%%k_therm > 0`) and `num_fluids = 1`: the ghost keeps the image-point pressure and takes the density of the mirrored temperature through the equation of state. | ||
|
|
||
| - `Twall` specifies the prescribed surface temperature when `thermal_bc = 1` and must be positive in that case. | ||
| - `Twall` specifies the prescribed surface temperature when `thermal_bc = 1` and must be positive in that case. With chemistry it must also lie within the tabulated thermodynamic range [200, 5000] K. |
There was a problem hiding this comment.
where does the table come from?
|
|
||
| $:GPU_ROUTINE(function_name='s_phase_density', parallelism='[seq]', cray_inline=True) | ||
|
|
||
| real(wp), intent(in) :: pres, T |
There was a problem hiding this comment.
why does this subroutine just call a function
| if (Ys_s(k) < Ys_IP(k)) theta_Y = min(theta_Y, blend_safety*Ys_s(k)/(Ys_IP(k) - Ys_s(k))) | ||
| end do | ||
|
|
||
| T_g = T_s + theta_T*(T_s - T_IP) |
Contribution Policy
We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
PR template credit: junegunn
Summary
patch_ib%thermal_bc = 1(prescribedTwall) now also works withchemistry = F, on the Fourier conduction path of #1906 (fluid_pp%k_therm). Until now an IB without chemistry was always adiabatic. Non-reacting single-gas cases (heated/cooled particle beds, shock–particle) need a fixed surface temperature without turning on the chemistry machinery.Discussed on the CM3C Slack and at the CM3C weekly meeting with @sbryngelson and @danieljvickers.
Implementation
In
s_ibm_correct_state, forchemistry = Fandthermal_bc = 1only:s_phase_temperature, and the ghost density from the news_phase_densityinm_eos(the inverse ofs_phase_temperatureat fixed pressure). The ghost-point kernel holds no EOS coefficients.T_g = T_w + θ (T_w − T_IP), with θ = 1 (the mirror) unless the temperature limiter cuts it. The pressure and velocity are unchanged.q_T_sfat the ghost is thereforeT_g, and the existingm_conductionface flux gives the wall heat flux. There is no new kernel.s_blend_ghost_stateis split intos_blend_ghost_temperature(which takes its bounds as arguments) ands_blend_ghost_species. With chemistry the bounds are the NASA window, unchanged. Without chemistry the only bound is positivity, since there is no fit range. Each bound is compared before dividing, soT_hi = huge()cannot overflow.thermal_bc = 0without chemistry is untouched (the existing mirror is already adiabatic).thermal_bc = 1requiresk_therm > 0,num_fluids = 1andTwall > 0, and anythermal_bcother than 0 or 1 is rejected.k_thermalready limits the EOS to ideal or stiffened gas, so the state-dependent families never reachs_phase_density. The 200–5000 KTwallwindow now applies only with chemistry.No new parameter:
heat_conduction(anyk_therm > 0) acts as the switch. A temperature flag separate from chemistry, as Dan suggested, can come later.Verification
A cylinder (2D) or sphere (3D) held at
Twallin quiescent air (ideal gas, constant k) in a periodic box, run to τ = αt/R² = 1. This is pure conduction: no gravity, and no flow except the expansion or contraction the heating drives.Reference. A 1-D radial solution of the low-Mach equations in the same closed volume (Majda & Sethian 1985): variable density ρ = p₀/(R T), the induced radial velocity from ∇·u, and the uniform thermodynamic pressure p₀(t) of the closed box. These are the equations MFC solves as Ma → 0. As ΔT → 0 the solver reduces to the constant-density solution and, with an open far field, to Carslaw & Jaeger. Errors below are for the absorbed heat (MFC: change of the total gas energy in the closed box) and the rms gas temperature.
Reference solution: equations, numerics and sources
Radial domain R < r < R_out, where R_out gives the same gas volume as the periodic box (d = 2 cylinder, d = 3 sphere), with an adiabatic, impermeable outer wall:
with T(R, t) = T_w, u(R) = u(R_out) = 0, T(r, 0) = T_∞ and p_0(0) = p_∞. Here \dot Q_gas is the heat flow from the wall into the gas. The absorbed heat follows from the thermodynamic pressure, H(t) = V_gas (p_0(0) − p_0(t))/(γ − 1), which is the change of the total gas energy that MFC measures.
Numerics: finite volume with 1500 cells clustered at the wall, BDF time integration. Grid-converged to 1e-6 in H, and gas mass conserved to 1e-10 (ΔT = 6 K) and 1e-5 (ΔT = 300 K). As ΔT → 0 it matches the constant-density solution to 1e-4. That solution, in turn, reproduces the Carslaw & Jaeger sphere closed form and cylinder integral to 1e-5 when given an open far field and dp_0/dt = 0.
Small ΔT (wall 294 K, gas 300 K):
Large ΔT, compressibility (wall 600 K, gas 300 K; the near-wall density halves):
MFC converges to the variable-density solution and not to the constant-density one, so the density change and the flow it drives are captured. Both errors are first order, as expected for a ghost-cell IB.
Other checks
2D -> 1 Fluid(s) -> IBM -> Isothermal(5BBDFB29), whose golden holds the isothermal ghost densities.Relation to other work
s_blend_ghost_*calls.ib_ip_min_distaddresses that at its source, and this path should benefit from it directly; I haven't tested that combination yet.s_blend_ghost_temperature, as a stability guard rather than an accuracy fix.thermal_bc = 3(Add thermal_bc = 3: a lumped solid whose wall temperature follows its heat balance (after #1956) #2058), the validator here already rejects it without chemistry rather than ignoring it silently.thermal_layer, the initial density ρ ∝ 1/T at fixed pressure holds for ideal gas but not for stiffened gas.