Skip to content

Equilibrium - BUGFIX! - Build F and P of g-file and IMAS equilibria from the tabulated FF′ and p′ - #506

Closed
d-burg wants to merge 1 commit into
developfrom
bugfix/efit-profiles-from-file-derivatives
Closed

d-burg wants to merge 1 commit into
developfrom
bugfix/efit-profiles-from-file-derivatives

Conversation

@d-burg

@d-burg d-burg commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Release note

  • Audience: users
  • Numerical impact: every g-file (efit, efit_arclength, efit_by_inversion) and IMAS result moves. On the DIII-D-like example: Δ′ diagonal up to 15 %, free-boundary energy 0.6 %, SLAYER γ 6 %, NTV torque 2.8 %. Solovev is unchanged. (harness @ b3f412f)
  • Migration: re-pin local baselines for g-file and IMAS cases; set profile_source = "values" in [Equilibrium] to reproduce earlier results exactly.

F and P of a g-file or IMAS equilibrium are now the integrals of the file's FF′ and p′, anchored at the boundary values, instead of the tabulated F and P. Differentiating a tabulated F amplified its rounding error into the current gradient, which put noise of several units on Δ′ for 257-node files written in single precision. Closes #504.

What changed

  • read_efit and read_imas fill the F and P columns of the 1D table through one helper (integrate_profile_derivatives), always for both profiles together. Everything downstream is untouched and still differentiates one spline.
  • New [Equilibrium] key profile_source: "derivatives" (default) or "values" (previous behaviour, bit-identical to develop).
  • The reader logs the largest interior difference between the rebuilt and tabulated profiles and warns above 1e-4 of F or 5 % of the peak pressure. It falls back to "values" with a warning when the derivative arrays are absent, all zero, non-finite, or opposite in sign for only one of the two profiles.

Regression report

regress --cases diiid_n1,diiid_n1_riccati,diiid_slayer_n1,diiid_error_field,diiid_multi_n,efit_fixedbdy_separatrix,gal_resistive_diiid,gal_resistive_pe,solovev_n1 --refs 0e68a0553,b3f412f35 --force (julia 1.11.9, x86_64, pinned manifest, 8 threads).

case / quantity                               develop 0e68a0553   b3f412f35        diff
diiid_n1_riccati   (12 changed, 5 unchanged)
  delta prime (BVP diagonal)                  [5 elem]            [5 elem]         14.68%
  delta prime (raw side-major)                [10 elem]           [10 elem]        3.10%
  total energy Re(et[1])                      8.037194e-01        7.988495e-01     0.61%
  q0 / q95 / beta_n                                                                4e-6 / 4e-6 / 0.08%
diiid_n1           (42 changed, 11 unchanged)
  total energy Re(et[1])                      8.012318e-01        7.961432e-01     0.64%
  plasma energy Re(ep[1])                     -1.348486e+00       -1.361062e+00    0.93%
  island half-widths                          [5 elem]            [5 elem]         0.50%
  NTV torque FGAR [N.m]                       5.296746e-01        5.446895e-01     2.83%
  PE toroidal torque                          5.087465e-02        -5.063447e-02    sign flip (see notes)
diiid_slayer_n1    (12 changed, 5 unchanged)
  SLAYER gamma_Hz [2/1,3/1,4/1]               [3 elem]            [3 elem]         6.04%
  SLAYER omega_Hz [2/1,3/1,4/1]               [3 elem]            [3 elem]         0.12%
gal_resistive_diiid (7 changed, 3 unchanged)
  gal PEST3 Delta diagonal                    [4 elem]            [4 elem]         0.40%
  ||gal Delta_coil block||                    2.453508e+02        2.478311e+02     1.01%
diiid_error_field  (7 changed, 1 unchanged)
  EFC whole-field NTV torque per kAt^2        [1 elem]            [1 elem]         2.83%
  EFC rotation-scan shifts [rad/s]            [9 elem]            [9 elem]         50.00%  (see notes)
  EFC residual torque against rotation        [9 elem]            [9 elem]         65.79%  (see notes)
diiid_multi_n      (14 changed, 5 unchanged)   largest change 0.22%
efit_fixedbdy_separatrix (3 changed)           q0, q95, beta_n at 1e-9
gal_resistive_pe   8 quantities N/A on both refs
solovev_n1         22 unchanged

Unit tests on the same commit: equilibrium 305/305, IMAS 32/32, rerun-from-h5 40/40.

Validation beyond the harness

  • Old route is exact: profile_source = "values" gives develop's Δ′(2/1) to every digit on two files.
  • Same equilibrium state, two g-files (five pairs, inputs agree to 1e-5; see Equilibrium - g-file reader differentiates FPOL and PRES and discards FFPRIM and PPRIME, putting rounding noise on Δ′ #504): the gap in Δ′(2/1) was 3.7 rms on develop. With this change on a fixed grid (ldp, mpsi = 256) it is 0.027 rms.
  • Fortran cross-check: on the in-repo TkMkr_D3Dlike_Hmode.geqdsk with F rebuilt from FF′, Julia and Fortran STRIDE give Δ′(2/1) of 7.27 and 7.28. Fortran GPEC has the same defect and will need the same fix.
  • Grad-Shafranov consistency: on the files' own (R,Z) grids, ψ(R,Z) follows FF′/p′ and not the node-scale structure of the differentiated F (regression coefficient 0.07 ± 0.02 over 24 files, where 0 means FF′/p′ and 1 means the differentiated profiles).

Notes for reviewers

⚠️ Requires third-party human review before merging. Do not merge without an approving review.


🤖 Generated with Claude Code

…rom the tabulated FF′ and p′

The g-file and IMAS readers took F and P from the tabulated profiles and left
their derivatives to the cubic spline. F varies by only a few percent across the
plasma, so rounding in the table (single precision in TokaMaker and classic EFIT
output) leaves F′ nearly right in value but wrong in slope at the node scale, and
Δ′ responds to that slope.

`profile_source = "derivatives"` (the new default) integrates the file's FF′ and
p′ inward from the boundary values of F and P, always for both profiles together.
`profile_source = "values"` keeps the previous behaviour, and the reader falls
back to it with a warning when the derivative arrays are absent or unusable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@d-burg d-burg self-assigned this Oct 6, 2026
@github-actions github-actions Bot added bugfix Something was wrong and now is not changed-results Results move or an interface breaks - read before upgrading labels Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

This pull request is missing a reviewer.

If you are not ready to name them, mark this pull request as a draft.
docs/development/contributors.md suggests lead developers to ask.
Merging is not blocked here, but no pull request may be merged without human review.

1 similar comment
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

This pull request is missing a reviewer.

If you are not ready to name them, mark this pull request as a draft.
docs/development/contributors.md suggests lead developers to ask.
Merging is not blocked here, but no pull request may be merged without human review.

@d-burg

d-burg commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

Parking this PR. OpenFUSIONToolkit/OpenFUSIONToolkit#358 fixes the noise at the writer, and with double-precision g-files develop gives clean Δ′ without this change. Integrating FF′ and p′ also moves surfaces near pedestal kinks by more than the noise it removes (cubic, trapezoid and Akima rules disagree there by several units). Results and the open points are in #504. The one CI failure is the Δ′(2/1) pin of the in-repo example, which this change moves by design; it is left as is while the PR is parked.

@matt-pharr

Copy link
Copy Markdown
Collaborator

@d-burg do we want to close this and just have a loud warning if a user tries to use a single-precision equilibrium with an integrator that calculates Delta'? @StuartBenjamin has a PR stacked on this one, I am going to unstack it and rebase on develop because the i-file compatibility improvement is useful.

@d-burg

d-burg commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

@matt-pharr Agreed, let’s close this. The Δ′ noise it targets comes from the input file, not from GPEC. OFT’s double-precision g-file writer (OFT PR 358) removes it for every new file with GPEC unchanged. Rebuilding F and P from FF′ and p′ also moves the edge surfaces through its quadrature, which is more change than the problem warrants.

I’d warn when a geqdsk's FPOL looks single precision (every value round trips through Float32 exactly) and the integrator computes Δ′: probably should say that Δ′ can carry noise of order +/- 2-3 at 257 radial nodes, and suggest the user regenerates the file in double precision. I can open that as a small PR unless you'd like to?

@matt-pharr

Copy link
Copy Markdown
Collaborator

@d-burg good idea, you are free to open that PR and add me as a reviewer

@logan-nc

Copy link
Copy Markdown
Collaborator

Per the thread this is agreed closed: the Δ′ noise comes from single-precision g-files, which OFT's double-precision writer fixes, and #596 (draft) carries the loud warning Matt asked for. Suggest closing and dropping the milestone so the 2.0 list reflects what is still moving.

@d-burg

d-burg commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this, as @logan-nc suggested, and removing it from the 2.0 milestone. #596 takes the alternative agreed here: GPEC's quadrature stays as it is, and a run on a g-file with single-precision FPOL warns, since that is where the Δ′ noise comes from. New files written by OFT's double-precision writer don't have the problem. The quadrature change here also moved the edge surfaces, which is the reason not to merge it as it stands.

@d-burg d-burg removed this from the GPEC v2.0.0 milestone Oct 27, 2026
@d-burg d-burg closed this Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something was wrong and now is not changed-results Results move or an interface breaks - read before upgrading

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Equilibrium - g-file reader differentiates FPOL and PRES and discards FFPRIM and PPRIME, putting rounding noise on Δ′

3 participants