Skip to content

Correct hstar being stored on data structure#4396

Open
timothy-nunn wants to merge 2 commits into
mainfrom
hstar-saved-incorrectly
Open

Correct hstar being stored on data structure#4396
timothy-nunn wants to merge 2 commits into
mainfrom
hstar-saved-incorrectly

Conversation

@timothy-nunn

Copy link
Copy Markdown
Collaborator

I think hstar is being incorrectly set on the data structure in calculate_confinement_time. calculate_confinement_time is also called when creating the H-factor table for the OUT.DAT. I believe that the hstar from this is what was being written to the MFILE, not the hstar from the selected confinement scaling.

@codecov-commenter

codecov-commenter commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.80%. Comparing base (e24884c) to head (22a9482).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
process/models/physics/confinement_time.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4396      +/-   ##
==========================================
- Coverage   48.84%   48.80%   -0.04%     
==========================================
  Files         151      151              
  Lines       29377    29348      -29     
==========================================
- Hits        14348    14323      -25     
+ Misses      15029    15025       -4     

☔ View full report in Codecov by Harness.
📢 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.

@timothy-nunn timothy-nunn marked this pull request as ready for review June 29, 2026 15:15
@timothy-nunn timothy-nunn requested a review from a team as a code owner June 29, 2026 15:15
@je-cook je-cook added the Data structure Issues related to the data structure label Jun 30, 2026

@chris-ashe chris-ashe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just really needs a way to be able to take the loss power terms from whatever equation is being used as the value is hardcoded for a single scaling at the moment

Comment thread process/models/physics/confinement_time.py Outdated
@@ -1049,7 +1055,7 @@ def calculate_confinement_time(
** 0.31

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will only work for the selected scaling if the loss power index is (1-0.31), so really just the IPB98 scalings in this case

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This will need to be turned into an issue. This PR is just stopping hstar being incorrectly set every time calculate_confinement_time is called

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

Labels

Data structure Issues related to the data structure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants