Skip to content

Implement Bernstein's breakdown for the perturbed potential energy#213

Draft
parkjk wants to merge 59 commits into
developfrom
snu_offshoot
Draft

Implement Bernstein's breakdown for the perturbed potential energy#213
parkjk wants to merge 59 commits into
developfrom
snu_offshoot

Conversation

@parkjk

@parkjk parkjk commented Jun 20, 2025

Copy link
Copy Markdown
Contributor

…the develop branch.

@parkjk parkjk self-assigned this Jun 20, 2025
Comment thread docs/nametag Outdated
@@ -0,0 +1 @@
snu_offshoot No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is this file?

Comment thread input/dcon.in Outdated
bin_bal2=f ! Binary output for bal_flag functions

netcdf_out=t ! Replicate ascii dcon.out information in a netcdf file
bernstien_flag=t

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please document things as you add features and flags

Comment thread input/match.in Outdated
ideal_flag=t
contour_flag=t

sol_num=1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please document

Comment thread dcon/bernstein.f Outdated

c-----------------------------------------------------------------------
c subprogram 2. bernstein_shear.
c computes local magnetic shear S(ψ, θ) from Eq. (29) of GPEC Notes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@logan-nc

Copy link
Copy Markdown
Contributor

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

jaesun57 and others added 14 commits May 26, 2026 12:55
Add recon_flag3 path computing the |C|^2 decomposition of delta-W_p:
- gpeq_recon3: A=|Q|^2/mu0, B=2Re(V*.Q)/mu0 (V computed directly from
  the equilibrium current, not via C-Q), Cpar=K2 xi_n^2, Iperp term,
  plus optional per-theta surf_extra(0:mthsurf,22) field pack.
- gpout_recon3: 30-column surface output (gated by recon_out) and a
  C-free dW_p = 1/2(A+B+Iperp-dst1-dst3) via gpeq_dst (dst2 cancels).
- Unify recon1/2/3 terminal+log output (dW_p, dW_p-normalize, GPEC ep,
  DCON ep); fix recon1 DCON ep normalization to use dcon_fac.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add gpec_recon_terms_solX.out (per-point C-form energy densities:
c2_den, c2_psi/theta/zeta_den, dst1/2/3_den) from gpout_recon, the
recon1 analog of recon3_terms for 2 dW_p = C2/mu0 - dst1 - dst2 - dst3.

gpeq_epf / gpeq_dst gain optional per-theta density out-args; their
theta loops now run 0..mthsurf with an itheta<mthsurf integration
guard so the flux-surface integrals are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
main.tex section 4.1: add "Verification: C as an effective field" passage
(proof that (curl C).grad psi = 0; Eulerian vs Lagrangian frames; effective
current j_eff = curl C / mu0 by analogy with Ampere's law) and Figure 1
(C_eulerian_lagrangian_n1.png, force-added since *.png is gitignored). Also
reframe the V-component block as the correction term, drop section 6.1
(dcon_fac moved to the section-6 intro), and rename sections 4.1/4.2.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add "!docs/tex/recon/C_eulerian_lagrangian_n1.png" so the figure included by
docs/tex/recon/main.tex is tracked without `git add -f`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- A.11: replace the proportionality with an exact equality; expose the
  1/(B^2|grad psi|^2) prefactor and that the numerator vanishes by force
  balance mu0 j x B = mu0 p' grad psi.
- A.2 intro: correct which V-products enter H4. Only Y*.V, U*.V, and
  |V|^2 (with the -mu0 K scalar) form H4; X*.V feeds the C block and
  Z*.V = mu0 p' xi_psi'^* xi_psi feeds the E block.
- Remove redundant "Input Variables and Parameters" table.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Appendix A.1 presented V only through its components A_2,A_3, which read
backwards (the components are the *result* of expanding the physical field).
Fix the framing:

- State up front that V is the edge current rotated into the surface,
  V = (xi_psi/|grad psi|^2) mu0 j x grad psi -- a concrete (psi,theta)-grid
  field -- and that A_2,A_3 are just its Jacobian-weighted contravariant
  components.
- Add the scalar-triple-product rule (A.4):
  W . V = xi_psi (a mu0 j^zeta - b mu0 j^theta) for W = a v_2 + b v_3,
  i.e. the cofactor identity in coordinate-free form.
- Replace the repeated per-product cofactor derivations for X*.V, Z*.V, U*.V
  with one-line references to (A.4); drop the now-redundant cofactor
  intermediate equation.

No change to results; presentation only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gpout_recon: dump gpec_recon_qvmn_sol*.out = covariant Q_i (bare delta-B = bv*_mn, captured right after gpeq_epf before gpeq_K/gpeq_dst). Effective field is cvmn = C_i = Q_i + V_i; cwmn is the contravariant J*C^i.

The quantitative QvsC figure built its (curl Q).grad(psi) panel from cwmn (contravariant) fed into the covariant curl operator -> spurious RMS 3.8e-2. Rebuilt from qvmn (covariant Q_i): RMS 2.1e-3 (Q pierces) vs 2.1e-12 (C tangent), matching the C-verify diagnostic; both panels now use covariant components as the caption states. Q-vs-C table + caption updated; figure swapped C_eulerian_lagrangian_n1.png -> QvsC_quant_n1.png (+ gitignore exception).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The xi_flag/xifile namelist entries + declarations were WIP and never wired up (idcon_inject_wt was never called). Removed from gpec.f, gpglobal.F, and input/gpec.in to avoid future conflicts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@logan-nc

Copy link
Copy Markdown
Contributor

@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

Copy link
Copy Markdown
Collaborator

@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

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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

Bring 107 develop commits (RDCON/RMATCH resistive work, harvest removal,
positive-q sign conventions, install dependency regeneration) into the
recon energy-decomposition branch.

Conflict resolutions:
- gpglobal.F / gpec.f: keep both sides (recon covariant arrays, gpout_*
  types, recon_* flags + develop bwp_mn_rmatch, bpsi, use_res_spot,
  singthresh_slayer_inpr_prof)
- gpeq.f: keep recon's new subroutines (gpeq_c .. gpeq_recon3); take
  recon-side subprogram numbering
- out_ahg2msc / vac_memory: adopt develop's refactor (moved gpec -> equil
  module); dropped recon's stale gpec-side copies
- solovev gpec.in: take develop's example settings
- CLAUDE.md: keep deleted (recon removed it from VCS; local copy retained)

Builds clean: make gpec succeeds, gpec executable produced.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@parkjk

parkjk commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

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.

actions-user and others added 4 commits June 25, 2026 03:06
Match input/match.in comment to match/match.f; sol_num=1 selects the most unstable DCON ideal eigenfunction. Addresses snu_offshoot review Q2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop unintended net-diff leftovers from abandoned bernstein experiments: input/dcon.in kin_flag t (was f), dcon/dcon.F xi' comment typo, equil/equil_out.f whitespace. Addresses snu_offshoot review Q3.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jaesun57

Copy link
Copy Markdown
Collaborator

@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 xixi' — 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

Copy link
Copy Markdown
Contributor

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. recondevelop).
  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

parkjk commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

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

jaesun57 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

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!

@parkjk parkjk changed the title A new branch for snu students to incorporate local developments into … Implement Bernstein's breakdown for the perturbed potential energy Jun 25, 2026
@parkjk parkjk marked this pull request as draft June 25, 2026 14:42
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.

4 participants