Skip to content

Conversation

@timothy-nunn
Copy link
Collaborator

@timothy-nunn timothy-nunn commented Oct 22, 2025

Removes F-values from PROCESS

  • Add a test for the constraints that runs and checks no attribute/type errors occur (fixed a few bugs that arose before the removal)
  • Removed f-values from constraints, iteration variables, inputs, scan variables, and their mention in the docs. The follow f-values have been kept but have been removed from their ability to iterate:
    • fl_h_threshold
    • fiooic
    • fjohc
    • fjohc0
    • fdene
    • fradpwr
  • Removed f-values from all regression tests
    • st_regression.IN.DAT used some weird f-value bounds so I have updated the constraints (see comment below).
  • Added an additional constraint (22) to enforce L-mode (constraint 15 remains to enforce H-mode).

@timothy-nunn timothy-nunn linked an issue Oct 22, 2025 that may be closed by this pull request
@timothy-nunn timothy-nunn self-assigned this Oct 30, 2025
@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2025

Codecov Report

❌ Patch coverage is 80.85106% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.22%. Comparing base (f59059d) to head (a400212).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
process/pfcoil.py 0.00% 4 Missing ⚠️
process/constraints.py 80.00% 3 Missing ⚠️
process/superconducting_tf_coil.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3947      +/-   ##
==========================================
+ Coverage   46.03%   46.22%   +0.19%     
==========================================
  Files         123      123              
  Lines       28970    28774     -196     
==========================================
- Hits        13335    13301      -34     
+ Misses      15635    15473     -162     

☔ 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.

@timothy-nunn timothy-nunn force-pushed the 3936-remove-f-values branch 3 times, most recently from 1bd708f to 6302f99 Compare November 6, 2025 13:45
Comment on lines 2685 to 2692
ixc = 46
fp_hcd_injected_max = 1.0
boundl(46) = 0.6
boundu(46) = 1.5
* DESCRIPTION: f-value for injected power variation range
* JUSTIFICATION: Setup to allow the injected power to vary

p_hcd_injected_max = 150.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When fp_hcd_injected_max = 1.5: $\frac{225.0}{150.0}-1.5 = 0$ therefore the new bound should be $225.0$

*--------------------------Numerics Variables---------------------------*
*‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾*

epsvmc = 1.0E-11
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was much lower than other regression tests

@timothy-nunn timothy-nunn marked this pull request as ready for review November 13, 2025 16:54
@timothy-nunn
Copy link
Collaborator Author

How to update an input file to not use f-values

  1. Ensure all equality constraint definitions (icc = ...) occur before any inequality constraints.
  2. Set the variable neqns equal to the number of equality constraints.
  3. Remove obsolete f-values from the input file.
  4. Remove iteration variables that corresponded to f-values.

In 99% of cases, this will allow your file to run without f-values and without treating inequality constraints as equality. However, if the input file specified weird bounds on the f-value iteration variables, you will need to modify the constraint bound (max/min) to account for this change.

Copy link
Collaborator

@mkovari mkovari left a comment

Choose a reason for hiding this comment

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

See comments



!!! Info "Constraints"
A full list of constraints is given on the variable description page in the row labelled
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to update this in due course.

