Reject zero-thickness superconducting TF at input validation

[dallonby]

Closes #4534

Overview

Adds a check_process validation so that the silent zero-thickness-TF failure mode described in the issue becomes an immediate, actionable input error instead of an unexplained downstream solver failure.

Changes

• process/core/init.py: after the existing ixc 13/140 mutual-exclusion check, raise ProcessValidationError when i_tf_sup is superconducting, neither ixc 13 nor ixc 140 is active, and dr_tf_inboard <= 0. The message tells the user the three ways to fix their file. Stellarators (istell != 0, which calculate dr_tf_inboard during the model run) and IFE are excluded.
• New tests/unit/core/test_init.py: five tests — the rejected configuration, plus accepted configurations for explicit thickness, ixc 140 active, resistive TF, and stellarator.
• tests/unit/core/test_input.py (test-suite change, per CONTRIBUTING): the parser tests run init_process on minimal input snippets (e.g. just epsvmc = 1.0) and relied on config validation never examining the TF geometry; their shared fixture scaffold now sets a valid dr_tf_inboard so the parser tests keep testing parsing. No expected values change.

Behavioural impact

Valid configurations are unaffected (all seven shipped regression inputs pass: they each set ixc 13/140 or dr_tf_inboard, or are stellarator/IFE). The only newly-rejected configurations are ones that previously produced a machine with no inboard TF coil.

Verification

• Full unit suite: 847 passed / 4 skipped; integration suite: 21 passed / 1 skipped (Python 3.12, local).

Found during an independent audit of v3.4.2.

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.47%. Comparing base (986a8f0) to head (1409e50).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4535      +/-   ##
==========================================
- Coverage   49.47%   49.47%   -0.01%     
==========================================
  Files         150      151       +1     
  Lines       30069    30078       +9     
==========================================
+ Hits        14878    14882       +4     
- Misses      15191    15196       +5     

☔ View full report in Codecov by Harness.
📢 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.

[je-cook]

@ukaea/process-model-review this may adversely effect the optimiser as it is currently allowed to search invalid regions of space I think?

I suppose this is only checking init

[mkovari]

Obviously this needs to be fixed - maybe someone who has looked at this code more recently than me - @chris-ashe or someone?

[dallonby]

Init only. check_process is called from exactly one place, init_process (init.py:79), which SingleRun.__init__ runs once (main.py:339 then :432) before the Models objects are even built (main.py:340), so before Scan, SolverHandler or VMCON get anywhere near it. Scans don't re-init between points (scan.py just sets the swept variable), VaryRun spawns a fresh SingleRun per iteration, and dr_tf_inboard only gets written during model evaluation when ixc 140 is active (build.py:1684) or for stellarators, both of which the check excludes.

To be sure I wasn't reasoning my way past something, I instrumented it on the branch: 1 call to check_process vs 26 model evaluations (spherical_tokamak_eval, fsolve), 2029 (large_tokamak_nof, VMCON), 4286 (low_aspect_ratio_DEMO) and 16175 (a 3-point scan). One call every time, before the solve. So the optimiser's freedom to wander through invalid regions is untouched; the check only looks at inputs that are constant for the whole run.

Two things I should say explicitly though. It is a deliberate behaviour change for scaffold inputs that rely on the dr_tf_inboard = 0 default, which is why the test_input.py fixture needed a value. For what it's worth, on main those inputs currently either hit an uncaught ZeroDivisionError (tfcoil/base.py:424, circular case) or, with i_tf_case_geom = 1, report a "feasible solution" with a winding pack of -0.34 m and a negative turn count. And I only covered the superconducting case that was reported; a resistive TF with the same zero goes down the same build.py path, so it could be extended in a follow-up if you'd like.

[timothy-nunn]

I think this condition should be removed because on the first iteration with ixc = 13 the IN.DAT-defined value of dr_tf_inboard is used which would cause the model errors this condition is designed to protect against.

[timothy-nunn]

This would also require a slight change to the error message below.

[timothy-nunn]

Hopefully someone from @ukaea/process-model-review can confirm, but I think we can probably test this condition for superconducting and resistive TF coils. Resistive TF still uses dr_tf_inboard and it would not surprise me if it being 0 or negative would be equally problematic here too.

[timothy-nunn]

I want to avoid having to do specific setup in this test wherever possible. A better option for me is to turn off check_process since its not actually useful for this test.

@pytest.fixture(autouse=True)
def turn_off_check_process(monkeypatch):
    monkeypatch.setattr(init, "check_process", lambda *_: None)

Rebasing to include #4553 (now merged onto main) will also be useful because that will check none of these tests fail because of the above change.

[timothy-nunn]

@je-cook the only impact on the "solver" (on the solution) will be that some users may need to update their initial point if they currently have dr_tf_inboard /= 0 which could change the path/solution.

[dallonby]

Thanks — all four points taken, in a89dcb2 (rebased onto main, so #4553's match= checks run against the fixture change).

One precision on the ixc = 13 case, because it slightly strengthens your change: with dr_tf_inboard = 0 and ixc = 13, the run already failed before any model evaluation — load_iteration_variables computes scale = 1/value, and check_iteration_variable rejects an exactly-zero value with the generic "iteration variable is 0" error. What the exemption really left open is a negative input value: that passes check_iteration_variable (it only tests |v| ≤ 1e-12/NaN/inf), the negative scale inverts the variable's scaled bounds, and the first model evaluation runs with a negative thickness. The check now rejects both at input validation with the actionable message; the message no longer suggests ixc = 13 as a remedy.

The conductor-model condition is removed rather than widened: dr_tf_inboard is only derived under ixc = 140 (Build.calculate_radial_build) or for stellarators, neither of which depends on i_tf_sup, so resistive coils had the identical path. Your check_process monkeypatch is in as written.

Unit 855 passed / 4 skipped; integration 21 passed / 1 skipped — all shipped inputs pass the widened check.

[timothy-nunn]

Sorry that I missed this in my first review, I would prefer that this was parametrize'd rather than using a for loop over bad values

[timothy-nunn]

Suggested change

	"""These are parser tests; configuration validation is not under test."""
	"""These are parser tests; configuration validation is not a part of test."""

[timothy-nunn]

Thanks @dallonby, I think I was actually wrong about the ixc = 13 case because the input parser would reject a negative value. Nevertheless I still think keeping the validation logic as is makes sense as it does no harm.

[timothy-nunn]

Just a change we made yesterday that has not quite filtered through. Otherwise all looks good, thank you for finding this bug and providing a fix!