Skip to content

i_confinement_time ITPA20-IL Scaling equation fix#4158

Open
OceanNuclear wants to merge 2 commits intomainfrom
ocean/pc-4017
Open

i_confinement_time ITPA20-IL Scaling equation fix#4158
OceanNuclear wants to merge 2 commits intomainfrom
ocean/pc-4017

Conversation

@OceanNuclear
Copy link
Copy Markdown
Contributor

Description

Closes #4017
reference [^23] is dereferenced into [^22].

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.

@OceanNuclear OceanNuclear requested a review from a team as a code owner March 31, 2026 18:37
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.44%. Comparing base (6980eb9) to head (2d82030).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4158      +/-   ##
==========================================
+ Coverage   47.97%   49.44%   +1.47%     
==========================================
  Files         143      149       +6     
  Lines       30125    29796     -329     
==========================================
+ Hits        14451    14734     +283     
+ Misses      15674    15062     -612     

☔ 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.

@OceanNuclear
Copy link
Copy Markdown
Contributor Author

Before the PR, the documentation page looks like:
image
The top reference ([22]) shows this equation:
image
matching the top equation. It also shows the bottom equation:
image

However, the bottom reference ([23]) shows this equation:
image
But the [3] here is actually citing the first paper ([22]). This also explains the index rounding issue mentioned in #4017.

@OceanNuclear
Copy link
Copy Markdown
Contributor Author

According to reference [22], the values (and the errors) for the top equation is:
image

and for the bottom equation is:
image

@OceanNuclear OceanNuclear marked this pull request as draft March 31, 2026 18:56
@OceanNuclear OceanNuclear changed the title i_confinement_time ITPA20-IL Scaling equation fix i_confinement_time ITPA20-IL Scaling equation fix Mar 31, 2026
@OceanNuclear
Copy link
Copy Markdown
Contributor Author

Regarding making the roundings more consistent, these are the values (α) and their uncertainties (σ) as recorded in the first paper ([22]):

ITPA20 scaling (H-mode) $\alpha_0$ $\alpha_I$ $\alpha_B$ $\alpha_n$ $\alpha_P$ $\alpha_R$ $\alpha_{1+\delta}$ $\alpha_\kappa$ $\alpha_\epsilon$ $\alpha_M$
Mean 0.0534 0.976 0.218 0.2442 -0.6687 1.71 0.362 0.799 0.354 0.195
σ 0.0013 0.015 0.013 0.009 0.0063 0.023 0.033 0.027 0.031 0.016
Mean + 0.5 σ 0.05405 0.9835 0.2245 0.2487 -0.66555 1.7215 0.3785 0.8125 0.3695 0.203
Mean – 0.5 σ 0.05275 0.9685 0.2115 0.2397 -0.67185 1.6985 0.3455 0.7855 0.3385 0.187
Best rounding 0.053 0.98 0.22 0.24 -0.67 1.7 0.36 0.8 0.35 0.2
ITPA20-IL scaling (H-mode) $\alpha_0$ $\alpha_I$ $\alpha_B$ $\alpha_n$ $\alpha_P$ $\alpha_R$ $\alpha_{1+\delta}$ $\alpha_\kappa$ $\alpha_\epsilon$ $\alpha_M$
Mean 0.067 1.291 -0.134 0.1473 -0.6442 1.194 0.56 0.673 0 0.302
σ 0.0021 0.017 0.017 0.0088 0.0063 0.027 0.032 0.039 0.033 0.016
Mean + 0.5 σ 0.06805 1.2995 -0.1255 0.1517 -0.64105 1.2075 0.576 0.6925 0.0165 0.31
Mean – 0.5 σ 0.06595 1.2825 -0.1425 0.1429 -0.64735 1.1805 0.544 0.6535 -0.0165 0.294
Best rounding 0.067 1.29 -0.13 0.15 -0.644 1.2 0.56 0.67 0 0.3

@OceanNuclear
Copy link
Copy Markdown
Contributor Author

@stuartmuldrew can you identify what the (+0059 - 0.032) is doing in the first term of the top equation, and the (+0.030 - 0.018) in the first term of the bottom equation? I can't see these being the upper and lower bounds of that constant

@OceanNuclear OceanNuclear marked this pull request as ready for review April 9, 2026 21:27
@timothy-nunn
Copy link
Copy Markdown
Collaborator

@jonmaddock have you approved this as a maintainer as ready to be merged? Also are you managing this PR and should I put you as the assignee?

@stuartmuldrew
Copy link
Copy Markdown
Collaborator

@stuartmuldrew can you identify what the (+0059 - 0.032) is doing in the first term of the top equation, and the (+0.030 - 0.018) in the first term of the bottom equation? I can't see these being the upper and lower bounds of that constant

My interpretation from the equation and the table is that is the error on the constant at the start. Admittedly, that is a pretty large upper bound.

@OceanNuclear
Copy link
Copy Markdown
Contributor Author

OceanNuclear commented Apr 13, 2026

Thanks @stuartmuldrew. I think those values contradict with the actual upper and lower bound I calculated using the values given in the table (tabulated in the previous comment). Perhaps the (+0.059 - 0.032) and (+0.030 - 0.018) will remain a mystery forever. It doesn't affect anything above anyways.

As for the failing regression test, that's due to the actual constants being changed (due to correction in number of digits rounded). Shall we change the regression test so that it passes then? @timothy-nunn

@timothy-nunn timothy-nunn requested a review from jonmaddock April 14, 2026 16:12
@je-cook je-cook added the Physics Relating to the physics models label Apr 20, 2026
Copy link
Copy Markdown
Contributor

@jonmaddock jonmaddock left a comment

Choose a reason for hiding this comment

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

I don't understand why certain coefficients are being rounded to a lower accuracy: can you explain?

@OceanNuclear
Copy link
Copy Markdown
Contributor Author

@stuartmuldrew Asked for a more consistent rounding in the issue #4017. The reduced accuracy reflects the lower confidence/higher uncertainty.

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

Labels

Physics Relating to the physics models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect Reference for ITPA20-IL Scaling

6 participants