OBS_VARS_HELP.update(
dict.fromkeys(
fvalues_list,
"F-values are no longer supported, please use inequality constraints",
Copy link
Collaborator

Choose a reason for hiding this comment

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

When the inequality constraints are documented we could add a link to the docs here.

numerics.boundu[37]
* pfcoil_variables.j_cs_critical_flat_top_end
# numerics.boundu[fjohc] * (which should have been 1)
pfcoil_variables.j_cs_critical_flat_top_end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this can ever be false:
abs(pfcoil_variables.j_cs_flat_top_end) > 0.99e0 * abs(pfcoil_variables.j_cs_critical_flat_top_end)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep this warning (cslimit).
Keep fjohc and fjohc0 as variables (not iteration) and include them in the constraint.
Use them instead of the bounds in this line of code.

"CS not using max current density: further optimisation may be possible"
)

# Check whether CS coil currents are feasible from engineering POV
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally these warnings should be recreated but it's no big deal.


# The operation current density weighted with the global iop/icrit fraction
lhs[:] = constraint_variables.fiooic * jcrit_vector
lhs[:] = jcrit_vector
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't really correct anymore, I suspect, and the same may be true for the code below.

Copy link
Collaborator Author

@timothy-nunn timothy-nunn Nov 25, 2025

Choose a reason for hiding this comment

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

What is the recourse here then? Should fiooic be added back? Or is there a way to calculate it from other variables in PROCESS (maybe the mentioned iop/icrit)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I really don't know anything about the stellarator code. I don't know what lhs and rhs are.


This constraint can be activated by stating `icc = 15` in the input file.

The scaling value `fl_h_threshold (ixc=103)` can be varied to set the required margin around the threshold.
Copy link
Collaborator

Choose a reason for hiding this comment

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

fl_h_threshold should be kept and renamed. This new way of using it should be documented here.

Copy link
Collaborator Author

@timothy-nunn timothy-nunn Nov 25, 2025

Choose a reason for hiding this comment

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

I have updated the description and docs to document how I think fl_h_threshold is working now. We should probably go through this at some point (with some example input files) to ensure its working as before/expected.

In particular, we should carefully check how it was used to enforce L-mode/H-mode before.

@timothy-nunn
Copy link
Collaborator Author

timothy-nunn commented Nov 25, 2025

Hi @ajpearcey @mkovari

Thank you both for the reviews. I have resolved everything that I have actioned. The following comments are still outstanding:

@timothy-nunn
Copy link
Collaborator Author

timothy-nunn commented Nov 25, 2025

@chris-ashe has pointed me to f_c_tf_turn_operating_critical which is possibly the same as fiooic.

Looking at an MFile, they have very similar values:

fiooic___________________________________________________________________ (itvar024)_____________________ 7.28529047768881166e-01 
Actual_current_/_critical_current________________________________________ (f_c_tf_turn_operating_critical)_ 7.28529093232043401e-01 OP 

So I can replace the warning and its use in stellarator with f_c_tf_turn_operating_critical.

This will not work for the Stellarator (#3947 (comment)) because f_c_tf_turn_operating_critical is not set, nor are a lot of variables related to the TF critical current (@ym1906 any ideas since you're probably most familiar with the Stellarator models? I could add an input in that is specifically a margin for the Stellarator model that would not iterate and would default to say 0.7).

@mkovari
Copy link
Collaborator

mkovari commented Nov 26, 2025

There are two separate constraint equations for enforcing the L-H threshold: icc = 15 and 73.
In both cases it would be tidier to have separate constraints for L and H mode.
In any case there needs to be a margin defined by a parameter (not an iteration variable) to replace fl_h_threshold.

@timothy-nunn
Copy link
Collaborator Author

There are two separate constraint equations for enforcing the L-H threshold: icc = 15 and 73. In both cases it would be tidier to have separate constraints for L and H mode. In any case there needs to be a margin defined by a parameter (not an iteration variable) to replace fl_h_threshold.

@ajpearcey @chris-ashe @jonmaddock do we have any thoughts on this? I'm not sure we can use fl_h_threshold as set out here.
image

I think this requires the margin (f-value) to be iterating.


# The operation current density weighted with the global iop/icrit fraction
lhs[:] = constraint_variables.fiooic * jcrit_vector
lhs[:] = jcrit_vector
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I really don't know anything about the stellarator code. I don't know what lhs and rhs are.

numerics.boundu[37]
* pfcoil_variables.j_cs_critical_flat_top_end
# numerics.boundu[fjohc] * (which should have been 1)
pfcoil_variables.j_cs_critical_flat_top_end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep this warning (cslimit).
Keep fjohc and fjohc0 as variables (not iteration) and include them in the constraint.
Use them instead of the bounds in this line of code.

"j_cs_pulse_start / j_cs_critical_pulse_start shouldn't be above 0.7 "
"for engineering reliability"
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

These warnings are fine. Remember the constraints on current density may not be applied (if T margin is used instead).

"f_c_tf_turn_operating_critical shouldn't be above 0.7 for engineering reliability"
)

po.ovarre(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This warning is fine.

args : output structure : residual error; constraint value;
fiooic: f-value for TF coil operating current / critical
j_tf_wp_critical: critical current density for winding pack (A/m2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We still need fiooic:
The current density margin can be set using fiooic.

This constraint can be activated by stating `icc = 5` in the input file.

The value of `i_density_limit` can be set to apply the relevant limit . The scaling value `fdene` can be varied also.
The value of `i_density_limit` can be set to apply the relevant limit. The variable `fdene` can be set to scale the constraint bound: `nd_plasma_electrons_vol_avg` / `nd_plasma_electrons_max` <= `fdene`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

i_density_limit isnt always limits that apply to the volume averaged density.

$$

**Therefore it is recommended to always use `icc = 15` if trying to simulate a plasma scenario specifically in L or H-mode**
Again, `fl_h_threshold` can be used to add a margin to the constraint. For example, `fl_h_threshold = 1.25` ensures that `p_plasma_separatrix_mw` is at least 25% less than the threshold power.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it not be 25% more?

******************
nsweep = 17 * b_tf_inboard_max, maximum peak toroidal field
isweep = 6
sweep = 10.5, 10.4, 10.3, 10.2, 10.1, 10.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know how close the solution is initially to this inboard leg limit? Would it be better to have a scan example that directly changes a variable instead of a limit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think for this PR I will leave it but we should probably create an issue to make a more meaningful example

)


@ConstraintManager.register_constraint(22, "MW", ">=")
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the sake of numerical sanity should this not be further down?

@timothy-nunn
Copy link
Collaborator Author

Chris also recommends having two separate margins for constraint 15 and 22

@timothy-nunn
Copy link
Collaborator Author

timothy-nunn commented Dec 15, 2025

Hi All, I have added in the two separate variables and updated the docstrings/docs accordingly. Please check my work carefully to ensure this new way of constraining on the LH threshold makes sense.

I have had to make some changes in physics.py because fl_h_threshold was used there, I am not sure if what I have done is correct?

\mathtt{fl\_h\_threshold} \times \underbrace{\mathtt{p\_l\_h\_threshold\_mw}}_{\text{Power from scaling}} >= \mathtt{p\_plasma\_separatrix\_mw}
$$

**Therefore it is recommended to always use `icc = 15` if trying to simulate a plasma scenario specifically in L or H-mode**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really icc = 15 is mandatory (unless a different mode is selected), since the plasma must be in some mode.

data_structure.current_drive_variables.n_beam_decay_lengths_core_required * cc,
)


Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this here all of a sudden?
(Equation to fix number of NBI decay lengths to plasma centre)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved the equations around so that they were in order

- data_structure.physics_variables.p_l_h_threshold_mw
/ data_structure.constraint_variables.l_mode_threshold_margin,
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This code really is a cumbersome way to write a simple equation.
It would be easier to read if each of the return quantities had a name.
The code is fine but I think you may need to rename the margin. l_mode_threshold_margin is a fraction, not a margin. If l_mode_threshold_margin = 0.8 then the margin is 20%.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove f-values

6 participants