Conversation
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
60e91f8 to
662879e
Compare
There was a problem hiding this comment.
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_geomcomparisons/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.
| # Trapezoidal WP | ||
| # -------------- | ||
| else: | ||
| elif i_tf_wp_geom == SuperconductingTFWPShapeType.TRAPEZOIDAL: | ||
| # Thickness of winding pack section at r_tf_wp_inboard_outer [m] |
There was a problem hiding this comment.
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: je-cook <81617086+je-cook@users.noreply.github.com>
…ing for invalid geometry index
je-cook
left a comment
There was a problem hiding this comment.
Please also do my previous review comments
…nt and update geometry handling in Build class
…ity in geometry handling
… improved clarity and consistency
Description
Checklist
I confirm that I have completed the following checks: