Conversation
…oil.py` and `plot_proc.py`
… files for consistency.
…es to improve clarity and consistency.
…ncy across multiple files
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3693 +/- ##
==========================================
- Coverage 38.13% 38.03% -0.11%
==========================================
Files 88 88
Lines 22470 22558 +88
==========================================
+ Hits 8570 8579 +9
- Misses 13900 13979 +79 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR renames several CS turn parameters for clarity, updates their usage in calculations and I/O routines, and adds new plotting functions to visualize the CS coil and turn structures.
- Renamed
l_cond_cst,d_cond_cst,r_in_csttodr_cs_turn,dz_cs_turn,radius_cs_turn_cable_spaceacross data files, Fortran, and Python code. - Updated CS turn geometry calculations, output routines (
output_cs_structure), and removed deprecated CF writes. - Added
plot_cs_coil_structureandplot_cs_turn_structureinprocess/io/plot_proc.pyand integrated them intomain_plot.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/data/large_tokamak_MFILE.DAT | Updated CS turn parameter names in unit test data |
| tests/integration/data/*MFILE.DAT | Updated CS turn parameter names in integration data |
| examples/data/*MFILE.DAT | Updated example data to reflect renamed parameters |
| source/fortran/pfcoil_variables.f90 | Renamed CS turn variables in module definition |
| process/tf_coil.py | Refactored CS turn geometry calculation to use new names |
| process/pfcoil.py | Replaced old CS turn vars, added output_cs_structure |
| process/io/plot_proc.py | Added plotting functions for CS coil and turn, integrated into workflow |
| documentation/proc-pages/eng-models/central-solenoid.md | Updated docs to use new parameter names |
Comments suppressed due to low confidence (3)
process/io/plot_proc.py:4347
- The function docstring mentions a
demo_rangesparameter which isn't in the signature. Update or remove that reference to keep docs accurate.
def plot_cs_coil_structure(axis, fig, mfile_data, scan, colour_scheme=1):
process/io/plot_proc.py:4445
- The data key
'n_pf_coil_turns[n_cs_pf_coils-1]'is unlikely to exist. You should fetch'n_pf_coil_turns'and then index it withn_cs_pf_coils-1rather than including brackets in the key.
f"CS height vs TF internal height: {mfile_data.data['f_z_cs_tf_internal'].get_scan(scan):.2f}\n"
source/fortran/pfcoil_variables.f90:399
- The comment for
dr_cs_turnstill refers to "Length of CS of CS coil turn conduit". Update it to clearly describe the new meaning (e.g., "radial length of a CS turn conduit").
real(dp) :: dr_cs_turn
…ations for CS turn cable space and add output variables for conduit dimensions.
…new images and improve figure captions.
clmould
left a comment
There was a problem hiding this comment.
Looks good, just a couple minor comments.
Also, the old variables d_cond_cst, l_cond_cst and r_in_cst need to be updated in vardes.md.
| csfv.t_structural_vertical = csfv.t_structural_radial | ||
| # add a check for negative conduit thickness | ||
| if csfv.t_structural_radial < 1.0e-3: | ||
| csfv.t_structural_radial = 1.0e-3 |
There was a problem hiding this comment.
Was this needed? Or can it safely be removed? (I am unsure either way)
There was a problem hiding this comment.
Its a fudge in my eyes that creates discontinuities in the calculations
There was a problem hiding this comment.
I agree that it creates discontinuities, but I think that removing it may cause issues where t_structural_radial becomes negative. A continuous kludge has been implemented for pdivt in #3579 (more info in #3578), so perhaps this idea could be used here so it ensures that t_structural_radial does not go negative whilst keeping things continuous
| csfv.t_structural_vertical = csfv.t_structural_radial | ||
| # add a check for negative conduit thickness | ||
| if csfv.t_structural_radial < 1.0e-3: | ||
| csfv.t_structural_radial = 1.0e-3 |
There was a problem hiding this comment.
I agree that it creates discontinuities, but I think that removing it may cause issues where t_structural_radial becomes negative. A continuous kludge has been implemented for pdivt in #3579 (more info in #3578), so perhaps this idea could be used here so it ensures that t_structural_radial does not go negative whilst keeping things continuous
This pull request introduces several updates to improve the handling and visualization of central solenoid (CS) coil and turn structures in the codebase. Key changes include updates to data files to rename parameters for clarity, and enhancements to the plotting functionality to visualize CS coil and turn structures. This includes a temporary output plotting method
output_cs_structure()🔄 Renames
d_cond_cst->dz_cs_turnl_cond_cst->dr_cs_turnr_in_cst->radius_cs_turn_cable_spaceVisualization Enhancements:
plot_cs_coil_structureandplot_cs_turn_structurefunctions toprocess/io/plot_proc.pyfor visualizing CS coil cross-sections and turn conductor structures. These functions plot detailed geometries and annotate parameters such as turn dimensions, cable space, and structural thickness.main_plotfunction, adding new subplots (fig10) for CS coil and turn structure visualizations.Updates to Main Plotting Workflow:
main(args=None)function to include the new figure (page10) for CS structure visualizations, ensuring it is saved to the PDF output and properly closed after execution.✍️
plot_procupdateThe new image is now made in
plot_procits a first step so its not very pretty at the momentChecklist
I confirm that I have completed the following checks: