Skip to content

Equilibrium - BUGFIX - Select the rectangular profiles_2d by grid_type in read_imas - #476

Merged
jhalpern30 merged 2 commits into
developfrom
fix/read-imas-two-bugs
Oct 7, 2026
Merged

jhalpern30 merged 2 commits into
developfrom
fix/read-imas-two-bugs

Conversation

@calin1989

@calin1989 calin1989 commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Release note

  • Audience: users
  • Numerical impact: none (harness @ 4681a3c)
  • Migration: none

GPEC can now read IMAS equilibria that store more than one 2D map, such as those from TEQUILA. read_imas previously always took the first profiles_2d entry, which for TEQUILA is not the rectangular R,Z grid.

Regression report

Regression Report: diiid_n1
=================================================================================================
Ref 1: 6553fe205  @ 6553fe205 (2026-10-07)
       env: julia 1.12.6, arm64-apple-darwin24.0.0, manifest ece8f5d1 (pinned), 4 threads/4 BLAS
Ref 2: 4681a3c06  @ 4681a3c06 (2026-10-07)
       env: julia 1.12.6, arm64-apple-darwin24.0.0, manifest ece8f5d1 (pinned), 4 threads/4 BLAS
-------------------------------------------------------------------------------------------------
Quantity                                      6553fe205        4681a3c06        Diff       Status
-------------------------------------------------------------------------------------------------
total energy Re(et[1])                        8.013943e-01     8.013943e-01     0.0e+00    OK
total energy Im(et[1])                        1.233500e-04     1.233500e-04     0.0e+00    OK
plasma energy Re(ep[1])                       -1.348114e+00    -1.348114e+00    0.0e+00    OK
vacuum energy Re(ev[1])                       2.149508e+00     2.149508e+00     0.0e+00    OK
vacuum matrix min eigenvalue                  1.873975e-01     1.873975e-01     0.0e+00    OK
plasma energy (all)                           [35 elem]        [35 elem]        0.0e+00    OK
vacuum energy (all)                           [35 elem]        [35 elem]        0.0e+00    OK
total energy (all)                            [35 elem]        [35 elem]        0.0e+00    OK
ODE steps (saved)                             1449             1449             0.0e+00    OK
ODE steps (total)                             1444             1444             0.0e+00    OK
q0                                            1.204212e+00     1.204212e+00     0.0e+00    OK
q95                                           4.781723e+00     4.781723e+00     0.0e+00    OK
beta_t                                        1.327024e-02     1.327024e-02     0.0e+00    OK
beta_n                                        1.372511e+00     1.372511e+00     0.0e+00    OK
internal inductance li1                       8.842230e-01     8.842230e-01     0.0e+00    OK
internal inductance li2                       7.080727e-01     7.080727e-01     0.0e+00    OK
internal inductance li3                       7.304309e-01     7.304309e-01     0.0e+00    OK
poloidal beta betap1                          6.680738e-01     6.680738e-01     0.0e+00    OK
poloidal beta betap2                          5.349836e-01     5.349836e-01     0.0e+00    OK
poloidal beta betap3                          5.518763e-01     5.518763e-01     0.0e+00    OK
# singular surfaces                           5                5                0.0e+00    OK
singular psi locations                        [5 elem]         [5 elem]         0.0e+00    OK
singular q values                             [5 elem]         [5 elem]         0.0e+00    OK
current beta betaj                            4.236479e-01     4.236479e-01     0.0e+00    OK
plasma volume                                 1.829472e+01     1.829472e+01     0.0e+00    OK
plasma current                                1.152130e+00     1.152130e+00     0.0e+00    OK
mpert                                         35               35               0.0e+00    OK
npert                                         1                1                0.0e+00    OK
toroidal field bt0                            2.006573e+00     2.006573e+00     0.0e+00    OK
wall field bwall                              3.880145e-01     3.880145e-01     0.0e+00    OK
aspect ratio                                  2.845746e+00     2.845746e+00     0.0e+00    OK
elongation kappa                              1.708322e+00     1.708322e+00     0.0e+00    OK
q profile (checksum)                          ed7c21fd61df...  ed7c21fd61df...  identical  OK
pressure profile (checksum)                   e15550827bf1...  e15550827bf1...  identical  OK
Mercier D_I profile (checksum)                5a6fcb1c3a97...  5a6fcb1c3a97...  identical  OK
resistive interchange D_R profile (checksum)  6284a4c9a75a...  6284a4c9a75a...  identical  OK
ballooning Delta' profile (checksum)          fe06936f5840...  fe06936f5840...  identical  OK
island half-widths                            [5 elem]         [5 elem]         0.0e+00    OK
Chirikov parameter                            [5 elem]         [5 elem]         0.0e+00    OK
||resonant area-weighted field||              5.207668e-04     5.207668e-04     0.0e+00    OK
dominant-coupling singular values             [3 elem]         [3 elem]         0.0e+00    OK
|forcing overlap with dominant mode|          1.422984e-04     1.422984e-04     0.0e+00    OK
|delta_nominal| of coil set 1                 N/A              N/A              N/A        N/A
||ddelta/d(shift)|| over coil sets            N/A              N/A              N/A        N/A
||ddelta/d(tilt)|| over coil sets             N/A              N/A              N/A        N/A
PE plasma energy                              3.422586e+00     3.422586e+00     0.0e+00    OK
PE vacuum energy                              3.174509e+00     3.174509e+00     0.0e+00    OK
PE surface energy                             5.841099e+00     5.841099e+00     0.0e+00    OK
PE toroidal torque                            -5.062793e-02    -5.062793e-02    0.0e+00    OK
NTV torque FGAR [N·m]                         5.332926e-01     5.332926e-01     0.0e+00    OK
NTV kinetic energy dW FGAR [J]                7.991398e-02     7.991398e-02     0.0e+00    OK
Runtime (s)                                   177.0s           170.4s                      --
||forcing b~|| (root-area-weighted)           4.683328e-04     4.683328e-04     0.0e+00    OK
resonant area-weighted field b^r              [5 elem]         [5 elem]         0.0e+00    OK
=================================================================================================
Summary: 50 unchanged, 3 missing/N/A

Notes for reviewers

read_imas now selects the rectangular profiles_2d entry with IMASdd's findfirst(:rectangular, …), as FUSE does, and stops with an error if there is none. The test mock now sets grid_type.index = 1, as real IMAS writers do.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the bugfix Something was wrong and now is not label Sep 28, 2026
@calin1989 calin1989 self-assigned this Sep 28, 2026
@calin1989
calin1989 force-pushed the fix/read-imas-two-bugs branch 2 times, most recently from 4b95561 to c57b7c8 Compare September 28, 2026 17:20

@jhalpern30 jhalpern30 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.

Nice work! Some requested changes before we get this merged, mainly focusing on comments. I challenge you to not use Claude to make the below changes on this PR :)

Comment thread src/Equilibrium/ReadEquilibrium.jl Outdated
"Ensure the 2D ψ(R,Z) map is stored in dd.equilibrium.")
end
prof2d = eqt.profiles_2d[1]
# Solvers may store several profiles_2d representations of the same equilibrium.

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.

There is no need to write an entire paragraph as a comment here (and above). Think - what is the most useful (if any) pieces of information a human should know about this code? LLMs have a tendency to flood code comments with extraneous information we've been trying to avoid, so its good practice to read through everything you propose to add and write it from a human perspective. My general rule of thumb is if you're adding more lines of comment than code (as is the case in this PR), you need to take a look again.

For example: a good comment here might be "find 2d profiles on the rectangular grid for a general input equilibrium"

Comment thread src/Equilibrium/ReadEquilibrium.jl Outdated
Comment thread src/Equilibrium/ReadEquilibrium.jl Outdated
# returning missing, so a lone 2D map is taken as-is: that case is unambiguous and
# keeps equilibria that never filled grid_type working exactly as before.
grid_index(p) = getproperty(p.grid_type, :index, 0)
prof2d = if length(eqt.profiles_2d) == 1

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.

You say this section is exactly how FUSE locates the grid - can you point me to the relevant source code here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, to what I understand FUSE selects the grid by type and not position. And the source code for this is src/actors/equilibrium/equilibrium_actor.jl lines 164 and 455, fresco_actor.jl line 85 and eggo_actor.jl line 132.( See equilibrium_actor.jl#L164, #L455, fresco_actor.jl#L85 and eggo_actor.jl#L132. So Claude suggested to use grid_type.index == 1 (the same identifier, since 1 => :rectangular in IMASdd) rather than that helper, because findfirst(:rectangular, …) skips entries whose grid_type is unset.

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.

Looking at all of these FUSE functions, it looks like they use the IMASdd helper

eqt2d = findfirst(:rectangular, eqt.profiles_2d)

is there a reason you aren't using this? It seems to be more convenient/clear.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes it was for the mock to not fail, but we can solve this within the tests so switched to findfirst(:rectangular, eqt.profiles_2d). I added one line to the test mock (prof2d.grid_type.index = 1), because the helper skips maps without a grid type and the mock was the only input that didn't set one. Real files (EFIT.jl, FRESCO, CHEASE, TEQUILA) all set it.

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.

Great, thanks!

Comment thread src/Equilibrium/ReadEquilibrium.jl Outdated
@calin1989
calin1989 force-pushed the fix/read-imas-two-bugs branch from 60b8438 to bb3340e Compare October 3, 2026 14:07
@calin1989 calin1989 changed the title Equilibrium - BUGFIX - Select the rectangular profiles_2d and normalize the psi grid in read_imas Equilibrium - BUGFIX - Select the rectangular profiles_2d by grid_type in read_imas Oct 3, 2026
@calin1989
calin1989 requested a review from jhalpern30 October 3, 2026 15:05
@calin1989
calin1989 force-pushed the fix/read-imas-two-bugs branch from bb3340e to 67b48ea Compare October 6, 2026 17:08

@jhalpern30 jhalpern30 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.

Everything looks good now, thanks @calin1989! I will merge this in once tests finish

@jhalpern30
jhalpern30 merged commit e8f9db0 into develop Oct 7, 2026
12 of 13 checks passed
@jhalpern30
jhalpern30 deleted the fix/read-imas-two-bugs branch October 7, 2026 18:53
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants