Skip to content

♻️ Refactor tf coil structure#3606

Merged
clmould merged 19 commits intomainfrom
refactor_tf_coil_structure
Apr 7, 2025
Merged

♻️ Refactor tf coil structure#3606
clmould merged 19 commits intomainfrom
refactor_tf_coil_structure

Conversation

@chris-ashe
Copy link
Copy Markdown
Collaborator

@chris-ashe chris-ashe commented Mar 26, 2025

This pull request is focussed around creating new classes for the the different kinds of TF coil configurations and have seperate callable instances of those models.

There is now a TFCoil class that holds the generic methods that all types of TF coils. Most of the TF coil calculations were split between sctfcoil.py and tfcoil.py with not much structure or care. With even some resistive calculations being in the superconducting file....

A new resisitive_tf_coil.py file exists with a new ResisitiveTFCoil(TFCoil) class that inherits from TFCoil the same is true for SuperconductingTFCoil(TFCoil).

It is hoped for future PR's that the different run methods can be used with super() and the output seperated from the functions. This will be done in other PR's to prevent this one becoming too large.

  • process/caller.py: Added conditional execution of the resistive toroidal field coil model based on i_tf_sup variable and added a call to portsz method from the build model.

Class name updates for consistency:

  • process/main.py: Updated imports to use ResistiveTFCoil and SuperconductingTFCoil instead of Sctfcoil and TFcoil.
  • process/main.py: Updated the __init__ method to initialize ResistiveTFCoil and SuperconductingTFCoil instances.
  • tests/unit/test_vacuum.py: Updated the docstring to reflect the new class name TFCoil.## 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 Mar 26, 2025
@chris-ashe chris-ashe self-assigned this Mar 26, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 26, 2025

Codecov Report

Attention: Patch coverage is 34.61538% with 119 lines in your changes missing coverage. Please review.

Project coverage is 31.01%. Comparing base (0bef5e6) to head (ef57263).
Report is 120 commits behind head on main.

Files with missing lines Patch % Lines
process/resistive_tf_coil.py 34.91% 110 Missing ⚠️
process/caller.py 0.00% 3 Missing ⚠️
process/main.py 50.00% 3 Missing ⚠️
process/output.py 0.00% 2 Missing ⚠️
process/io/plot_proc.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3606      +/-   ##
==========================================
- Coverage   31.05%   31.01%   -0.05%     
==========================================
  Files          86       87       +1     
  Lines       20350    20412      +62     
==========================================
+ Hits         6319     6330      +11     
- Misses      14031    14082      +51     

☔ 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 refactor_tf_coil_structure branch from 3532617 to de27ce0 Compare March 26, 2025 16:43
@chris-ashe chris-ashe requested review from Copilot and removed request for Copilot March 27, 2025 10:55
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 refactors the TF coil structure to improve naming consistency and separate the implementation of superconducting and resistive models.
  • Key changes include updating class names and imports in main.py, modifying model invocations in caller.py, and adjusting test documentation accordingly.

Reviewed Changes

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

File Description
tests/unit/test_vacuum.py Updated fixture parameter docstring to use standardized "TFCoil" capitalization.
process/main.py Renamed and updated TF coil classes and added the resistive TF coil instantiation.
process/caller.py Modified TF coil model calls to conditionally run the resistive model and update output.
Comments suppressed due to low confidence (2)

tests/unit/test_vacuum.py:29

  • [nitpick] Ensure consistent naming for TF coil objects across the code by using 'TFCoil' uniformly, as reflected in this diff.
:param tfcoil: fixture containing an initialised `TFCoil` object

process/caller.py:267

  • Potential double invocation of tfcoil.run() when ft.tfcoil_variables.i_tf_sup equals 1; clarify whether the call with output=False and the subsequent call are both intended.
self.models.tfcoil.run(output=False)

@chris-ashe chris-ashe force-pushed the refactor_tf_coil_structure branch 4 times, most recently from d6c97d3 to a2d242e Compare March 28, 2025 13:22
@chris-ashe chris-ashe requested a review from timothy-nunn March 28, 2025 13:23
@chris-ashe chris-ashe changed the title 🚧 Refactor tf coil structure ♻️ Refactor tf coil structure Mar 28, 2025
@chris-ashe chris-ashe marked this pull request as ready for review March 28, 2025 13:23
@chris-ashe chris-ashe requested review from clmould and Copilot April 2, 2025 07:30
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 pull request refactors the TF coil structure by introducing distinct classes for resistive and superconducting configurations, consolidating related methods under the new TFCoil hierarchy.

  • Refactored imports and class initializations in process/main.py
  • Updated conditional logic for executing resistive and superconducting TF coil models in process/output.py and process/caller.py
  • Modified test documentation and plotting utilities to reflect the updated class names

Reviewed Changes

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

Show a summary per file
File Description
tests/unit/test_vacuum.py Updated docstring to reflect the new TFCoil class naming
process/output.py Condition now selects the resistive model when appropriate
process/main.py Updated imports and instance creation for the new TF coil classes
process/io/plot_proc.py Changed constant used in plotting to reference superconducting types
process/caller.py Adjusted model calls to conform with the refactored TF coil structure
Comments suppressed due to low confidence (2)

process/main.py:661

  • [nitpick] Consider renaming 'sctfcoil' to 'superconducting_tf_coil' for improved clarity and consistency with the updated class naming.
self.sctfcoil = SuperconductingTFCoil()

process/output.py:68

  • The branch for the superconducting TF coil does not call an output or run method. If an output action is expected for superconducting coils similar to the resistive branch, please add the appropriate method call.
if ft.tfcoil_variables.i_tf_sup == 1:

@chris-ashe chris-ashe removed the request for review from timothy-nunn April 2, 2025 08:35
@chris-ashe chris-ashe force-pushed the refactor_tf_coil_structure branch from a2d242e to 0fcf6dc Compare April 7, 2025 10:55
@chris-ashe chris-ashe force-pushed the refactor_tf_coil_structure branch from 0fcf6dc to ef57263 Compare April 7, 2025 12:07
@clmould clmould merged commit 931da77 into main Apr 7, 2025
18 checks passed
@je-cook je-cook deleted the refactor_tf_coil_structure branch January 5, 2026 15:05
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