Skip to content

Add option for density pedestal setting#4181

Open
chris-ashe wants to merge 8 commits intomainfrom
add_option_for_density_pedestal_setting
Open

Add option for density pedestal setting#4181
chris-ashe wants to merge 8 commits intomainfrom
add_option_for_density_pedestal_setting

Conversation

@chris-ashe
Copy link
Copy Markdown
Collaborator

@chris-ashe chris-ashe commented Apr 13, 2026

This pull request introduces significant improvements and clarifications to how the plasma density pedestal and separatrix values are set in the codebase, as well as enhancements to the handling and reporting of line-averaged electron temperature and density. The changes include a new switch to control the method for setting pedestal/separatrix values, refactoring of related logic, updates to input validation and parameter ranges, and improvements to output and documentation.

Pedestal and Separatrix Density Handling:

  • Introduced the i_nd_plasma_pedestal_separatrix switch and the DensityProfilePedestalType enum to control whether the pedestal and separatrix densities are set by user input or as fractions of the Greenwald limit, centralizing and clarifying this logic throughout the codebase.

  • Updated documentation to clearly describe the new switch and both methods for setting pedestal and separatrix densities, and removed outdated/ambiguous instructions.

  • Adjusted the allowed input and iteration variable ranges for f_nd_plasma_pedestal_greenwald and f_nd_plasma_separatrix_greenwald to reflect physically meaningful values and new logic.

Line-Averaged Quantities and Output Improvements:

  • Added calculation, storage, and output of the line-averaged electron temperature (temp_plasma_electron_line_avg_kev) and improved calculation of line-averaged electron density. These are now included in both the data structure and output files, and displayed in summary plots.

Code and Documentation Clean-up:

  • Refined and clarified docstrings and removed legacy comments related to now-obsolete ways of setting pedestal/separatrix densities.

These changes make the codebase more robust, user-friendly, and scientifically transparent regarding how key plasma profile parameters are set and reported.## Description

Checklist

I confirm that I have completed the following checks:

  • My changes follow the PROCESS style guide
  • I have justified any large differences in the regression tests caused by this pull request in the comments.
  • I have added new tests where appropriate for the changes I have made.
  • If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments.
  • If I have made documentation changes, I have checked they render correctly.
  • I have added documentation for my change, if appropriate.

@chris-ashe chris-ashe added Profiles Relating to the plasma profiles Refactor labels Apr 13, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 64.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.57%. Comparing base (2899a60) to head (5c1d9f5).

Files with missing lines Patch % Lines
process/models/physics/profiles.py 50.00% 5 Missing ⚠️
process/models/physics/physics.py 25.00% 3 Missing ⚠️
process/models/physics/plasma_profiles.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4181   +/-   ##
=======================================
  Coverage   49.56%   49.57%           
=======================================
  Files         148      148           
  Lines       29719    29737   +18     
=======================================
+ Hits        14731    14741   +10     
- Misses      14988    14996    +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chris-ashe chris-ashe requested a review from j-a-foster April 13, 2026 14:28
@chris-ashe chris-ashe marked this pull request as ready for review April 13, 2026 14:28
@chris-ashe chris-ashe requested a review from a team as a code owner April 13, 2026 14:28
Copilot AI review requested due to automatic review settings April 13, 2026 14:28
@chris-ashe
Copy link
Copy Markdown
Collaborator Author

Have taken the liberty to add the line averaged density and temp values here. @j-a-foster Can you please check the sanity of the line averaged calcs and also the output changes?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a switchable mechanism for setting pedestal/separatrix electron densities (user-input vs Greenwald-fraction) and extends profile reporting by computing and outputting line-averaged electron temperature.

Changes:

  • Introduced i_nd_plasma_pedestal_separatrix / DensityProfilePedestalType and centralized pedestal/separatrix density assignment via Greenwald fractions.
  • Added temp_plasma_electron_line_avg_kev computation and surfaced it in outputs and summary plots.
  • Updated input validation/ranges and refreshed related documentation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
process/models/physics/profiles.py Adds DensityProfilePedestalType and NeProfile.set_pedestal_and_separatrix_values() to centralize pedestal/separatrix density setting.
process/models/physics/plasma_profiles.py Computes/stores line-averaged electron temperature for both parabolic and pedestal parameterisations.
process/models/physics/physics.py Routes pedestal/separatrix density setup through the new NeProfile method; outputs line-averaged Te; updates output gating to use the new switch.
process/data_structure/physics_variables.py Adds i_nd_plasma_pedestal_separatrix and temp_plasma_electron_line_avg_kev to the data structure + initialization defaults.
process/core/solver/iteration_variables.py Adjusts iteration-variable bounds for Greenwald-fraction inputs.
process/core/io/plot/summary.py Displays line-averaged density and temperature in profile summary plots.
process/core/input.py Updates input ranges for Greenwald-fraction variables and attempts to add the new switch (currently with a naming bug).
process/core/init.py Updates density validation logic to use the new switch when interpreting manual pedestal/separatrix densities.
documentation/source/physics-models/profiles/plasma_profiles.md Removes outdated guidance for toggling Greenwald-fraction behavior via negative inputs.
documentation/source/physics-models/profiles/plasma_density_profile.md Documents the new switch and the Greenwald-fraction method (contains an iteration-variable index typo).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread process/core/input.py
"nd_plasma_separatrix_electron": InputVariable(
data_structure.physics_variables, float, range=(0.0, 1e21)
),
"i_nd_plasma_pedestal_electron": InputVariable(
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Input parsing defines i_nd_plasma_pedestal_electron, but the new switch used throughout the codebase is i_nd_plasma_pedestal_separatrix. As written, the input file cannot set the new switch (and will instead create an unused module attribute), so runs will always use the default method. Rename this input key to i_nd_plasma_pedestal_separatrix (and consider keeping an alias for backwards compatibility if needed).

Suggested change
"i_nd_plasma_pedestal_electron": InputVariable(
"i_nd_plasma_pedestal_separatrix": InputVariable(

Copilot uses AI. Check for mistakes.
$$


$\texttt{f_nd_plasma_pedestal_greenwald}$ and $\texttt{f_nd_plasma_separatrix_greenwald}$ can be set as iteration variables respectively by using `ixc = 45`
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says f_nd_plasma_pedestal_greenwald can be set as an iteration variable with ixc = 45, but the actual iteration variable index for this parameter is 145 (see process/core/solver/iteration_variables.py). Update the docs to use ixc = 145 to avoid misleading users.

Suggested change
$\texttt{f_nd_plasma_pedestal_greenwald}$ and $\texttt{f_nd_plasma_separatrix_greenwald}$ can be set as iteration variables respectively by using `ixc = 45`
$\texttt{f_nd_plasma_pedestal_greenwald}$ and $\texttt{f_nd_plasma_separatrix_greenwald}$ can be set as iteration variables respectively by using `ixc = 145`

Copilot uses AI. Check for mistakes.
Comment on lines +130 to +136
physics_variables.temp_plasma_electron_line_avg_kev = (
physics_variables.temp_plasma_electron_vol_avg_kev
* (1.0 + physics_variables.alphat)
* (sp.special.gamma(0.5) / 2.0)
* sp.special.gamma(physics_variables.alphat + 1.0)
/ sp.special.gamma(physics_variables.alphat + 1.5)
)
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temp_plasma_electron_line_avg_kev is newly computed and stored for both parabolic and pedestal profile parameterisations, but the existing unit tests for PlasmaProfile don't assert this new output. Please extend the relevant tests (e.g. tests/unit/test_plasma_profiles.py) to cover the new line-averaged electron temperature calculation for at least one parabolic and one pedestal case.

Copilot uses AI. Check for mistakes.
Comment on lines +231 to +257
def set_pedestal_and_separatrix_values(self):
"""Sets the pedestal and separatrix density values based on the user input or greenwald fraction method."""

if (
DensityProfilePedestalType(physics_variables.i_nd_plasma_pedestal_separatrix)
== DensityProfilePedestalType.USER_INPUT
):
pass
elif (
DensityProfilePedestalType(physics_variables.i_nd_plasma_pedestal_separatrix)
== DensityProfilePedestalType.GREENWALD_FRACTION
):
physics_variables.nd_plasma_pedestal_electron = (
physics_variables.f_nd_plasma_pedestal_greenwald
* PlasmaDensityLimit.calculate_greenwald_density_limit(
c_plasma=physics_variables.plasma_current,
rminor=physics_variables.rminor,
)
)
physics_variables.nd_plasma_separatrix_electron = (
physics_variables.f_nd_plasma_separatrix_greenwald
* PlasmaDensityLimit.calculate_greenwald_density_limit(
c_plasma=physics_variables.plasma_current,
rminor=physics_variables.rminor,
)
)

Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new set_pedestal_and_separatrix_values() method introduces a switch-controlled code path (USER_INPUT vs GREENWALD_FRACTION) that directly affects nd_plasma_pedestal_electron / nd_plasma_separatrix_electron, but there are no unit tests exercising either branch. Add tests that (1) verify densities are left unchanged for USER_INPUT and (2) verify the Greenwald-fraction path matches PlasmaDensityLimit.calculate_greenwald_density_limit() for a representative plasma_current/rminor.

Copilot uses AI. Check for mistakes.
@timothy-nunn timothy-nunn self-assigned this Apr 14, 2026
DensityProfilePedestalType(physics_variables.i_nd_plasma_pedestal_separatrix)
== DensityProfilePedestalType.USER_INPUT
):
pass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this if statement

$$


$\texttt{f_nd_plasma_pedestal_greenwald}$ and $\texttt{f_nd_plasma_separatrix_greenwald}$ can be set as iteration variables respectively by using `ixc = 45`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$\texttt{f_nd_plasma_pedestal_greenwald}$ and $\texttt{f_nd_plasma_separatrix_greenwald}$ can be set as iteration variables respectively by using `ixc = 45`
`f_nd_plasma_pedestal_greenwald` and `f_nd_plasma_separatrix_greenwald` can be set as iteration variables respectively by using `ixc = 45`

This seems to be the convention on this page for PROCESS variables name outside of equations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Profiles Relating to the plasma profiles Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants