Skip to content

❄️ Update cryostat docs#3501

Merged
timothy-nunn merged 16 commits intomainfrom
update_cryostat_docs
Jan 31, 2025
Merged

❄️ Update cryostat docs#3501
timothy-nunn merged 16 commits intomainfrom
update_cryostat_docs

Conversation

@chris-ashe
Copy link
Copy Markdown
Collaborator

@chris-ashe chris-ashe commented Jan 25, 2025

Description

This pull request includes several changes to the naming conventions for cryostat-related variables across multiple files, ensuring consistency and clarity in the codebase. The most important changes involve renaming variables and updating documentation to reflect these changes.

🔄 Namespace changes

Variables

  • rdewex -> r_cryostat_inboard
  • rpf2dewar -> dr_pf_cryostat
  • hcryopf -> dz_pf_cryostat
  • zdewex -> z_cryostat_half_inside
  • vdewex -> vol_cryostat
  • ddwex -> dr_cryostat
  • clhsf -> f_z_cryostat
  • clh1 -> dz_tf_cryostat

✨ New additions

  • vol_cryostat_internal: Represents the internal volume of the cryostat

  • cryostat_output(): New function to output the cryostat details in a dedicated function

New output from OUT.dat

image

🐛 Bugs

  • The previous calculation for the volume of the cryostat structure was:
fwbs_variables.vdewex = (
            (2.0 * np.pi * fwbs_variables.rdewex) * 2.0 * fwbs_variables.zdewex
            + (2.0 * np.pi * fwbs_variables.rdewex**2)
        ) * build_variables.ddwex

Which wasnt easy to interpret so a new model for the cryostat structure volume has been introduced which is simply
dividing the cryostat into 2 nested cylinders and taking the internal volume from the bigger one. This caused a 1% change in the calculated volume compared to the tests (810 -> 818 $m^3$)

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 Jan 25, 2025
@chris-ashe chris-ashe added the Documentation Improvements or additions to documentation label Jan 25, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 25, 2025

Codecov Report

❌ Patch coverage is 38.88889% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.84%. Comparing base (c145648) to head (17d23bf).
⚠️ Report is 525 commits behind head on main.

Files with missing lines Patch % Lines
process/build.py 8.33% 11 Missing ⚠️
process/io/plot_proc.py 0.00% 5 Missing ⚠️
process/stellarator.py 0.00% 4 Missing ⚠️
process/geometry/cryostat_geometry.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3501      +/-   ##
==========================================
+ Coverage   30.83%   30.84%   +0.01%     
==========================================
  Files          80       80              
  Lines       19322    19325       +3     
==========================================
+ Hits         5958     5961       +3     
  Misses      13364    13364              

☔ 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 update_cryostat_docs branch from ff8c516 to 327bb37 Compare January 27, 2025 15:14
@chris-ashe chris-ashe requested a review from j-a-foster January 27, 2025 15:20
@chris-ashe chris-ashe marked this pull request as ready for review January 28, 2025 14:04
@chris-ashe chris-ashe force-pushed the update_cryostat_docs branch from fd8350f to 33eec1e Compare January 28, 2025 14:06
@chris-ashe chris-ashe changed the title Update cryostat docs ❄️ Update cryostat docs Jan 28, 2025
Copy link
Copy Markdown
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.

Docs look good.

…tion and cryostat volume section for improved clarity
@chris-ashe chris-ashe force-pushed the update_cryostat_docs branch from 33eec1e to 17d23bf Compare January 31, 2025 09:21
Copy link
Copy Markdown
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.

Regression test failures are acceptable - only constraint variables have changed significantly. Happy for this to be merged.

ajpearcey pushed a commit that referenced this pull request Feb 26, 2025
* 🔄Rename variable rdewex to r_cryostat_inboard for clarity and consistency

* 🔄 Rename variable rpf2dewar to dr_pf_cryostat for clarity and consistency

* 🔄 Rename variable hcryopf to dz_pf_cryostat for clarity and consistency

* 🔄 Rename variable zdewex to dz_cryostat_half_inside for clarity and consistency

* 🔄 Rename variable vdewex to vol_cryostat for clarity and consistency

* 🔄 Rename variable ddwex to dr_cryostat for clarity and consistency

* 🔄 Rename variable dz_cryostat_half_inside to z_cryostat_half_inside for clarity and consistency

* 🎨 Refactor external_cryo_geometry method for improved clarity and documentation

* 🔄 Rename variable clhsf to f_z_cryostat for clarity and consistency in documentation and code

* 🔄 Rename variable clh1 to dz_tf_cryostat for clarity and consistency in documentation and code

* 🎨 Update cryostat documentation to include vertical clearance calculation and cryostat volume section for improved clarity

* 🎨 Enhance cryostat documentation and comments for volume calculation clarity

* 🧪 Update expected values in ExternalCryoGeometryParam test for accuracy

* ❇️ Add `vol_cryostat_internal` to represent the void space inside the cryostat.

* 🎨 Add cryostat_output method to output cryostat geometry details to the output file

* 🔥 Remove redundant output variables from CCFE_HCPB class for cleaner code
@je-cook je-cook deleted the update_cryostat_docs 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

Cryostat Documentation Improvements or additions to documentation New Variables Variable rename

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants