Skip to content

♻️ Refactor some radiation methods for clarity - #4219

Merged
timothy-nunn merged 18 commits into
mainfrom
line_brem_profile_bug
Jul 27, 2026
Merged

♻️ Refactor some radiation methods for clarity#4219
timothy-nunn merged 18 commits into
mainfrom
line_brem_profile_bug

Conversation

@chris-ashe

Copy link
Copy Markdown
Collaborator

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 added Radiation Plasma radiated power Bug Something isnt working labels May 5, 2026
@codecov-commenter

codecov-commenter commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.06452% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.63%. Comparing base (ef96cb5) to head (9a40cd6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
process/models/physics/impurity_radiation.py 58.62% 12 Missing ⚠️
process/models/physics/physics.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4219      +/-   ##
==========================================
- Coverage   48.64%   48.63%   -0.01%     
==========================================
  Files         151      151              
  Lines       29703    29705       +2     
==========================================
- Hits        14449    14448       -1     
- Misses      15254    15257       +3     

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

@chris-ashe
chris-ashe force-pushed the line_brem_profile_bug branch from 439cbc9 to 4c7c3ce Compare May 28, 2026 14:09
@chris-ashe
chris-ashe force-pushed the line_brem_profile_bug branch from 03ddd71 to e4e975a Compare June 17, 2026 10:17
@chris-ashe chris-ashe changed the title 🐛 Line brem profile bug ♻️ Refactor some radiation methods for clarity Jul 10, 2026
@chris-ashe chris-ashe added Refactor and removed Bug Something isnt working labels Jul 10, 2026
@chris-ashe
chris-ashe marked this pull request as ready for review July 10, 2026 13:22
@chris-ashe
chris-ashe requested a review from a team as a code owner July 10, 2026 13:22
@chris-ashe
chris-ashe requested a review from timothy-nunn July 10, 2026 14:40
@timothy-nunn timothy-nunn self-assigned this Jul 10, 2026
Comment thread process/models/physics/impurity_radiation.py Outdated
Comment on lines +477 to 478
indices = np.digitize(temp_electron_kev, bins)
indices[indices >= bins.shape[0]] = bins.shape[0] - 1

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.

temp_electron_kev is typed as allowed to be a float but this function would fail here because indices will be a np.int64 which you cannot do item assignment to

Comment on lines +517 to +518
nd_electron_profile: np.array | float,
temp_electron_profile_kev: np.array | float,

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.

Again, I don't think these will work as floats because indexing wont work

@chris-ashe
chris-ashe requested a review from timothy-nunn July 10, 2026 15:03
Comment on lines +380 to +388
def z2index(zimp, data: DataStructure) -> int:
for i in range(len(data.impurity_radiation.impurity_arr_label)):
if zimp == data.impurity_radiation.impurity_arr_z[i]:
return i

# Should only get here if there is a problem
raise ProcessValueError(
"Element with the given charge is not in the impurity array", zimp=zimp
)

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.

I can't see this function being used anywhere

Comment thread process/models/physics/impurity_radiation.py Outdated
@chris-ashe
chris-ashe requested a review from timothy-nunn July 24, 2026 10:46
Comment on lines 448 to 457

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.

Can you also add : to the other arguments and the solve the merge conflicts

chris-ashe and others added 12 commits July 24, 2026 16:46
…nd improve comments

Co-authored-by: Copilot <copilot@github.com>
…n imprad_profile method

Co-authored-by: Copilot <copilot@github.com>
… accuracy in parameter types and documentation

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
…mathematical notation

Co-authored-by: Copilot <copilot@github.com>
…ions

Co-authored-by: Copilot <copilot@github.com>
@chris-ashe
chris-ashe force-pushed the line_brem_profile_bug branch from f6ed7f3 to de4ec34 Compare July 24, 2026 15:46
@chris-ashe
chris-ashe requested a review from timothy-nunn July 24, 2026 15:50
@timothy-nunn
timothy-nunn merged commit 7eb4025 into main Jul 27, 2026
10 of 11 checks passed
@timothy-nunn
timothy-nunn deleted the line_brem_profile_bug branch July 27, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Radiation Plasma radiated power Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Incorrect calculation of line and Bremsstrahlung plasma radiation components

3 participants