8000 🐛 Refine vertical build calc by chris-ashe · Pull Request #3710 · ukaea/PROCESS · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

🐛 Refine vertical build calc #3710

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

🐛 Refine vertical build calc #3710

wants to merge 2 commits into from
+97 −55

Conversation

chris-ashe
Copy link
Collaborator
@chris-ashe chris-ashe commented Jul 7, 2025 •
edited
Loading

This pull request refactors the calculate_vertical_build method in process/build.py to improve readability and maintainability by replacing the variable vbuild with vertical_build_upper. Additionally, it updates the logic to use more descriptive variable names and modifies some calculations related to plasma and structure dimensions.

Refactoring for readability:

  • Replaced the variable vbuild with vertical_build_upper throughout the method to improve clarity and better align with the variable's purpose. This change affects multiple calculations and assignments, ensuring the new variable name is consistently used.

🐛 Adjustments to plasma-related calculations:

  • 🐛 Updated the calculation for the top scrape-off thickness by replacing dz_fw_plasma_gap with dz_xpoint_divertor, reflecting a change in the model's approach to scrape-off layer dimensions. [1] [2]

Improvements to structure dimension calculations:

  • Modified the calculation for the total height of the TF coil (tf_height) to use the new vertical_build_upper variable, ensuring consistency in the updated logic.
  • Adjusted the calculation of build_variables.tfoffset to use vertical_build_upper instead of vbuild, aligning with the refactored variable naming.

Single null case logic updates:

  • Added logic for the single null case, including calculations for dz_vv_upper and dz_fw_upper based on averages of upper and lower dimensions, improving the handling of single null configurations.## 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 self-assigned this Jul 7, 2025
@chris-ashe chris-ashe added Bug Something isn't working Machine Build labels Jul 7, 2025
@codecov-commenter
Copy link
codecov-commenter commented Jul 7, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 28 lines in your changes missing coverage. Please review.

Project coverage is 37.62%. Comparing base (f0ffc3a) to head (e85b5c7).

Files with missing lines Patch % Lines
process/build.py 0.00% 28 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3710      +/-   ##
==========================================
- Coverage   37.63%   37.62%   -0.01%     
==========================================
  Files          88       88              
  Lines       22827    22830       +3     
==========================================
  Hits         8590     8590              
- Misses      14237    14240       +3

☔ 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 refine_vertical_build_calc branch 2 times, most recently from 4a9623f to 1d5fa3d Compare July 10, 2025 14:37
@chris-ashe chris-ashe force-pushed the refine_vertical_build_calc branch from 1d5fa3d to e85b5c7 Compare July 10, 2025 14:57
@chris-ashe chris-ashe requested a review from j-a-foster July 11, 2025 12:53
@chris-ashe chris-ashe marked this pull request as ready for review July 11, 2025 12:54
j-a-foster
j-a-foster approved these changes Jul 11, 2025
View reviewed changes
Copy link
Collaborator
@j-a-foster j-a-foster left a comment

Choose a reason for hiding this comment

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

Happy with changes.

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

@j-a-foster j-a-foster j-a-foster approved these changes

Assignees

@chris-ashe chris-ashe

Labels
Bug Something isn't working Machine Build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chris-ashe @codecov-commenter @j-a-foster
0