-
Notifications
You must be signed in to change notification settings - Fork 17
💨 ♻️ Create Vacuum Vessel class #4002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4002 +/- ##
==========================================
+ Coverage 46.49% 46.51% +0.01%
==========================================
Files 123 123
Lines 28777 28800 +23
==========================================
+ Hits 13381 13397 +16
- Misses 15396 15403 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c30853a to
ad32540
Compare
ad32540 to
790a52c
Compare
c3a8019 to
7869c1a
Compare
5791a40 to
4c8f2d7
Compare
j-a-foster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with transfer of code.
process/vacuum.py
Outdated
| + dr_fw_inboard | ||
| + dr_fw_outboard | ||
| ) | ||
| z_top = z_top + dz_blkt_upper + dz_shld_upper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this was inside the if-else statement
PROCESS/process/blanket_library.py
Lines 116 to 133 in ac78fdb
| if divertor_variables.n_divertors == 2: | |
| htop = hbot | |
| else: | |
| # Blanket | |
| htop = build_variables.z_plasma_xpoint_upper + 0.5 * ( | |
| build_variables.dr_fw_plasma_gap_inboard | |
| + build_variables.dr_fw_plasma_gap_outboard | |
| + build_variables.dr_fw_inboard | |
| + build_variables.dr_fw_outboard | |
| ) | |
| # Shield | |
| if icomponent == 1: | |
| htop = htop + build_variables.dz_blkt_upper | |
| # Vacuum Vessel | |
| if icomponent == 2: | |
| htop = ( | |
| htop + build_variables.dz_blkt_upper + build_variables.dz_shld_upper | |
| ) |
602d8e6 to
05778f2
Compare
17cecdf to
9123ecf
Compare
| if d_vv_all > 1.0e-6: | ||
| ccfe_hcpb_module.vv_density = fwbs_variables.m_vv / fwbs_variables.vol_vv | ||
| else: | ||
| ccfe_hcpb_module.vv_density = 0.0 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is causing ccfe_hcpb_module.vv_density to equal 0 in the unit test. You should mock this now that it has been moved into the vacuum class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although, if this is not being calculated here we need to check the call order to ensure its being calculated before this method is called
| # changes in the same location. | ||
| fwbs_variables.vol_vv = fwbs_variables.fvoldw * fwbs_variables.vol_vv | ||
|
|
||
| ccfe_hcpb_module.vv_density = fwbs_variables.m_vv / fwbs_variables.vol_vv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this was
Lines 477 to 480 in ac78fdb
| if d_vv_all > 1.0e-6: | |
| ccfe_hcpb_module.vv_density = fwbs_variables.m_vv / fwbs_variables.vol_vv | |
| else: | |
| ccfe_hcpb_module.vv_density = 0.0 |
Why has the
if statement been removed?
… tests for elliptical vessel volumes
9123ecf to
e562fa8
Compare
This pull request refactors the calculation and management of vacuum vessel geometry and volumes, moving these responsibilities out of the
blanket_libraryand into a new dedicatedVacuumVesselclass. The vacuum vessel logic is now encapsulated and run independently, leading to a cleaner separation of concerns and easier maintenance. Related test code and obsolete code paths have also been updated or removed to match the new structure.Refactoring and Encapsulation of Vacuum Vessel Calculations:
Introduced a new
VacuumVesselclass inprocess/vacuum.pythat handles all vacuum vessel geometry and volume calculations, including methods for D-shaped and elliptical vessel geometries, and applies the vessel coverage factor and density calculation. The vacuum vessel logic is now invoked viaself.models.vacuum_vessel.run()in the main model runner.Removed all vacuum vessel-related calculations from
blanket_library.py, including volume and half-height computations, as well as code that previously handled the vacuum vessel as a third component (icomponent==2). The blanket and shield calculations now only consider two components.Test Code Updates:
test_blanket_library.pythat referenced vacuum vessel geometry or volumes, since these are now handled by the newVacuumVesselclass and are no longer part of the blanket library interface.Other Code Cleanups:
hcpb.py, as this is now handled in the new class.This refactor improves code maintainability by isolating vacuum vessel logic, reducing coupling between the blanket library and vessel calculations, and ensuring that each component is responsible for its own domain.## Description
Checklist
I confirm that I have completed the following checks: