Skip to content

Add supercon tf wp shape types#4194

Merged
je-cook merged 9 commits intomainfrom
add_supercon_tf_wp_shape_types
Apr 20, 2026
Merged

Add supercon tf wp shape types#4194
je-cook merged 9 commits intomainfrom
add_supercon_tf_wp_shape_types

Conversation

@chris-ashe
Copy link
Copy Markdown
Collaborator

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 TF Coil Toroidal field coil Refactor labels Apr 16, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 70.96774% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.10%. Comparing base (d43ba2a) to head (59ac2a1).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
process/models/tfcoil/superconducting.py 61.90% 8 Missing ⚠️
process/core/init.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4194      +/-   ##
==========================================
+ Coverage   49.62%   52.10%   +2.47%     
==========================================
  Files         148      148              
  Lines       29969    30389     +420     
==========================================
+ Hits        14873    15835     +962     
+ Misses      15096    14554     -542     

☔ 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 force-pushed the add_supercon_tf_wp_shape_types branch from 60e91f8 to 662879e Compare April 17, 2026 09:06
@chris-ashe chris-ashe marked this pull request as ready for review April 17, 2026 09:07
@chris-ashe chris-ashe requested a review from a team as a code owner April 17, 2026 09:07
Copilot AI review requested due to automatic review settings April 17, 2026 09:07
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 introduces an explicit enum for superconducting TF winding-pack (WP) shape selection and starts using it in TF WP/case geometry logic and ripple calculations, replacing raw integer “magic values”.

Changes:

  • Added SuperconductingTFWPShapeType (IntEnum) to represent WP shape types (rectangular, double rectangular, trapezoidal).
  • Updated superconducting TF WP and case geometry branching to compare against enum members instead of integers.
  • Updated build and init logic to use the enum for i_tf_wp_geom comparisons/default assignment.

Reviewed changes

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

File Description
process/models/tfcoil/superconducting.py Adds the WP shape enum and uses it in WP/case geometry branching.
process/models/build.py Uses the new enum when selecting WP geometry-dependent parameters for ripple calculation.
process/core/init.py Sets default i_tf_wp_geom using the new enum and updates a validation check to compare against the enum.

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

Comment thread process/models/tfcoil/superconducting.py
Comment thread process/models/tfcoil/superconducting.py
Comment on lines 1123 to 1126
# Trapezoidal WP
# --------------
else:
elif i_tf_wp_geom == SuperconductingTFWPShapeType.TRAPEZOIDAL:
# Thickness of winding pack section at r_tf_wp_inboard_outer [m]
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

The switch on i_tf_wp_geom no longer has a final else branch. If i_tf_wp_geom is ever an unexpected value (e.g. the default -1 before check_process runs), none of the geometry/area variables will be assigned and the later area check/return will raise UnboundLocalError. Add an explicit fallback (e.g. raise ProcessValueError/ProcessValidationError with allowed values, or map unknown values to a default) to keep behavior safe and diagnosable.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread process/models/tfcoil/superconducting.py Outdated
Comment thread process/models/tfcoil/superconducting.py
Comment thread process/models/build.py
chris-ashe and others added 2 commits April 17, 2026 17:08
Co-authored-by: je-cook <81617086+je-cook@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@je-cook je-cook left a comment

Choose a reason for hiding this comment

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

Please also do my previous review comments

Comment thread process/models/tfcoil/superconducting.py Outdated
@je-cook je-cook self-assigned this Apr 20, 2026
…nt and update geometry handling in Build class
@chris-ashe chris-ashe requested a review from je-cook April 20, 2026 08:25
Comment thread process/models/tfcoil/superconducting.py Outdated
Comment thread process/core/init.py Outdated
@je-cook je-cook enabled auto-merge (squash) April 20, 2026 10:48
@je-cook je-cook merged commit b9e97d7 into main Apr 20, 2026
10 of 11 checks passed
@je-cook je-cook deleted the add_supercon_tf_wp_shape_types branch April 20, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor TF Coil Toroidal field coil

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants