Skip to content

Convert input.f90 to python#3552

Merged
timothy-nunn merged 9 commits intomainfrom
3528-convert-inputf90-to-python
Mar 19, 2025
Merged

Convert input.f90 to python#3552
timothy-nunn merged 9 commits intomainfrom
3528-convert-inputf90-to-python

Conversation

@timothy-nunn
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn commented Feb 19, 2025

Converts PROCESS input file parsing, validation, and actioning (mostly setting variables) to Python

Limitations

  • Cannot parse string arrays (might be possible but I do not think its done in the code)

Regression failures

It seems there is a slight rounding difference that is causing the regression tests to fail. At the moment, some of the optimising runs fail because they slightly drift off their usual course. This can observed by amending the test_parse_real on main and adding:

[
        ("0.546816593988753", 0.546816593988753),
        ("0.6", 0.6),
]

The test fails with

FAILED tests/unit/test_input.py::test_parse_real[0.546816593988753-0.546816593988753] - assert 0.5468165939887529 == 0.546816593988753
FAILED tests/unit/test_input.py::test_parse_real[0.6-0.6] - assert 0.6000000000000001 == 0.6

It should be noted that this failure only happens when the difference is big enough, for example neither of the following fail despite asserting that the same parsed value is exactly equal to two slightly different values:

[
        ("0.008", 0.0080000000000000002),
        ("0.008", 0.008),
]

None of these tests fail on this branch

Notes

  • Did not include factor, ftol as an input anymore, they are HYBRD variables and should be removed.
  • Did not include nvar as it should be obsolete
  • I did not copy across the warnings for dr_blkt_inboard and dr_blkt_outboard + i_blanket_type == 3 because this just printed a message. It should be checked that the docs make this very clear.
    case ('dr_blkt_inboard')
    if (i_blanket_type == 3) then
    !CCFE HCPB model with Tritium Breeding Ratio calculation
    write(outfile,*) '**********'
    write(outfile,*) 'ERROR. BLNKITH input is not required for CCFE HCPB model with Tritium Breeding Ratio calculation -'
    write(outfile,*) 'please remove it from the input file'
    write(outfile,*) '**********'
    else
    call parse_real_variable('dr_blkt_inboard', dr_blkt_inboard, 0.0D0, 10.0D0, &
    'Inboard blanket thickness (m)')
    ! Inboard blanket does not exist if the thickness is below a certain limit.
    if(dr_blkt_inboard>=0.0D00.and.dr_blkt_inboard<=1.0D-3) then
    dr_blkt_inboard = 0.0D00 ! Inboard blanket thickness is zero
    i_blkt_inboard = 0 ! Inboard blanket does not exist
    end if
    end if
    case ('dr_blkt_outboard')
    if (i_blanket_type == 3) then
    !CCFE HCPB model with Tritium Breeding Ratio calculation
    write(outfile,*) '**********'
    write(outfile,*) 'ERROR. dr_blkt_outboard input is not required for CCFE HCPB model with Tritium Breeding Ratio calculation -'
    write(outfile,*) 'please remove it from the input file'
    write(outfile,*) '**********'
    else
    call parse_real_variable('dr_blkt_outboard', dr_blkt_outboard, 0.0D0, 10.0D0, &
    'Outboard blanket thickness (m)')
    end if
  • Did not include the following warning because it is likely no longer relevant
    if (i_tf_sc_mat == 2) then
    write(outfile,*) ' '
    write(outfile,*) '**********'
    write(outfile,*) 'Warning if you are using an old input file:'
    write(outfile,*) 'i_tf_sc_mat=2 usage has changed -'
    write(outfile,*) 'please check validity!'
    write(outfile,*) '**********'
    write(outfile,*) ' '

@timothy-nunn timothy-nunn linked an issue Feb 19, 2025 that may be closed by this pull request
@timothy-nunn timothy-nunn force-pushed the 3528-convert-inputf90-to-python branch from 42c78b6 to b765375 Compare February 19, 2025 16:31
@timothy-nunn timothy-nunn changed the title Convert inputf.90 to python Convert input.f90 to python Feb 19, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 19, 2025

Codecov Report

❌ Patch coverage is 81.90476% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.59%. Comparing base (cfab6dc) to head (25f4001).
⚠️ Report is 463 commits behind head on main.

Files with missing lines Patch % Lines
process/init.py 74.07% 21 Missing ⚠️
process/input.py 86.82% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3552      +/-   ##
==========================================
+ Coverage   31.20%   31.59%   +0.39%     
==========================================
  Files          83       84       +1     
  Lines       19945    20112     +167     
==========================================
+ Hits         6223     6355     +132     
- Misses      13722    13757      +35     

☔ 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 3528-convert-inputf90-to-python branch 3 times, most recently from ccf63cc to 9faa9da Compare February 26, 2025 15:50
@timothy-nunn
Copy link
Copy Markdown
Collaborator Author

timothy-nunn commented Feb 26, 2025

Differences seen in the once-through input files at 0% tolerance come from a slight difference parsing in Python vs Fortran.

A difference is seen in the ST once through when parsing the following variables in Python instead of Fortran

beta, coreradius, coreradiationfraction, casths_fraction, d_vv_bot, dr_bore, dr_shld_outboard, dr_vv_outboard, etahtp, f_nd_alpha_electron, f_sync_reflect, fbeta_max, fipir, fne0, fradpwr, fradwall, fstrcase, fstrcond, ind_plasma_internal_norm, rhopedn, shldlth, thkcas, thshield_vb, vgap_xpoint_divertor

where the error accumulates more so due to the naive implementation (sum the digits multiplied by their place value).

I have taken some of these variables and written a test into test_input.py. These tests assert that the float is parsed and returned accurately when accessed. When running these tests with the input.f90 parser, they all failed:

tests/unit/test_input.py::test_parse_real_didnt_work_in_f90[0.546816593988753] FAILED                                                      [ 70%]
tests/unit/test_input.py::test_parse_real_didnt_work_in_f90[0.13134204235647895] FAILED                                                    [ 72%]
tests/unit/test_input.py::test_parse_real_didnt_work_in_f90[0.75] FAILED                                                                   [ 75%]
tests/unit/test_input.py::test_parse_real_didnt_work_in_f90[0.7] FAILED                                                                    [ 77%]
tests/unit/test_input.py::test_parse_real_didnt_work_in_f90[0.3] FAILED                                                                    [ 79%]
tests/unit/test_input.py::test_parse_real_didnt_work_in_f90[0.1293140904093427] FAILED 

But all passed when using my new Python parser. Note that Fortran even had problem parsing simple numbers with one decimal place (ie they had an error that was large enough that Python did not consider them equal)

@timothy-nunn timothy-nunn force-pushed the 3528-convert-inputf90-to-python branch from 9faa9da to b6d7fd2 Compare February 27, 2025 11:54
@timothy-nunn timothy-nunn self-assigned this Feb 27, 2025
@timothy-nunn timothy-nunn marked this pull request as ready for review February 27, 2025 13:18
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.

This is great Tim, SO much better. Just some clarifying comments required please. Did you check out cerberus for this?

Comment thread tests/unit/test_input.py
Comment thread process/input.py
Comment thread process/input.py Outdated
variable_config,
)

# again its the input name not the target name
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this comment be clarified?

Comment thread process/input.py Outdated
Comment thread scripts/create_dicts.py Outdated
Comment thread process/init.py
Comment thread tests/unit/test_input.py Outdated
@timothy-nunn timothy-nunn force-pushed the 3528-convert-inputf90-to-python branch from 29498a7 to fb5a88e Compare March 6, 2025 10:16
@timothy-nunn timothy-nunn requested a review from jonmaddock March 6, 2025 10:16
@timothy-nunn timothy-nunn force-pushed the 3528-convert-inputf90-to-python branch from fb5a88e to 6bd85b4 Compare March 6, 2025 10:52
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, I'm happy with this. Did you look into cerberus?

@timothy-nunn timothy-nunn force-pushed the 3528-convert-inputf90-to-python branch from 6bd85b4 to 25f4001 Compare March 19, 2025 15:04
@timothy-nunn
Copy link
Copy Markdown
Collaborator Author

Great, I'm happy with this. Did you look into cerberus?

@jonmaddock I didn't end up using cerberus because the validation we implement here is very minimal and existing at the time of investigation. In future, it will be worth looking at this as we convert our data structure and IO formats.

@timothy-nunn timothy-nunn merged commit 32083c7 into main Mar 19, 2025
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 input.f90 to Python

3 participants