♻️ Refactor Bruner div heat split - #4511
Conversation
chris-ashe
added
Physics
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4511 +/- ##
==========================================
- Coverage 49.32% 49.26% -0.07%
==========================================
Files 150 150
Lines 29802 29940 +138
==========================================
+ Hits 14700 14750 +50
- Misses 15102 15190 +88 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
chris-ashe
changed the title
e5d6816 to
95eb74f
Compare
… for clarity and update references
…date references in physics calculations
…pdate references in physics calculations
…date references in plasma calculations; add Brunner's method for divertor power splits calculation
…aust calculations
…rSeparatrixPowerSplits dataclass and update related methods
…ations in Physics class
…ted calculations in Physics class
…ated calculations in Physics class
…ted calculations in Physics class
…ated calculations in Physics class
…output formatting in PlasmaExhaust class
…lations in Physics class
…orSeparatrixPowerSplits
…Data and exhaust modules
…s in Physics class
…modules and update related calculations
… and distances for double null configuration
…plit plotting functions
… inner divertor power splits and update references
95eb74f to
6168a31
Compare
… power decay length and update related variables and tests
|
|
||
| @staticmethod | ||
| def calculate_brunner_divertor_power_splits( | ||
| dr_plasma_outboard_midplane_separatrix_separation: float, |
There was a problem hiding this comment.
this name is far too long to be used in the function (hence the formatting nightmare), fine if you want it for input but please rename it to something more useable inside the function
je-cook
left a comment
je-cook
left a comment
There was a problem hiding this comment.
I think generally looks fine, some stylistic things and a model review required.
I have general comment on long variable names. I understand and mostly agree with the reasoning within the data structure (as that is user facing).
However within functions I think long variable names are less readable and make the code harder to read not easier. I have to scroll either along or down remove all the extraneous information and then see what the calculation is. This is annoying when its everywhere.
My suggestion is when creating variables within a function that they are reasonably short, not necessarily 6char limits or anything silly but just shorter. Anything thats user facing (function I/O, data structure), continue as before.
Happy to have a discussion here (or offline)
| f_p_outboard_upper = np.zeros_like(dr_sep_values) | ||
|
|
||
| for idx, dr_sep in enumerate(dr_sep_values): | ||
| div_power_splits = PlasmaExhaust().calculate_brunner_divertor_power_splits( |
There was a problem hiding this comment.
As your using it here you dont need to initialise the PlasmaExhaust class (its a staticmethod).
I would suggest moving the method out of the class and just leaving in the the exhaust file and calling that here. If you need it on the class for some reason have a method that passes all logic to the function.
| """Plot Brunner divertor power split fractions as a stack plot over dr_sep.""" | ||
| # Use the case decay length when available; fall back to 1 mm if absent. | ||
|
|
||
| len_plasma_sol_outboard_power_decay = mfile.get( |
There was a problem hiding this comment.
then you only need one call to mfile (as you've used below). Also we're inside a function you dont need to use the full length of all the names here, this is probably a general point I will flag too
| len_plasma_sol_outboard_power_decay = mfile.get( | |
| len_sol_outboard_pd, len_sol_inboard_pd = mfile.get_variables("len...", "len...", scan=scan) |
| po.ovarre( | ||
| self.outfile, | ||
| "Required distance between the first and second plasma separatrixes at " | ||
| "the outer midplane (δR_sep) [m]", | ||
| "(dr_plasma_outboard_midplane_separatrix_separation)", | ||
| self.data.physics.dr_plasma_outboard_midplane_separatrix_separation, | ||
| "OP ", | ||
| ) | ||
| po.oblnkl(self.outfile) | ||
|
|
||
| po.ovarre( | ||
| self.outfile, | ||
| "Outboard side heat flux decay length (m)", | ||
| "(len_sol_outboard_power_decay)", | ||
| self.data.physics.len_sol_outboard_power_decay, | ||
| "OP ", | ||
| ) | ||
| po.oblnkl(self.outfile) | ||
|
|
||
| po.ovarre( | ||
| self.outfile, | ||
| "Fraction of separatrix power on the inner target(s)", | ||
| "(f_p_div_inboard_separatrix)", | ||
| self.data.physics.f_p_div_inboard_separatrix, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Fraction of separatrix power on the outer target(s)", | ||
| "(f_p_div_outboard_separatrix)", | ||
| self.data.physics.f_p_div_outboard_separatrix, | ||
| "OP ", | ||
| ) | ||
|
|
||
| po.oblnkl(self.outfile) | ||
|
|
||
| po.ovarre( | ||
| self.outfile, | ||
| "Fraction of separatrix power on the inner lower target", | ||
| "(f_p_div_lower_inboard_separatrix)", | ||
| self.data.physics.f_p_div_lower_inboard_separatrix, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Separatrix power on the inner lower target", | ||
| "(p_div_lower_inboard_separatrix_mw)", | ||
| self.data.physics.p_div_lower_inboard_separatrix_mw, | ||
| "OP ", | ||
| ) | ||
| po.oblnkl(self.outfile) | ||
|
|
||
| po.ovarre( | ||
| self.outfile, | ||
| "Fraction of separatrix power on the outer lower target", | ||
| "(f_p_div_lower_outboard_separatrix)", | ||
| self.data.physics.f_p_div_lower_outboard_separatrix, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Separatrix power on the outer lower target", | ||
| "(p_div_lower_outboard_separatrix_mw)", | ||
| self.data.physics.p_div_lower_outboard_separatrix_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
this kind of style makes big output blocks much easier to read IMO. Less duplication of function calls and thier arguments, below suggestion is not finished just for gist
| po.ovarre( | |
| self.outfile, | |
| "Required distance between the first and second plasma separatrixes at " | |
| "the outer midplane (δR_sep) [m]", | |
| "(dr_plasma_outboard_midplane_separatrix_separation)", | |
| self.data.physics.dr_plasma_outboard_midplane_separatrix_separation, | |
| "OP ", | |
| ) | |
| po.oblnkl(self.outfile) | |
| po.ovarre( | |
| self.outfile, | |
| "Outboard side heat flux decay length (m)", | |
| "(len_sol_outboard_power_decay)", | |
| self.data.physics.len_sol_outboard_power_decay, | |
| "OP ", | |
| ) | |
| po.oblnkl(self.outfile) | |
| po.ovarre( | |
| self.outfile, | |
| "Fraction of separatrix power on the inner target(s)", | |
| "(f_p_div_inboard_separatrix)", | |
| self.data.physics.f_p_div_inboard_separatrix, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Fraction of separatrix power on the outer target(s)", | |
| "(f_p_div_outboard_separatrix)", | |
| self.data.physics.f_p_div_outboard_separatrix, | |
| "OP ", | |
| ) | |
| po.oblnkl(self.outfile) | |
| po.ovarre( | |
| self.outfile, | |
| "Fraction of separatrix power on the inner lower target", | |
| "(f_p_div_lower_inboard_separatrix)", | |
| self.data.physics.f_p_div_lower_inboard_separatrix, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Separatrix power on the inner lower target", | |
| "(p_div_lower_inboard_separatrix_mw)", | |
| self.data.physics.p_div_lower_inboard_separatrix_mw, | |
| "OP ", | |
| ) | |
| po.oblnkl(self.outfile) | |
| po.ovarre( | |
| self.outfile, | |
| "Fraction of separatrix power on the outer lower target", | |
| "(f_p_div_lower_outboard_separatrix)", | |
| self.data.physics.f_p_div_lower_outboard_separatrix, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Separatrix power on the outer lower target", | |
| "(p_div_lower_outboard_separatrix_mw)", | |
| self.data.physics.p_div_lower_outboard_separatrix_mw, | |
| "OP ", | |
| ) | |
| for op in [ | |
| ( | |
| "Required distance between the first and second plasma separatrixes at " | |
| "the outer midplane (δR_sep) [m]", | |
| "(dr_plasma_outboard_midplane_separatrix_separation)", | |
| self.data.physics.dr_plasma_outboard_midplane_separatrix_separation, | |
| ), | |
| None, | |
| (...), | |
| ]: | |
| if op is None: | |
| po.oblnkl(self.outfile) | |
| else: | |
| desc, var, val = op | |
| po.ovvare(self.outfile, desc, var, val, "OP ") |
There was a problem hiding this comment.
I rather imagined that the new data structure would allow us to write something simple like
Output variables a, b, c, d
| return p_plasma_rad_mw / p_plasma_heating_mw | ||
|
|
||
| @staticmethod | ||
| def calculate_brunner_divertor_power_splits( |
There was a problem hiding this comment.
Could I have @ukaea/process-model-review look at this please
4 participants
Description
This pull request introduces significant improvements to the plasma exhaust modeling and visualization in PROCESS. The main changes include the addition of new documentation for plasma exhaust physics, refactoring and renaming of variables related to divertor power splits for clarity and consistency, and the implementation of new plotting functions to visualize separatrix power splits and Brunner model power sharing. These updates enhance both the usability and accuracy of the plasma exhaust modeling, while also improving the documentation and user interface.
Plasma Exhaust Modeling and Documentation:
plasma_exhaust.md) describing separatrix power, divertor protection metrics, and the Brunner model for power splits.Plasma Exhaustto the navigation inmkdocs.ymlfor easier access to the new documentation.Variable Renaming and Data Structure Updates:
f_p_div_lowertof_p_div_lower_separatrixand updated related obsolete variable mappings for clarity and consistency in input, scan, and obsolete variable code.PhysicsDatato include explicit variables for separatrix power splits and target powers (e.g.,f_p_div_inboard_separatrix,p_div_lower_inboard_separatrix_mw, etc.), replacing ambiguous or legacy variable names.Visualization and Plotting Enhancements:
Large tokamak
ST
Codebase Maintenance:
DivertorNumberModelsandPlasmaExhaustwhere required. [1] [2]These changes collectively improve the clarity, maintainability, and scientific accuracy of the plasma exhaust modeling in PROCESS, while also enhancing the user experience through better documentation and visualization.
Checklist
I confirm that I have completed the following checks: