Skip to content

Create diamagnetic current class#4156

Open
chris-ashe wants to merge 5 commits intomainfrom
create_diamagnetic_current_class
Open

Create diamagnetic current class#4156
chris-ashe wants to merge 5 commits intomainfrom
create_diamagnetic_current_class

Conversation

@chris-ashe
Copy link
Copy Markdown
Collaborator

@chris-ashe chris-ashe commented Mar 31, 2026

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 requested a review from a team as a code owner March 31, 2026 15:12
@chris-ashe chris-ashe added Physics Relating to the physics models Refactor labels Mar 31, 2026
@chris-ashe chris-ashe force-pushed the create_diamagnetic_current_class branch from 59f1dbb to 9355159 Compare March 31, 2026 15:14
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 31, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 69.38776% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.00%. Comparing base (5960a7f) to head (387eb16).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
process/models/physics/plasma_current.py 67.44% 14 Missing ⚠️
process/models/physics/physics.py 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4156      +/-   ##
==========================================
- Coverage   48.02%   48.00%   -0.03%     
==========================================
  Files         143      143              
  Lines       30088    30157      +69     
==========================================
+ Hits        14451    14477      +26     
- Misses      15637    15680      +43

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

@timothy-nunn timothy-nunn self-assigned this Apr 1, 2026
timothy-nunn
timothy-nunn requested changes Apr 1, 2026
View reviewed changes
process/models/physics/plasma_current.py Outdated
return self._full_name_


class PlasmaDiamagneticCurrent:
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 1, 2026

Choose a reason for hiding this comment

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

Can you implement Model here please

process/models/physics/plasma_current.py Outdated
)

@staticmethod
@nb.jit(nopython=True, cache=True)
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 1, 2026

Choose a reason for hiding this comment

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

Suggested change
@nb.jit(nopython=True, cache=True)
@nb.njit(cache=True)

Trying to move to this style of decorator for numba

process/models/physics/plasma_current.py Outdated
return beta / 2.8

@staticmethod
@nb.jit(nopython=True, cache=True)
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 1, 2026

Choose a reason for hiding this comment

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

Suggested change
@nb.jit(nopython=True, cache=True)
@nb.njit(cache=True)

process/models/physics/plasma_current.py Outdated
)

if (
PlasmaDiamagneticCurrentModel(physics_variables.i_diamagnetic_current)
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 1, 2026

Choose a reason for hiding this comment

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

Suggested change
PlasmaDiamagneticCurrentModel(physics_variables.i_diamagnetic_current)
physics_variables.i_diamagnetic_current

process/models/physics/plasma_current.py Outdated
current_drive_variables.f_c_plasma_diamagnetic_hender
)
elif (
PlasmaDiamagneticCurrentModel(physics_variables.i_diamagnetic_current)
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 1, 2026

Choose a reason for hiding this comment

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

Suggested change
PlasmaDiamagneticCurrentModel(physics_variables.i_diamagnetic_current)
physics_variables.i_diamagnetic_current

process/models/physics/plasma_current.py Outdated
return self._full_name_


class PlasmaDiamagneticCurrent:
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 1, 2026

Choose a reason for hiding this comment

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

Can you move the outputing of this models variables (e.g. f_c_plasma_diamagnetic_scene) into an output method on this class rather than in bootstrap_current.

timothy-nunn
timothy-nunn requested changes Apr 2, 2026
View reviewed changes
process/models/physics/plasma_current.py

@staticmethod
@nb.jit(nopython=True, cache=True)
@nb.jit(cache=True)
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Apr 2, 2026

Choose a reason for hiding this comment

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

This and others needs to be njit (the n replaces the need for nopython=True)

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

Reviewers

@timothy-nunn timothy-nunn timothy-nunn requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@timothy-nunn timothy-nunn

Labels

Physics Relating to the physics models Refactor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chris-ashe @codecov-commenter @timothy-nunn