preionfrac as a function of z#417
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the nonlinear response pipeline to propagate the current propagation position z into time-domain response evaluation, enabling preionfrac (pre-ionisation fraction) to be specified as a function of z.
Changes:
- Thread
zthroughEt_to_Pt!and its call sites so responses can be evaluated with position context. - Update built-in nonlinear responses (Kerr, Raman, Plasma) to accept
; z=...and use it for z-dependent pre-ionisation inPlasmaCumtrapz. - Add helper
getpreionfracand documentation clarifying constant vs z-dependentpreionfrac.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/NonlinearRHS.jl |
Adds z keyword plumbing to Et_to_Pt! and updates transform call sites to pass the current z. |
src/Nonlinear.jl |
Updates response call signatures to accept ; z, and implements z-dependent preionfrac support in PlasmaCumtrapz. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function Et_to_Pt!(Pt, Et, responses, density::Number; z=0.0) | ||
| fill!(Pt, 0) | ||
| for resp! in responses | ||
| resp!(Pt, Et, density) | ||
| resp!(Pt, Et, density; z=z) | ||
| end |
There was a problem hiding this comment.
Et_to_Pt! now unconditionally calls each response as resp!(...; z=z). This is a breaking change for any externally-supplied response that previously only implemented (out, E, ρ) (it will now throw a MethodError due to an unexpected keyword). Consider either (a) documenting/enforcing the new response signature at the public API boundary (e.g. wrapping user responses on input so they accept ; z), or (b) adding a fast fallback path that calls without z when the method doesn’t accept that keyword, to preserve backward compatibility.
| if preionfrac > 0.0 | ||
| @warn("Using preionfrac > 0.0 is not a well founded physical model. Use only after careful consideration.") | ||
| end | ||
| else # preionfrac is a callable |
There was a problem hiding this comment.
For the callable preionfrac branch, the constructor only emits a warning but does not validate that preionfrac is actually callable with a single z argument. Passing a non-Number non-callable value will fail later at runtime inside getpreionfrac/Plasma* with a MethodError. Consider adding an applicable(preionfrac, 0.0) (or similar) check here and throwing a clear ArgumentError if it isn’t callable.
| else # preionfrac is a callable | |
| else # preionfrac is intended to be a callable | |
| if !applicable(preionfrac, 0.0) | |
| throw(ArgumentError("preionfrac must be either a Number in [0, 1] or a callable that accepts a single Float64 argument (z).")) | |
| end |
| if Plas.preionfrac isa Number | ||
| return Plas.preionfrac | ||
| else | ||
| return Plas.preionfrac(z) |
There was a problem hiding this comment.
getpreionfrac returns Plas.preionfrac(z) without validating the result. Unlike the constant Number path, this means out-of-range values (<0 or >1) (or non-numeric returns) will silently propagate and can produce unphysical ionisation fractions and downstream computations (e.g. (1 - Plas.fraction[ii]) becoming negative). Consider validating the evaluated value (and throwing DomainError like the constant case) at least once per call site (e.g. inside getpreionfrac).
| return Plas.preionfrac(z) | |
| val = Plas.preionfrac(z) | |
| if !(val isa Number) | |
| throw(DomainError(val, "preionfrac(z) must return a numeric value between 0 and 1")) | |
| end | |
| !(0.0 <= val <= 1.0) && throw(DomainError(val, "preionfrac must be between 0 and 1")) | |
| return val |
| function PlasmaCumtrapz(t, E, ratefunc, ionpot; preionfrac=0.0) | ||
| rate = similar(t) |
There was a problem hiding this comment.
This change introduces new behavior where preionfrac may be a z-dependent callable. There are existing tests covering constant preionfrac domain validation (see test/test_ionisation.jl), but there is no coverage for (1) callable preionfrac being accepted and applied at different z values, and (2) callable returns outside [0,1] producing a clear error. Adding tests for these cases would help prevent regressions.
No description provided.