Read TRANSP output: a transcript that resolves grids by name and converts nothing - #658
Merged
Merged
Conversation
…erts nothing vaft.code.transp reads a TRANSP run's .CDF and stops there. There is no config, no prepare_* and no run_*: VAFT reads a TRANSP run, it does not launch one, so there is no $TRANSPHOME either. Three things the file forces, each of which the code this replaces got wrong: - A variable's grid comes from its dimension name, never its length. X (zone centres) and XB (zone outer boundaries) are the *same length* in a real run -- 20 and 20 for the MAST case -- so the legacy length comparison silently returned every boundary quantity as a centre one. on_x/on_xb refuse the wrong grid and say that interpolating between them is the caller's explicit choice. - Units come from each variable's own attribute: NE says N/CM**3, TQIN says Nt-M/CM3, PLFLX says Wb/rad. Nothing takes them from an argument, so no wrong flag can cost a factor of a million. - The profiles are densities, not per-zone integrals. Worth stating because the sibling adapter is the other way round -- NUBEAM's Plasma State writes per-zone integrals -- and assuming the family convention carries is an error of order the zone volume. Checked rather than assumed: sum(TQIN) is 5.6e-7, which matches nothing in the file, while sum(TQIN*DVOL) is -0.544 N m, the order of the run's own newton-metre torque scalars (BPHXB = -0.713). TQTOTNB is refused by name, pointing at TQIN: it is a dimensionless zero. The file's own integrity statements are checked on open -- PLFLX2PI/PLFLX is 2*pi, X and XB increase and interleave -- because a run killed mid-write leaves a readable header over truncated data that a classic netCDF reads back as silent zeros. psi_norm is PLFLX/PLFLXA, not (P-P[0])/(P[-1]-P[0]), which would call the innermost zone boundary the axis. enclosed_torque is the one place the grids interact: TQIN and DVOL are both cm-based and zone-centre, so their product is already newton metres, and the cumulative sum lands on the zone boundaries. Read through xarray's scipy backend, explicitly: netCDF4 is not a VAFT dependency and letting xarray auto-detect would exercise whichever backend happens to be installed. Reading is lazy -- a production file holds ~1900 variables. Verified against a real restricted run (45453X01, not committed): TQTOTNB is a dimensionless scalar, PLFLX[-1] == PLFLXA, PLFLX2PI/PLFLX = 6.2831855, the 750 ms enclosed torque totals -0.5441 N m, and psi_norm[0] = 0.0049462188 -- the value the committed reference .kin begins at. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n fail Review of the TRANSP adapter found the reader trusting three things the file does not guarantee, and a fixture too tame to catch any of them. Correctness: - TIME and TIME3 are separate dimensions. A sample index chosen on TIME was applied to profiles on TIME3 with nothing checking they agree: a mismatched pair either raised a bare IndexError from inside the adapter or, worse, returned a profile from one instant while reporting another. They are now required to hold the same instants, which is what the runs looked at do. - X and XB were indexed on axis 0 without checking they carry a time axis. A run writing its grids once as plain (X,) made state.x a single radial point while the profiles stayed full length. Both now go through _at_time, as TranspSlice.variable already did. - psi_norm_xb read PLFLXA through the time-sliced accessor, which is right, but .reshape(-1)[0] read like "take the first sample" and no test could tell the difference: the fixture held PLFLXA constant. The step is now explicit and refuses a whole series, and the fixture's edge flux rises through the run as the reference file's does (0.0272 to 0.0765 Wb/rad). - close() left the object half-working -- cached variables answered, others died on a NoneType deep in xarray. A closed file now says so. Integrity. _check_integrity's docstring claimed a truncated file would be caught and nothing in it did that. Truncation is now detected twice: the backend's own failure to parse a short classic file is translated into a message that names the cause, and a size floor catches what it tolerates (5,566,599 bytes declared against 5,921,444 on disk for the reference run, so no false positive). Grid monotonicity and interleaving are checked at every sample rather than the first -- a zeroed tail is exactly a row that has stopped increasing -- and a PLFLX holding no non-zero finite value is refused instead of skipping the Wb/rad check and reporting the file sound. Cross-file: - collect_transp_outputs described a runid filter it never implemented, so it picked up the <runid>PH.CDF sibling and let mtime decide the runid. It now matches TRANSP's naming and orders by name; other .CDF files are listed under "other_cdf" rather than dropped. - The private vaft.code.gpec._netcdf import pulled the whole GPEC package in behind a reader that needs nothing from it (1.43 s against 0.61 s for nubeam), and imported float_attr without using it. Replaced by a local twelve-line helper. - TQTOTNB is not dimensionless: it declares Nt-M/CM3 and is empty. Corrected in the module, torque.py, the docs and the fixture docstring. - TRANSPResult carried a `native` field nothing populated; dropped, and the returncode/ok convention is now stated. zone_volume and TranspFormatError added to the __all__ lists that omitted them. - The TRANSP prose sat between the entry-point table and "the NUBEAM equivalent", stranding that phrase's antecedent; moved after the NUBEAM section. Tests. The fixture now carries what the reference file carries and this one did not: PLFLXA on TIME rather than TIME3 and varying through the run, a flux profile that is not proportional to XB (so psi_norm and the radial coordinate are distinguishable, 0.0049 against 0.05 at the innermost boundary in the real run), a variable on a third radial dimension, a scalar time series, a bare dimensionless scalar, and knobs to break one invariant at a time. Tests that asserted the fixture rather than the code are rewritten; the integrity branches that had no test -- shape, monotonicity, interleaving -- now have one. 24 tests to 37, and the three mutations that survived the review (PLFLXA[0] regardless of time, no shape check, no monotonicity check) are killed. The reference run still reads: -0.5441 N m enclosed at 750 ms, psi_norm[-1] exactly 1.0, psi_norm[0] 0.004946219. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR E of the V4 migration slice (hsyun_GPEC#26), and independent of #651 — this is the TRANSP side.
vaft.code.transpreads a run's.CDFand stops there. No*Config, noprepare_*, norun_*, no$TRANSPHOME: VAFT reads a TRANSP run, it does not launch one, and the package docstring says so rather than leaving a reviewer to wonder where they went.The three things the file forces
A variable's grid comes from its dimension name, never its length (C-39).
X(zone centres) andXB(zone outer boundaries) are the same length in a real run — 20 and 20 for the MAST case — so the legacy length comparison silently returned every boundary quantity as a centre one.on_x/on_xbrefuse the wrong grid, and say that interpolating between the two is the caller's explicit choice.Units come from each variable's own attribute (C-40).
NEsaysN/CM**3,TQINsaysNt-M/CM3,PLFLXsaysWb/rad. Nothing takes them from an argument, so no wrong flag costs a factor of a million.The profiles are densities, not per-zone integrals — and this is worth stating because the sibling adapter is the other way round. NUBEAM's Plasma State writes per-zone integrals; assuming the family convention carries is an error of order the zone volume. Checked rather than assumed:
sum(TQIN)is 5.6e-7, which matches nothing in the file, whilesum(TQIN·DVOL)is −0.544 N·m — the order of the run's own newton-metre torque scalars (BPHXB= −0.713).Also
TQTOTNBis refused by name, pointing atTQIN(C-19): it is a dimensionless zero. The file's own integrity statements are checked on open —PLFLX2PI/PLFLXis 2π,XandXBincrease and interleave — because a run killed mid-write leaves a readable header over truncated data that a classic netCDF reads back as silent zeros.psi_normisPLFLX/PLFLXA, not(P−P[0])/(P[−1]−P[0]), which would declare the innermost zone boundary to be the magnetic axis.enclosed_torqueis the one place the grids interact:TQINandDVOLare both cm-based and zone-centre, so the product is already newton metres, and the cumulative sum lands on the zone boundaries. Pairing them once, here, is deliberate.Read through xarray's scipy backend explicitly —
netCDF4is not a VAFT dependency and is imported nowhere invaft/, so letting xarray auto-detect would exercise whichever backend happens to be installed. No new dependency. Reading is lazy; a production file holds ~1900 variables.Verification
Against a real restricted run (45453X01, not committed, D-02), through the new adapter:
TQTOTNB.shape == ();PLFLX[-1] == PLFLXA;PLFLX2PI/PLFLX = 6.2831855; the 750 ms enclosed torque totals −0.5441 N·m; andpsi_norm[0] = 0.0049462188— the value the committed reference.kinbegins at, which is the C-39 evidence.24 tests on a synthetic NETCDF3_CLASSIC fixture shaped from that file, awkward details included: equal-length interleaved grids, CGS units per variable, a dimensionless empty
TQTOTNB,PLFLX2PI = 2π·PLFLX, and a variable the adapter deliberately does not catalogue. Develop gate: 4921 passed.🤖 Generated with Claude Code