Conversation
…oving warning on Hbs since formula is correct
… in rdcon_netcdf.f
There was a problem hiding this comment.
Pull request overview
This PR updates the Mercier/MRE-related calculations and NetCDF outputs to correct several formula/normalization issues (inverse minor aspect ratio/trapped fraction impacts, Wc prefactor flux normalization) and to adjust what is emitted in rdcon_output_n*.nc.
Changes:
- Update MRE geometry-derived quantities (e.g.,
eps_loc, trapped fraction) and revise bootstrap-drive-related term handling. - Correct
Wc_prefacnormalization to account for toroidal→(normalized) poloidal flux conversion. - Extend/adjust NetCDF outputs: add globals (
volume,Zeff,bwall), add profile varh, renameHbs→Hbs_prefac, and stop writing the brokenavg_mu0Jbs_dot_Bterm.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
rdcon/rdcon_netcdf.f |
Adds new global attributes and profile/MRE variables to NetCDF output; renames Hbs output and removes avg_mu0Jbs_dot_B output. |
rdcon/mercier.f |
Updates surface geometry/trapped-fraction calculation, revises MRE prefactors, and annotates/retains (but flags) the suspected-broken bootstrap term. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| c initialise extremum surface locations. | ||
| c----------------------------------------------------------------------- | ||
| rmax_loc=0.0 | ||
| rmin_loc=HUGE(1.0_8) | ||
| zmax_loc=-HUGE(1.0_8) | ||
| zmin_loc=HUGE(1.0_8) |
There was a problem hiding this comment.
The extrema initialization (and the associated per-θ extrema tracking) runs unconditionally for every Mercier scan, but the resulting geometric quantities are only used under MRE_flag. Consider moving the extrema init/update under IF(MRE_flag) to avoid extra work in non-MRE runs.
| CALL check( nf90_def_var(ncid, | ||
| $ "Dnc", nf90_double, p_dim, dnc_id)) | ||
| CALL check( nf90_def_var(ncid, | ||
| $ "Wc", nf90_double, p_dim, wc_id)) |
There was a problem hiding this comment.
avg_mu0Jbs_dot_B is no longer defined/written, but jbs_id is still declared (and mreterms%fs(:,6) still exists). Consider removing jbs_id (and/or documenting the skipped index) so the netCDF writer and mreterms layout don’t silently diverge.
| $ "Wc", nf90_double, p_dim, wc_id)) | |
| $ "Wc", nf90_double, p_dim, wc_id)) | |
| c | |
| c NOTE: The MRE terms layout (e.g., mreterms%fs(:,6)) still reserves | |
| c an index for the deprecated quantity avg_mu0Jbs_dot_B. That term | |
| c is intentionally no longer written to this NetCDF file, so no | |
| c nf90_def_var call appears here for it. The sequence of variables | |
| c defined below therefore skips that index by design. | |
| c |
| CALL check( nf90_put_att(ncid,nf90_global,'volume', volume)) | ||
| CALL check( nf90_put_att(ncid,nf90_global,'Zeff', Zeff)) | ||
| CALL check( nf90_put_att(ncid,nf90_global,'bwall', bwall)) |
There was a problem hiding this comment.
Global NetCDF attributes in this file consistently use lowercase names (e.g., betan, bwall, volume), but this adds 'Zeff' with mixed case. Consider renaming the attribute to zeff (or otherwise matching the established casing) to avoid surprising downstream readers that treat attribute names as case-sensitive.
| Jpara=psio*f1*avg(16)+p1*avg(18)*twopif/(twopi*psio) + | ||
| $ (twopif/twopi)**2*f1*avg(17)/psio !(mu0 included in p1)) | ||
| Jtor=p1*avg(12)/psio + (twopif/twopi)*f1*avg(13)/psio |
There was a problem hiding this comment.
Jtor is assigned but never used. If it’s not needed, remove the variable (and its declaration) to avoid dead code; otherwise, consider using it or documenting why it’s computed.
| mreterms%fs(ipsi,4)=ftr | ||
| mreterms%fs(ipsi,5)=mufrac | ||
| mreterms%fs(ipsi,6)=avg_Jboot_dot_B | ||
| mreterms%fs(ipsi,6)=avg_Jboot_dot_B !broken formula |
There was a problem hiding this comment.
mreterms%fs(ipsi,6) is being populated with a value marked as a “broken formula”. Since it’s also no longer written to netCDF, consider setting it to a sentinel (e.g., 0/NaN) or removing the field to prevent accidental downstream use of known-bad data.
| mreterms%fs(ipsi,6)=avg_Jboot_dot_B !broken formula | |
| mreterms%fs(ipsi,6)=0.0D0 ! intentionally set sentinel; avg_Jboot_dot_B formula is broken |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
A few of the formulas in the MRE scripts were incorrect, namely the way I calculated inverse minor aspect ratio (eps_local), and its corresponding impact on the trapped fraction and bootstrap drive. The second erroneous formula that has since been fixed was Wc_prefac - I realized the original paper used toroidal flux not poloidal flux. I added the conversion factor to convert to normalized poloidal flux consistent with everything else. Finally the J_bs_dot_B formula, although correctly copied from Callen, 2010 UW-CPTC 09-6R equation C18, seems incorrect, so I've stopped printing it out in rdcon_netcdf.f. I haven't removed the equation because I don't understand why it's broken, and I think its useful to keep it there - happy to remove it if you think that's cleaner though.
Aside from that mentioned above, I've added some small changes that improve this section of code overall: more comments and revising variable name Hbs -> Hbs_prefac to match what is actually being computed and printed