Skip to content

🔥 Remove divert() method, associated functions and tests.#3611

Merged
timothy-nunn merged 11 commits intomainfrom
remove_divert_method_and_variables
Apr 4, 2025
Merged

🔥 Remove divert() method, associated functions and tests.#3611
timothy-nunn merged 11 commits intomainfrom
remove_divert_method_and_variables

Conversation

@chris-ashe
Copy link
Copy Markdown
Collaborator

@chris-ashe chris-ashe commented Apr 1, 2025

This pull request includes a comprehensive set of changes focused on removing obsolete variables and constraints related to the divertor system, as well as updating error handling and initialization processes. The most important changes include the removal of specific variables and constraints, updates to error lists, and adjustments to initialization routines.

Removal of Obsolete Variables and Constraints:

  • process/input.py: Removed multiple InputVariable entries related to the divertor system, including bpsout, c1div, c2div, c3div, c4div, c5div, c6div, delld, fdfs, fififi, frrp, ksic, omegan, rlenmax, xparain, zeffdiv, and divdum.
  • source/fortran/constraint_equations.f90: Deprecated constraint equation 22 and updated the corresponding subroutine to reflect this change.
  • source/fortran/constraint_variables.f90: Removed the fdivcol variable and its initialization.
  • : Removed multiple variables related to the divertor system, including adas, bpsout, c1div, c2div, c3div, c4div, c5div, c6div, delld, dendiv, densin, divdum, fdfs, fhout, fififi, frrp, ksic, lamp, minstang, omegan, omlarg, ppdivr, ptpdiv, rlclolcn, rlenmax, tsep, xparain, and zeffdiv.

Updates to Error Handling:

Adjustments to Initialization Routines:

Miscellaneous:

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 Documentation Improvements or additions to documentation Divertor labels Apr 1, 2025
@chris-ashe chris-ashe self-assigned this Apr 1, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 1, 2025

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 31.43%. Comparing base (19c2c8a) to head (5f6b654).

Files with missing lines Patch % Lines
process/divertor.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3611      +/-   ##
==========================================
- Coverage   31.66%   31.43%   -0.24%     
==========================================
  Files          86       86              
  Lines       20198    20096     -102     
==========================================
- Hits         6396     6317      -79     
+ Misses      13802    13779      -23     

☔ 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 remove_divert_method_and_variables branch 5 times, most recently from 1d14a50 to 361b3f8 Compare April 2, 2025 08:06
@chris-ashe chris-ashe marked this pull request as ready for review April 2, 2025 08:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the divert() method along with its associated functions and tests from the codebase. The changes include extensive removal of divertor-related tests, deletion of corresponding input variables in the configuration, and removal of an initialization routine for the divertor.

Reviewed Changes

Copilot reviewed 4 out of 17 changed files in this pull request and generated no comments.

File Description
tests/unit/test_divertor.py Removed tests related to divert() and associated functions
process/input.py Removed input variable definitions for divertor functionality
process/init.py Removed initialization for divertor iteration variable 34
Files not reviewed (13)
  • process/utilities/errorlist.json: Language not supported
  • source/fortran/constraint_equations.f90: Language not supported
  • source/fortran/constraint_variables.f90: Language not supported
  • source/fortran/divertor_variables.f90: Language not supported
  • source/fortran/iteration_variables.f90: Language not supported
  • source/fortran/numerics.f90: Language not supported
  • tests/integration/ref_dicts.json: Language not supported
  • tests/regression/input_files/helias_5b.IN.DAT: Language not supported
  • tests/regression/input_files/large_tokamak.IN.DAT: Language not supported
  • tests/regression/input_files/large_tokamak_nof.IN.DAT: Language not supported
  • tests/regression/input_files/large_tokamak_once_through.IN.DAT: Language not supported
  • tests/regression/input_files/st_regression.IN.DAT: Language not supported
  • tests/regression/input_files/stellarator_helias_once_through.IN.DAT: Language not supported
Comments suppressed due to low confidence (1)

tests/unit/test_divertor.py:21

  • The test 'test_divtart' remains in the file after the removal of the divert() method and its associated functions. Please confirm that this test is still applicable or if it should be removed along with the other divertor tests.
def test_divtart(self, monkeypatch, divertor):

Comment thread process/divertor.py
Comment thread process/input.py
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

One question, then good2go

Comment thread process/divertor.py
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.

Deleted variables look fine to me.

@chris-ashe chris-ashe force-pushed the remove_divert_method_and_variables branch from 6fff7f2 to 5f6b654 Compare April 4, 2025 12:28
@timothy-nunn timothy-nunn merged commit 03ac4a6 into main Apr 4, 2025
14 of 18 checks passed
@timothy-nunn timothy-nunn deleted the remove_divert_method_and_variables branch April 4, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Divertor Documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants