Skip to content

Convert iteration_variables.f90 to Python#3570

Merged
timothy-nunn merged 6 commits intomainfrom
3432-convert-iteration_variablesf90-to-python
Apr 10, 2025
Merged

Convert iteration_variables.f90 to Python#3570
timothy-nunn merged 6 commits intomainfrom
3432-convert-iteration_variablesf90-to-python

Conversation

@timothy-nunn
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn commented Mar 6, 2025

Convert iteration_variables.f90 to Python.

Similar to #3552, I have created a config class and created an entry (in a dict this time) for each existing iteration variable.

Notes

  • I did not convert this q95/qbar multiple iteration variable name feature
    if ((ixc(i) == 18).and.(i_plasma_current /= 2)) name_xc(i) = 'q95'
    if ((ixc(i) == 18).and.(i_plasma_current == 2)) name_xc(i) = 'qbar'
  • The following if statement always resolved true
    if (abs(xcm(i)) > 1.0D-99) then
    scale(i) = 1.0D0/xcm(i)
    else
    scale(i) = 1.0D0
    end if
    because of this previous check
    if (abs(xcm(i)) <= 1.0D-12) then
    idiags(1) = i ; idiags(2) = ixc(i)
    write(*,*) 'Iteration variable ',ixc(i),'(',trim(lablxc(ixc(i))),') is zero.'
    call report_error(55)
    end if
  • Validation in convxc now uses the actual iteration value not its scaled value (where xc(i) below is a scaled iteration variable)
    if (abs(xc(i)) <= 1.0D-12) then
    idiags(1) = i ; idiags(2) = ixc(i)
    write(*,*) 'Iteration variable ',ixc(i),'(',trim(lablxc(ixc(i))),') is zero.'
    call report_error(58)
    end if
    ! Crude method of catching NaN errors
    !if ((abs(xc(i)) > 9.99D99).or.(xc(i) /= xc(i))) then
    if (isnan(xc(i)).or.(abs(xc(i))>9.99D99)) then
    idiags(1) = i ; idiags(2) = ixc(i) ; fdiags(1) = xc(i)
    call report_error(59)
    end if
  • Removed validation of scale in convxc because validation in the loading should protect against very small scales
    if (abs(scale(i)) < 1.0D-99) then
    idiags(1) = i ; idiags(2) = ixc(i)
    call report_error(60)
    end if

@timothy-nunn timothy-nunn linked an issue Mar 6, 2025 that may be closed by this pull request
@timothy-nunn timothy-nunn self-assigned this Mar 6, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 6, 2025

Codecov Report

Attention: Patch coverage is 47.50000% with 42 lines in your changes missing coverage. Please review.

Project coverage is 36.13%. Comparing base (05d827e) to head (ac5eca1).
Report is 116 commits behind head on main.

Files with missing lines Patch % Lines
process/iteration_variables.py 45.71% 38 Missing ⚠️
process/optimiser.py 50.00% 2 Missing ⚠️
process/caller.py 50.00% 1 Missing ⚠️
process/main.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3570      +/-   ##
==========================================
- Coverage   36.54%   36.13%   -0.42%     
==========================================
  Files          87       88       +1     
  Lines       22190    22106      -84     
==========================================
- Hits         8110     7988     -122     
- Misses      14080    14118      +38     

☔ 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 force-pushed the 3432-convert-iteration_variablesf90-to-python branch 2 times, most recently from 3438906 to 83ad60c Compare March 6, 2025 16:40
@timothy-nunn
Copy link
Copy Markdown
Collaborator Author

timothy-nunn commented Mar 12, 2025

This section of code could have been simplified

do i = 1,nvar
if (abs(xcm(i)) > 1.0D-99) then
scale(i) = 1.0D0/xcm(i)
else
scale(i) = 1.0D0
end if
scafc(i) = 1.0D0/scale(i)
xcm(i) = xcm(i)*scale(i)
end do

As you will see, we are essentially performing the following calculations
$s=1/x$
and then later on
$x:=x\times s$

so it follows that when $x$ is set at the end of the snippet, it is to $1$. While this is correct, simplifying the code as such causes a difference in the variable value that causes differences in once-throughs. My guess is that this happens because of some floating point error that accumulates. As a result, some weird expressions exist within the code that look like they should have been simplified.

@timothy-nunn timothy-nunn force-pushed the 3432-convert-iteration_variablesf90-to-python branch from 83ad60c to fe96bc1 Compare March 12, 2025 11:49
@timothy-nunn timothy-nunn marked this pull request as ready for review March 12, 2025 13:37
@timothy-nunn timothy-nunn requested a review from jonmaddock March 12, 2025 13:37
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.

Due to my inaction on this, there are now some conflicts. Could you resolve these before I review?

@timothy-nunn timothy-nunn force-pushed the 3432-convert-iteration_variablesf90-to-python branch from fe96bc1 to 9e45d97 Compare April 8, 2025 16:17
@timothy-nunn timothy-nunn requested a review from jonmaddock April 8, 2025 16:37
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.

Great, nice to get the dicts working nicely too.

@timothy-nunn timothy-nunn force-pushed the 3432-convert-iteration_variablesf90-to-python branch from 9e45d97 to 8923450 Compare April 10, 2025 08:22
@timothy-nunn timothy-nunn force-pushed the 3432-convert-iteration_variablesf90-to-python branch from 8923450 to ac5eca1 Compare April 10, 2025 08:30
@timothy-nunn timothy-nunn merged commit 54b7e35 into main Apr 10, 2025
14 of 18 checks passed
@timothy-nunn timothy-nunn deleted the 3432-convert-iteration_variablesf90-to-python branch April 10, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert iteration_variables.f90 to Python

3 participants