Repository navigation
Coupled step: a direct solve as the reference, and fixes after #28 - #29
Merged
Merged
Conversation
The predictor's last field (Eϕ_self_prev), the friction's part at t^n (Rue_ei) and an unset coil's plasma flux memory were written before the outer iteration. They are now written when an evaluation is accepted; the coil memory the circuits start from comes from coil_plasma_flux_memory, which writes nothing. Results are unchanged bit for bit.
coil_driven_column said the 1 A run jumps before the coupled solve takes over, which the gate re-solve and the script's own check contradict. picard_center_column described the old relaxed default (w = 0.5, 10 solves).
anderson_solve! becomes fixed_point_solve!, which calls fixed_point_step! on its stepper: AndersonMixer, or the new NewtonStepper, x + (1 - T)^-1 f with a factorization of W (1 - T) W^-1. On an affine map Newton's step lands on the fixed point at once; a residual no smaller than the best stalls, which ends the solve (accepted if it meets the stopping test).
coupled_map_jacobian assembles the outer map's linear part T column by column (N_b + N_c back-substitutions on the step's block LU), and Newton's step on W (1 - T) W^-1 lands on the fixed point from the first iterate; later evaluations only remove rounding. flags.ampere_picard.method selects it (:anderson by default). The coupled-solve tests now compare the default with this direct solve instead of a relaxed iteration run to 1e-10.
…embly solve_coupled_momentum_Ampere_equations_with_coils! (u-par eliminated, a full LU per iteration, the relaxed iteration, and an in-grid coil source without A_u), its stopping test picard_step_converged, and the export combine_Au_and_DeltaGS_sparse_matrices, which CoupledBlock replaced. Their tests now check the direct solve against the iteration, and the fixed-pattern block against the block matrix sparse algebra assembles.
DIRECT_PICARD (method = :direct, checked to 1e-10 with no floors) replaces the relaxed iteration run to 1e-10 as the expected step. All five cases pass with the same gaps as before; the reference takes 2 block solves per step instead of a few hundred.
…docs - distribute_coil_currents_to_Jphi! skipped currents below eps (in amperes), so the in-grid coils' source was not linear in their currents, which the direct solve assumes; it now skips exact zeros only. - NewtonStepper keeps its best iterate and returns it on :exhausted, as the fixed_point_step! contract says. - The coupled solve's docstring: a throw leaves u-par, the fields and the coils unchanged (the step's caches may be rebuilt), and the direct solve needs no eigenvalue of T equal to 1.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #29 +/- ##
==========================================
- Coverage 93.69% 93.64% -0.05%
==========================================
Files 47 50 +3
Lines 5075 5130 +55
==========================================
+ Hits 4755 4804 +49
- Misses 320 326 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
OuterSolvePolicy, as IonTransportPolicy: AndersonOuterSolve(; memory, relaxation_w), the default, and DirectOuterSolve(), the reference. Anderson's parameters move from PicardSettings into its policy, which checks them (memory >= 0, a finite positive weight; above 1 over-relaxes). outer_stepper resolves the policy, so the solve reads the default first and the reference code (outer_stepper for DirectOuterSolve, coupled_map_jacobian) sits below it.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Direct solving fails for valid zero-inductance coils, and policy conversion can bypass its finite-positive invariant.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds a direct reference solver for the coupled Ampère step while retaining Anderson mixing as the default.
Changes:
- Introduces outer-solve policies and a shared fixed-point driver.
- Adds direct affine-map solving with safer state acceptance.
- Updates coil deposition, tests, and examples.
| File | Description |
|---|---|
src/types.jl |
Defines outer-solve policies and settings. |
src/RAPID2D.jl |
Includes fixed-point numerics. |
src/physics/physics.jl |
Implements direct coupled solving. |
src/numerics/fixed_point.jl |
Adds the shared solver and Newton stepper. |
src/numerics/anderson.jl |
Adapts Anderson mixing to the shared driver. |
src/coils/circuit_equations.jl |
Makes coil deposition linear and adds non-mutating memory access. |
test/unit/physics/ampere_picard_test.jl |
Tests policies, direct solving, and rollback. |
test/unit/numerics/fixed_point_test.jl |
Tests Newton and fixed-point behavior. |
test/unit/numerics/anderson_test.jl |
Updates shared-driver tests. |
test/unit/coils/coil_plasma_flux_test.jl |
Compares iterative and direct coil steps. |
test/unit/coils/circuit_equations_test.jl |
Tests small-current linearity. |
examples/README.md |
Documents direct-reference comparisons. |
examples/coupled_step/common.jl |
Uses the direct reference in examples. |
examples/coupled_step/picard_regime_map.jl |
Updates regime-map comparisons. |
examples/coupled_step/picard_tight_box.jl |
Updates reference description. |
examples/coupled_step/picard_kstar_inboard_limited.jl |
Updates reference description. |
examples/coupled_step/picard_filament_shell.jl |
Updates reference description. |
examples/coupled_step/picard_center_column.jl |
Documents current solver behavior. |
examples/coupled_step/coil_driven_column.jl |
Documents gate-crossing behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… as stored A Coil with zero self-inductance (also the old default when it was left out) is refused when it is made: a toroidal loop always has some, and without it the coils' inductance matrix is indefinite and the direct solve's weights divide by it. The check runs once per coil, not per step. AndersonOuterSolve checks relaxation_w after converting it to Float64.
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.

Summary
A follow-up to #28. The coupled solve gains an opt-in direct method. It solves a step's equations at once, without iterating, and the default iterative solve is now checked against it. With this PR:
flags.ampere_picard.method = DirectOuterSolve()assembles the outer map's linear part and solves it in one Newton step. It replaces the earlier references: the relaxed iteration run toAndersonOuterSolve().IonTransportPolicyis:AndersonOuterSolve(; memory, relaxation_w), the default, andDirectOuterSolve(), the reference. Anderson's parameters move into it fromPicardSettings.Each item has a test.
The direct solve
The outer unknowns are the edge flux and the coil currents,$x = (\psi_b, I_c)$ . With the step's coefficients held at $t^n$ , every equation of the step is linear in its unknowns, so the map the iteration solves is affine, $g(x) = T x + c$ . Here $T$ is the response of $x$ to itself through the plasma current, and $c$ collects the forcing. The step solves
coupled_map_jacobian). ColumnNewtonStepper).anderson_solve!becomesfixed_point_solve!. It drives eitherAndersonMixerorNewtonStepperunder the same acceptance policy;outer_stepperbuilds one from the policy.Review comments on #28
Tests and examples
fixed_point_test.jl: Newton's step on an affine map that the relaxed iteration diverges on, with unknowns over twelve decades; a step that stalls; residuals that are not finite; the driver's handling of a stalled evaluation.ampere_picard_test.jl:coil_plasma_flux_test.jl: with a powered coil and a resistive loop, the direct solve takes the step the iteration converges to.circuit_equations_test.jl: the coils' source on the grid is linear in their currents, however small.examples/coupled_step/picard_*: the default solve against the direct solve.Behaviour changes
solve_coupled_momentum_Ampere_equations_with_coils!. It eliminated u∥, took a full LU every iteration, and added the in-grid coils' source withoutpicard_step_converged.combine_Au_and_ΔGS_sparse_matrices, whichCoupledBlockreplaced.PicardSettings:anderson_mandrelaxation_wbecomemethod = AndersonOuterSolve(; memory, relaxation_w), orDirectOuterSolve(). A relaxation weight above 1 (over-relaxation) is now allowed; it must be finite and positive.Coilneeds a positive self-inductance. Zero, or leaving it out (it defaulted to zero), is refused when the coil is made. A toroidal loop always has some, and without it the coils' inductance matrix is indefinite; the direct solve's weights also divided by it.Remaining (unchanged from #28)
Time levels at the step's start and end, plasma motion reaching the coils one step late, an older LU as a preconditioner, and the nonlinear coupling.