Skip to content

Conversation

@chris-ashe
Copy link
Collaborator

@chris-ashe chris-ashe commented Jan 23, 2026

This pull request makes significant changes to the blanket and shield modeling in process/blanket_library.py, primarily removing the calculation and handling of shield and vacuum vessel components from the blanket library. The shield and vacuum vessel are now modeled separately, as evidenced by their initialization and execution in process/main.py and process/caller.py. This refactoring simplifies the blanket library, making it responsible only for blanket calculations.

Key changes include:

Blanket Library Refactor

  • Removed all logic related to shield and vacuum vessel calculations from blanket_library.py, including their half-height, surface area, and volume calculations. The blanket library now exclusively handles blanket components.

Separation of Shield and Vacuum Vessel Models

  • Added new model classes for the shield and vacuum vessel (Shield, VacuumVessel) and initialized them in the main process class (process/main.py).

  • Modified the process caller to run the shield and vacuum vessel models as independent steps, decoupled from the blanket calculations.

These changes improve the modularity and maintainability of the code by clearly separating the responsibilities of the blanket, shield, and vacuum vessel models.## 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.

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2026

Codecov Report

❌ Patch coverage is 46.23656% with 100 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.48%. Comparing base (89a0b72) to head (6280dc6).

Files with missing lines Patch % Lines
process/vacuum.py 26.92% 76 Missing ⚠️
process/shield.py 72.30% 18 Missing ⚠️
process/blanket_library.py 81.81% 2 Missing ⚠️
process/caller.py 0.00% 2 Missing ⚠️
process/main.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4064      +/-   ##
==========================================
+ Coverage   46.46%   46.48%   +0.01%     
==========================================
  Files         122      123       +1     
  Lines       28838    28884      +46     
==========================================
+ Hits        13401    13427      +26     
- Misses      15437    15457      +20     

☔ 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 create_shield_class branch 5 times, most recently from d09316b to 8181f19 Compare January 29, 2026 11:54
@chris-ashe chris-ashe marked this pull request as ready for review January 29, 2026 16:08
@chris-ashe chris-ashe requested a review from a team as a code owner January 29, 2026 16:08
Copy link
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

Mainly style comments.

My concerns about call order in #4002 have been addressed other than I have now noticed the cryostat calculates stuff related to the VV that is used in blanket library, but its called after the blanket!?

Please close #4002 and rename this PR appropriately

@timothy-nunn timothy-nunn self-assigned this Jan 29, 2026
@timothy-nunn timothy-nunn changed the title ♻️ Create shield class Separate shield, vacuum vessel, and blanket calculations Jan 30, 2026
Copy link
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

I still think that these new models need to be added into output.py (essentially a copy of the caller that ensures output is written to the MFile)

return vol_vv_inboard, vol_vv_outboard, vol_vv

@staticmethod
def calculate_elliptical_vessel_volumes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Docstring

@chris-ashe
Copy link
Collaborator Author

I still think that these new models need to be added into output.py (essentially a copy of the caller that ensures output is written to the MFile)

I feel at this time it would be best to leave this until I have all of the systems properly separated out and we know that nothing is doing double calcs etc. Still need to do a few more PR's to sort out what goes in the blanket base class and what goes into the distinct models. Then we can start making things properly like plot_generic_blanket_info, plot_outboard_blanket_info etc

@timothy-nunn
Copy link
Collaborator

I still think that these new models need to be added into output.py (essentially a copy of the caller that ensures output is written to the MFile)

I feel at this time it would be best to leave this until I have all of the systems properly separated out and we know that nothing is doing double calcs etc. Still need to do a few more PR's to sort out what goes in the blanket base class and what goes into the distinct models. Then we can start making things properly like plot_generic_blanket_info, plot_outboard_blanket_info etc

This issue is that output.py essentially runs PROCESS again. So we could end up with variables being out of date and changing results if the models aren't run here

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants