Skip to content

Fix AEQDSK RSURF/ZSURF/AMINOR unit conversion: data is in metres, not cm - #442

Merged
torrinba merged 2 commits into
masterfrom
fix/aeqdsk-rsurf-aminor-units
Oct 9, 2026
Merged

torrinba merged 2 commits into
masterfrom
fix/aeqdsk-rsurf-aminor-units

Conversation

@smithsp

@smithsp smithsp commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Problem

boundary_separatrix.geometric_axis.r, geometric_axis.z, and minor_radius
return values ~100× too small on DIII-D.

Field Observed Expected
geometric_axis.r ~0.017 m ~1.7 m
minor_radius ~0.006 m ~0.6 m

Root cause

The _efit.json mappings apply a /100. conversion assuming CGS (cm), but
DIII-D MDSplus stores AEQDSK.RSURF, AEQDSK.ZSURF, and AEQDSK.AMINOR
in metres with units explicitly labelled in the tree.

Verified on shots 163303 :

  • AEQDSK.AMINOR ∈ [0.57, 0.63] m (units: metres)
  • AEQDSK.RSURF ∈ [1.63, 1.70] m (units: metres)

Fix

Remove the /100. from three TDI expressions in _efit.json.

Related

🤖 Generated with Claude Code

DIII-D EFIT stores AEQDSK.RSURF, AEQDSK.ZSURF, and AEQDSK.AMINOR with
units of metres in the MDSplus tree. The previous /100. conversions assumed
centimetres, returning values ~100x too small:

  boundary_separatrix.geometric_axis.r  → ~0.017 m  (expected ~1.7 m)
  boundary_separatrix.minor_radius      → ~0.006 m  (expected ~0.6 m)

Verified on DIII-D shots 163303 and 200000: AEQDSK.AMINOR ∈ [0.57, 0.63] m
and AEQDSK.RSURF ∈ [1.63, 1.70] m with units explicitly labelled "metres"
in the MDSplus tree. ZSURF follows the same EFIT unit convention.

Note: AEQDSK.SEPLIM, GAPIN, GAPOUT, GAPTOP, and GAPBOT also carry a /100.
conversion and should be checked against the tree units before a follow-up fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@torrinba torrinba left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I've confirmed that the MDSplus units for these are meters, which differs from the EFIT a-files and is likely the source of the error.

@torrinba
torrinba merged commit 777173c into master Oct 9, 2026
15 of 16 checks passed
@torrinba
torrinba deleted the fix/aeqdsk-rsurf-aminor-units branch October 9, 2026 23:59
@AreWeDreaming

Copy link
Copy Markdown
Collaborator

@torrinba So there are units stored for this one? I wonder if that could be part of the regression tests.

@torrinba

torrinba commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

@torrinba So there are units stored for this one? I wonder if that could be part of the regression tests.

That's correct. Most of the MDSplus equilibrium entries have units and all the ones I've checked are valid. We could add a test for the order of magnitude of some variables like RSURF and AMINOR, but others like ZSURF wouldn't be so easy (the mean or median could be close to zero).

Are you suggesting this should be an omas or imas_composer regression test? This discussion should probably be continued in #446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants