Implement Bernstein's breakdown for the perturbed potential energy

[parkjk]

…the develop branch.

[logan-nc]

What is this file?

[logan-nc]

Please document things as you add features and flags

[logan-nc]

Please document

[logan-nc]

Are these notes available to everyone? Can you add them to the repo or cite a publication?

[logan-nc]

@parkjk This branch appears to have completed a clear and finite feature addition. I recommend it be cleaned up no and merged into the develop branch for everyone to enjoy. Not doing so risks sliding into a never ending state of adding features but never finishing them rigorously - which could lead to a permanent split of the SNU code from the rest of the GPEC source.

[logan-nc]

@jaesun57, is this branch ready to merge? If not, what is needed and what it the expected timeline? I'd like to not let it live too much longer, as the mission will creep. PR's are best if finite so they do not diverge overmuch from the develop branch.

[jaesun57]

@logan-nc I'm mainly working on the recon branch right now, and it will take about two more weeks to finish. In the meantime, I'll make sure to keep both recon and snu_offshoot branches updated with the develop branch so they don't diverge too much. As for snu_offshoot, since I didn't create it myself, I'll need to check with @parkjk to see what else needs to be done there.

[logan-nc]

Thanks for the response @jaesun57.

What is the recon branch? Please open a Pull Request for it and make a clear statement of the intent as the opening comment. You can mark the PR and "draft" and that tells people not to bother reviewing the code yet, but it is important to signal what you are doing as it can impact what others are doing (we don't want multiple people accidentally doing the same thing or conflicting things). It looks like you branched off of snu_offshot - is your intent to merge back into snu_offshoot or directly into develop?

Similarly, it would be great if you could coordinate with the SNU team to edit the opening comment of this branch with a description of the finite objective, which will clarify whether it is ready to review and merge or not. We don't want infinitely diverging parallel develop branches.

Thanks!

[logan-nc]

Here is AI's summary of the branch:

Branch objective (finite)

Add a selectable solution-component parameter sol_num to MATCH, so the
ideal-MHD reconstruction can target any DCON ideal eigenfunction (column of
Wt) rather than only the most-unstable one.

What it does: sol_num (namelist match.in, default 1) selects which
column of Wt is used as the edge boundary condition in ideal_build, and
writes the result to solutions.bin (for sol_num=1) or solutions{sol_num}.bin
otherwise. With the default it reproduces existing behavior exactly, so the
change is backward-compatible.

Definition of done: It is ready to merge once
the diff is reduced to just that feature (plus its input template) and the
1 = least stable vs. 1 = most unstable wording is made consistent (see Q2).
Further SNU work belongs on its own purpose-scoped branch/PR, not here.

---

Questions needing clarification (@parkjk / @jaesun57)

1. Is sol_num the intended objective? It is the only substantive change
   surviving in the net diff (develop...snu_offshoot). The heavily-developed
   bernstein.f module (shear / geometry / curvature / metric calculations) was
   built up across many commits and then entirely deleted in 5ce5f7bb.

2. sol_num index description The new input/match.in comment says
   "1 is least stable" (good), but the match/match.f header comment says
   column 1 is the "most unstable ideal solution" (should be changed to match "least stable" terminology).

3. Leftover cruft The net diff also carries a few changes that
   look like leftovers from the abandoned experiments, unrelated to sol_num.
   Confirm whether these should be dropped before merge:

   • input/dcon.in: kin_flag t → f
   • dcon/dcon.F: comment xi' → xi (looks like a typo regression)
   • equil/equil_out.f: whitespace-only changes
   • docs/nametag: new branch-marker file

[parkjk]

Your points are good and well taken, though this touches on a big picture conversation we should have separately (a bit too much to get into here). Anyway, @jaesun57 has already committed to opening a PR within the next two weeks, so I'd ask for a little patience on that front. Once it's up, she'll follow the practice closely.

[jaesun57]

@logan-nc ,All addressed and pushed to snu_offshoot(now at ce90394)!

Q1 – sol_num: Yes, this is the objective. sol_num (match.in, default 1)
selects which DCON ideal eigenfunction (column of Wt) MATCH reconstructs —
so we can target the 2nd / 3rd / … solution instead of only the most-unstable
one. Default 1 reproduces existing behavior exactly. The old dcon/bernstein.f
(shear/geometry experiment) was an abandoned line, deliberately removed in
5ce5f7b — not part of the objective.

Q2 – index wording (fixed in 311922c): Standardized on "most unstable" in
both match.in and match.f:54, since sol_num=1 is the lowest-δW / most
-unstable column. (Went with "most unstable" rather than "least stable" as it's
the unambiguous description of column 1; happy to flip if you prefer the other
term.) Also fixed a "calculates" → "calculated" typo.

Q3 – leftovers (reverted to develop in 08ba8bd):

• input/dcon.in kin_flag f → t — restored to develop.
• dcon/dcon.F xi → xi' — restored the dropped prime (typo regression).
• equil/equil_out.f — whitespace reverted to develop.
• docs/nametag — already removed in 7ffde74; nothing left to drop.

All four leftover diffs vs develop are now zero.

That said, I still have a few other in-progress changes to wrap up, so I'll do
the final consolidation pass once those are all done.

[logan-nc]

Thanks @jaesun57 !

@parkjk this looks like a clean feature add rn. Our options are,

1. Update the PR name and merge this branch to develop → SNU continues to make new feature branches off develop as desired (e.g. recon → develop).
2. Branch a match_sol_num branch off on snu_offshoot now and merge that into develop as the finite feature, leave the snu_offshoot as an open ended sandbox for now and mark this PR as draft to it doesn't pop up when I am checking for stale PRs that need to get merged/worked-on.

Either way, I'm happy to chat about the big picture separately via slack, zoom, etc. or in person Aug 10.

[parkjk]

Option 1 sounds good. @jaesun57: Would this be a representing PR name at the moment?

"Implement Bernstein's breakdown for the perturbed potential energy"

or would you reserve it? Let me know the PR name otherwise and tell me when. Then I will change the name, merge pull, and let you go from there.

[jaesun57]

Would be a perfect name @parkjk and would be great to just close the pr now. And I'll keep merging regularly from now on to prevent the branch from drifting too far from develop. Thanks for the feedback, @logan-nc!