Repository navigation
Coupled step: u∥ solves the same equation on both sides of the Ampère gate - #30
Merged
Merged
Conversation
…pere gate - The coupled solve adds u_para diffusion with the operator update_ue_para! uses (A_diffu_e), theta-weighted as advection is, and reads theta from theta_imp.decay instead of fixing 1; Rue_ei follows it. - New theta_imp.circuit (default 1) weights the coils' resistive term in advance_coils! and in the coupled solve; both forced 1 before. - The psi predictor is the extrapolated mean induced field, without theta (unchanged at theta = 1). - coupled_ue_para_test.jl: given the step's induced field, update_ue_para! reproduces the coupled u_para and Rue_ei to rounding at theta = 0, 0.5, 1; Ampere on and off agree at weak coupling; the circuits' theta reaches both sides of the gate.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 93.64% 93.72% +0.07%
==========================================
Files 50 50
Lines 5130 5131 +1
==========================================
+ Hits 4804 4809 +5
+ Misses 326 322 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- update_ue_para!'s explicit branch builds the same right-hand side as the implicit one, with advection and diffusion at theta_op = 0, and divides by the friction's diagonal. Before, it applied advection to the partly updated u_para and had no diffusion. - The coupled solve honours Implicit: theta_op = 0 for advection and diffusion, theta_imp.decay on the friction and Rue_ei. - Tests: the u_para identity and the weak-coupling comparison with Implicit on and off; Implicit = false equals the implicit update at theta = 0 to rounding. - physics_test: the explicit drift golden re-measured (diffusion now in the explicit update; the old value is reproduced to 1e-16 with it off).
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Add coverage for the advertised diffusion-disabled control; circuit documentation also needs updating.
Review effort: Lite
Findings: None
What changed in this PR
Aligns the coupled Ampère solve with update_ue_para!, adding consistent diffusion, θ-weighting, explicit stepping, and independently weighted circuits.
Changes:
- Adds θ-weighted diffusion and explicit-mode consistency.
- Adds
θ_imp.circuitfor circuit integration. - Expands coupled-step regression coverage and updates the explicit baseline.
| File | Summary |
|---|---|
test/unit/physics/physics_test.jl |
Updates the explicit diffusion baseline. |
test/unit/physics/coupled_ue_para_test.jl |
Tests coupled/uncoupled momentum and circuit consistency. |
src/types.jl |
Adds circuit implicit weighting and documentation. |
src/physics/physics.jl |
Updates momentum, diffusion, and circuit stepping logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- calculate_circuit_matrices!, advance_LR_circuit_step! and CoilSystem now
state the theta-scheme the circuits run, (M + theta dt R) I^{n+1} =
M I^n - (1 - theta) dt R I^n + dt V^{n+1/2} - ..., with theta from
flags.theta_imp.circuit, and list the fields CoilSystem actually has.
- initialize_coil_system! takes theta from flags.theta_imp.circuit, so the
matrices are built with the run's weight from the start.
- The identity test of the coupled step runs diffusion off as well as on,
the control the PR describes (lost when the Implicit axis was added).
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.
Coupled step: u∥ solves the same equation on both sides of the Ampère gate
Summary
At the Ampère gate, u∥ passes from
update_ue_para!to the coupled solve, and the two did not solve the same equation:Include_ud_diffu_term, on by default, stopped acting once the plasma current reached the gate. MATLAB'sSolve_combined_ud_GS_equations_with_coilshas the same gap, and the port had kept it as aTODO.update_ue_para!readsθ_imp.decay.Implicit = false. The coupled solve ignored it.update_ue_para!'s explicit branch applied advection to the partly updated u∥ and had no diffusion, so it was not the θ = 0 case of its own implicit update.With this PR:
update_ue_para!'s, term by term and with the same θ. Diffusion uses the same operator, θ-weighted as advection is.Implicit = falseis the θ = 0 case of the same equation, for advection and diffusion, in both paths.update_ue_para!keeps its update without a matrix solve.θ_imp.circuit(default 1), read on both sides of the gate. Before, both sides forced θ = 1 on them.Impliciton and off. Given the field a step induces,update_ue_para!reproduces the coupled solve's u∥ to rounding, however strong the coupling.The u∥ equation
With the coefficients at$t^n$ , the first row of the coupled step is
update_ue_para!uses.θ_imp.decayon the friction and its ledgerRue_ei.θ_imp.decayon the nonlocal operators, or 0 withImplicit = false. The friction keeps its weight there: it is local, so the update still needs no matrix, and it is stiff.test_iFPC.mruns, diffused u∥ andCoupledBlockkeeps its sparsity pattern and its symbolic LU.DirectOuterSolvetakes the term in through the same LU.The circuits' θ
The coils' circuits are
θ_imp.circuit.advance_coils!reads it below the gate and the coupled solve above it.CoilSystem.θimponly records the weight its matrices were built with.θ_imp.circuit = 0.5makes the circuits second order.Tests
coupled_ue_para_test.jl. Each test was written first and failed before the change it covers: with diffusion on, at θ = 0 and ½, and for want ofθ_imp.circuit. Diffusion off is a control.update_ue_para!is given the step's induced fieldRue_eito rounding, at θ = 0, ½ and 1, withImpliciton and off, and with diffusion on and off. The circuits stay at their own weight.Implicit = falseis the θ = 0 case. With every u∥ term on andBehaviour changes
Include_ud_diffu_term = truechange above the gate. Below the gate, or with the flag off, nothing changes.θ_imp.decay = θ_imp.circuit = 1) the θ change leaves the coupled solve as it was, its first outer iterate included. Aθ_imp.decayother than 1 now applies above the gate too.Implicit = falseruns change: advection now acts onImplicitWeightsgainscircuit. Its positional constructor takes five weights; the keyword form is unchanged.full_startupis unchanged below the gate. Above it, the plasma current rises a little more slowly, and the gap narrows by the end of the run.force_balance_controlkeeps its verdict.coupled_step/*andcurrent_diffusionswitch diffusion off, so they are unchanged.Remaining