Repository navigation
fix(trx): the ten per-channel power integrals of TRGLOB convert W to MW (#238) - #270
HengyuLi-Ozaki-lab wants to merge 2 commits into
Conversation
…MW (k-yoshimi#238) TRGLOB's legacy power integrals divide by 1.D6 (POHT, PEXT, PRBT, PRCT, PRLT, PRSUMT, PCXT, PIET), and the totals they enter are in MW (PINT = POHT + PNB_TOT + PRF_TOT + PNF_TOT + PEXST; trprf.f90 sets PIC_TOT/PLH_TOT/PEC_TOT from the input powers in MW; trhelp.f90 documents PNB_TOT in MW). The ten per-channel integrals did not: PIC_NSNIC, PLH_NSNLH, PEC_NSNEC, PNB_NSNNB, PNBIN_NSNNB, PNBCL_NSNNB, PNF_NSNNF, PNFIN_NSNNF, PNFCL_NSNNF and PNFNN_NNF were in W, so the channel totals PNF_TOT, PNB_TOT and PRF_TOT were 1e6 too large, which corrupted PINT, QF and the confinement diagnostics that take PINT (TAUE1 and TAUE2 directly, TAUE89 and TAUE98 and so H98Y2 through fractional powers). Each of the ten now ends in /1.D6; nothing else changes (the particle-source integrals in between are in 1/s and stay as they are). The solver uses the W/m^3 profiles and is unaffected. Issue k-yoshimi#238; its seven secondary items are left for the module lead, as decided there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
k-yoshimi
left a comment
k-yoshimi
left a comment
There was a problem hiding this comment.
Review through the full cycle (Codex + in-house domain and verifier lenses + docs/workflow review), with trx/tr2 built here at develop, this head and merged with #269 (details on #269).
Verdict: approve. The ten changed statements are exactly the W power integrals; every consumer (PINT, QF, TAUE*, the printed summary, GVT, the [MW] plot labels, trhelp) expects MW; no consumer divides again; the particle-source integrals in the same loops are untouched; every live writer of the ten per-radius arrays stores W/m³. Runs: model_pnf=0 output identical to develop; a 1 keV fusion-on deck changes exactly the TAUE columns, TAUE1/2/89/98, QF, PINT/PNF_TOT lines by 1e6 and leaves WPT unchanged (the solver uses the W/m³ profiles); merged with #269 the 10 keV deck gives PINT 0.5393 MW, QF 1.294, TAUE1 53.13 s, as in your text.
The reviewers' "must fix" items are all pre-existing and independent of this diff (confirmed by reading the pre-diff code): the species-resolved NBI/RF arrays the integrals read have no writer, TRGLOB overwrites the per-beam profiles from them, and so on — your secondary items 1–2 plus a few more. I have added them to #238 for the lead; they are not asked of this PR. One of the suggested remedies (integrating PNB_NNBNR directly) is unsafe as written, because that TRGLOB overwrite is today the only thing that zeroes PNB_NNBNR for model_nnb=0; noted on #238.
LOW / optional: a CHANGELOG.md Fixed entry (one can cover #269 and #270, with a note that PINT, TAUE* and QF change by 1e6 for runs with NBI/RF/fusion on); trgrat.f90:387 TAUE label lacks [s], trgrar.f90:202 WB,WF [MJ] is a density. Pre-existing, not needed here.
Merge needs the lead's confirmation as well, per the plan on #236.
…-yoshimi#270) One Fixed entry for this pull request and k-yoshimi#270, as suggested in the review of k-yoshimi#269. Results of model_pnf>0 runs change; in NBI runs the printed beam collisional powers PBCLE to PBCLA change by 1e6 (now MW). PNB_TOT and PRF_TOT are not fixed by this, since the arrays they integrate are never filled in trx (k-yoshimi#238); they read 0 with model_nnb=1, PNBIN=10 and with PECIN=5 on k-yoshimi#238's deck with fusion off, develop against k-yoshimi#270. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sentences on the ten TRGLOB integrals and on the NBI powers were in k-yoshimi#269's CHANGELOG entry, although this pull request makes that change; the review of k-yoshimi#269 asked to move them here, so that develop's changelog is true whichever of the two merges first. The entry sits after the second [Unreleased] Fixed entry, a boundary no other open pull request inserts at. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vailable (k-yoshimi#269) As asked in the second review of k-yoshimi#269. The CHANGELOG entry covered k-yoshimi#270 too: its sentences on the ten TRGLOB integrals of trx/trrslt.f90 and on the NBI powers describe code this branch does not change, so they move to an entry on k-yoshimi#270, and this one is tagged (k-yoshimi#269; ...). "never set PNFIN_NSNNFNR" is replaced by what the code does: tr_pnf zeroes it on each call and nothing else writes it, so its totals stay 0. Also from that review: the model tables of trx/libnf.f90 and trx/trinit.f90 mark model_pnf=14 as not available (no He5 species can be defined), and the CASE(14) comment names the THe35 product and the arrays it would overrun (tr_pnf's (NSMAX,NNFMAX,NRMAX) arrays). No code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
k-yoshimi
left a comment
k-yoshimi
left a comment
There was a problem hiding this comment.
d2bbfcca adds the changelog entry only (CHANGELOG.md, 10 lines, nothing else in the diff against be320801). Checked against this branch's trx/trrslt.f90: the ten names are the ten statements that gained /1.D6; PINT at :426 sums them with POHT/PEXST; TAUE89 and TAUE98 take PINT**(-0.5) and PINT**(-0.69) at :469-475 and H98Y2=TAUE2/TAUE98 at :476, so the "through fractional powers" clause is right; PBCLE..PBCLA print PNBCL_NS which now comes from the converted integral; the PNB_TOT/PRF_TOT caveat matches trx/trpnb.f90:17 / trx/trprf.f90:23-25. CI 9/9. Merges clean against develop, #269, #231, #267 and #226.
Verdict: approve (renewed on this head). Merge with #269 on the lead's confirmation.
The second PR asked for in the triage on #236: #238's ten
/1.D6sites intrx/trrslt.f90, nothing else, in one commit ondevelop4b5463e4.TRGLOB's legacy power integrals divide by1.D6and the totals they enter are in MW (PINT = POHT + PNB_TOT + PRF_TOT + PNF_TOT + PEXST). The ten per-channel integrals did not; each now ends in/1.D6:PIC_NSNIC,PLH_NSNLH,PEC_NSNECPNB_NSNNB,PNBIN_NSNNB,PNBCL_NSNNBPNF_NSNNF,PNFIN_NSNNF,PNFCL_NSNNFPNFNN_NNFThe particle-source integrals between them are in 1/s and are not touched.
How it was built and run
As for the rate-path PR: macOS arm64, gfortran 15 with the run-time checks of
make.header, GSAF 4.16 and X11,mtxpon MUMPS with MPICH,make -C trx,trx/tr2 < deck, withTRPRNTtype 1 printed at the end. Ondevelopthe 10 keV D-T deck of #238 stops early with negative temperatures (reported time 0.040 s; the rates of #236 are 1e6 too large), so on this branch alone the 1 keV variant of it is used:model_pnf=1developPNF_TOTPINTTAUE1QFWPTQFstays large here because the rates are still 1e6 too large (#236) and the fusion power still accumulates over the steps (#235-2); both are fixed in the rate-path PR. Withmodel_pnf=0this branch prints the same output asdevelopapart from the CPU and elapsed-time lines.With the rate-path PR as well (both on a local branch), the 10 keV deck gives
PNF_TOT0.1109 MW,PINT0.5393 MW,QF1.294,TAUE153.13 s (66.76 s withmodel_pnf=0) andTAUE21.518 s (1.517 s); two consecutive runs give the same output apart from the time lines. The two PRs do not depend on each other and can be merged in either order.These are not the numbers in #238's table (
TAUE155.44 s and 44.59 s). #238 was measured on our reference branch, which is cut from #226's bpsi base, not fromdevelop. Run on this same deck, that branch'str2reproduces them, withPOHT0.5387 MW when fusion is off.developgives 0.4287 MW andTAUE166.76 s with fusion off, so the difference comes from the base, not from this change.d2bbfcca(docs only) adds this PR's ownCHANGELOG.mdentry: the sentences on the tenTRGLOBintegrals and on the NBI powers, which were in #269's entry, as asked in the second review of #269.🤖 Generated with Claude Code