Conversation
…ixed typos and updated compile instructions in DEFAULTS.inc
…al surface present
|
@StuartBenjamin you are one of the few who knows the guts of RMATCH... can you take a very brief look at these comments (in f27f665) and let me know if there should be any changes/if you know what some of the 'unknown's are? |
StuartBenjamin
left a comment
There was a problem hiding this comment.
Remembered a few of the extra terms!
… rmatch.ins, implemented Stuart's improvements.
…ch.in. Now, if not doing a scan and using rpec_flag, eigenvalue will be calculated based on the input rotation at a rational surface.
…rming LAR equilibria
…ies to be written to globalsol.bin for gpec
…eal/kin dcon eigenfunctions so the parallels are clear and readable
…ies to be read by rmatch
…into gpec as a diagnositc; fixed a breaking bug where match.f would dump too many psi points into galsol.bin, leading to the inner layer looking broken and being incorrect; Added xdraw files for RPEC solutions; improved a couple of rmatch numerical factors, including scaling the number of inner layer grid points by its width to keep the physical resolution consistent, as well as moving the resonant cell widths outside of this scaling to keep their widths consistent regardless of inner layer width.
|
@logan-nc The problems wound up being that the way rdcon was regridding was cutting off at a different psilim than I expected, resulting in the poorly conditioned solution set that we know occurs at some psi cutoff values. I haven't run it yet, but I think I should be able to get it to work nicely with Stuart's regridding contributions. Hopefully I'm not dooming it by speaking too soon! |
|
@claude Please briefly summarize this PR. |
|
Claude finished @matt-pharr's task in 1m 9s —— View job Summary of PR #210: Resistive Singular CouplingThis PR enhances GPEC's resistive MHD analysis capabilities by implementing scientifically robust singular coupling between RDCON (resistive stability) and GPEC (perturbed equilibrium) codes. The work enables accurate 3D field penetration calculations at rational surfaces where resistive effects dominate. Key Improvements1. Inner Layer Resolution Enhancement
2. Resistive Layer Coupling
3. Field Penetration Physics
4. Singular Spot Positioning
5. Code Infrastructure
6. Numerical Robustness
Files Changed100 files modified across RDCON, RMATCH, GPEC, EQUIL, and example input files (+2613/-1894 lines) StatusWork-in-progress with ongoing refinements to resistive layer width logic and interpolation scaling. Recent commits focused on finalizing parameters for resistive metrics publication. --- | Branch: RDCON_singcoup |
|
@matt-pharr can you confirm the truncation issue is fixed and this is ready for full review? Last message from you to me ended on a cliff hanger, "Hopefully I'm not dooming it by speaking too soon!" |
|
@claude review this PR |
|
Claude finished @logan-nc's task in 6m 29s —— View job PR #210 Review: Resistive Singular CouplingTasks:
This is a substantial WIP PR implementing resistive singular coupling. The physics approach (using RMATCH eigenvalues and layer widths to set per-surface sing_spots, reading Bugs1. Wrong variable in spline deallocation — IF(.NOT. sq_in%allocated)THEN
CALL spline_alloc(sq_in,ma,4)
ELSE
CALL spline_dealloc(sq) ! BUG: should be sq_in
CALL spline_alloc(sq_in,ma,4)
ENDIF
2. Duplicate WRITE statement — DO ising=1,msing
WRITE(out_unit,'(1x,f6.3,17(es17.8e3))') ! line 1930: blank write
WRITE(out_unit,'(1x,f6.3,17(es17.8e3))') ! line 1931: actual write
$ singtype(ising)%q,singtype(ising)%psifac,
$ spots(ising), ...Line 1930 is a spurious duplicate with no arguments — it writes a blank/garbage line before the actual data line. The first WRITE should be removed. Fix this → 3. Orphaned WRITE statement — WRITE(out_unit,'(1x,a13,a8,1x,a12,I2)') ! format but no arguments
WRITE(out_unit,'(1x,a12,1x,I4)')"msing =",msingAfter removing 4. SUBROUTINE equil_loadnamelists(...)
IF (out_ahg2msc) THEN ! checked BEFORE equil.in is read
WRITE(*,*) "WARNING: ..."
ENDIF
CALL ascii_open(in_unit,"equil.in","OLD")
READ(UNIT=in_unit,NML=equil_output) ! out_ahg2msc read hereThe Memory Leaks5. Both are allocated in the 6. Allocated alongside 7. Six Design/WIP Concerns8. When 9. Appears in the 10. Commented-out boundary guards ( lpsi=respsi-spots(ising)/(nn*ABS(singtype(ising)%q1))
! IF (lpsi < psilow) lpsi=psilow ! <-- commented outThese guards were presumably commented out for a reason (debugging?), but leaving them out risks out-of-bounds spline evaluations near the plasma boundary. Add a note explaining why they're disabled or re-enable them. 11. Deprecated The old implementation is left behind as commented-out code, and the function body now only calls 12. Duplicate "subprogram 2" numbering ( Both Positive Changes
SummaryThe PR has good physics motivation and several important bug fixes. Before merging, the critical |
- Restore qlim=MIN(qmax,qhigh): the qhigh namelist parameter was silently ignored after being dropped from the MIN() call, causing user-set high-q domain limits to have no effect. - Restore IF(qlow > qmin) condition in sing_min: the condition was replaced with IF(.FALSE.), making the psilow Newton iteration dead code and leaving psilow at its default value whenever qlow is active. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In the sq_in re-allocation branch, spline_dealloc was incorrectly called on the global equilibrium spline sq instead of the local input spline sq_in, corrupting the global spline state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The deprecation warning and vac_memory assignment were evaluated before equil.in was read, so the check always saw the default value (.FALSE.) and never fired for users who set out_ahg2msc=.TRUE. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
y_out is correctly declared as an automatic array sized by enstep. The ALLOCATE/DEALLOCATE added in this PR were incomplete (used undefined nstep instead of enstep) and unnecessary. Reverted to the develop branch pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
localcoup_out was allocated but never populated with data, causing uninitialized memory to be written to the cl_id NetCDF variable. localcoup_out was a renamed duplicate of localcoup_out_vecs, which IS properly filled. Remove localcoup_out entirely and read from localcoup_out_vecs directly. Also fix memory leaks: add matms, localcoup_out_vecs, and matmo to their respective DEALLOCATE statements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two spurious WRITE calls were left after a prior refactor: - A header WRITE with format '(1x,a13,a8,1x,a12,I2)' but no data - A duplicate data WRITE in the ising loop with no I/O list, producing a blank line before each valid data line Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bwpmns_rmatch was allocated alongside bwpmns but missing from the DEALLOCATE statement, causing a memory leak on every call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Six cellinfos component arrays (icell, iintvl, etypes, etypes_int, x1, x2) were allocated in gal_alloc but never freed in gal_dealloc, causing a memory leak on every resistive solve. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
deltac.f uses msing_bounds from msing_mod, but msing.o was not listed as a prerequisite of deltac.o. With wildcard source discovery, alphabetical order caused deltac.f to compile before msing.f, failing with 'Symbol msing_bounds not found in module'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Independent Review & Bug FixesI performed an independent review of this PR (cross-checking the Claude review from Mar 11) and found several confirmed bugs plus two additional memory leaks not previously identified. All fixes have been pushed as individual commits. Bug Fixes
Notes on Previous ReviewTwo items from the Mar 11 Claude review were not reproduced:
|
logan-nc
left a comment
There was a problem hiding this comment.
@matt-pharr I am happy if you are. Please check my bug fix commits before merging. In particular, I restored the qlow and qhigh functionality under the assumption it was removed as part of development checks and not intended to stay. But if there is something broken about these in RDCON now, then please add printed warnings and/or errors if/when users try to use them.
|
@matt-pharr you have a green check ✅ from the github checks and an approved review from me. Ready to hit merge?! |




Changes related to adding scientifically robust singular coupling to the RDCON -> GPEC workflow. WIP.