Skip to content

Convert constraints.f90 to Python#3630

Merged
timothy-nunn merged 8 commits intomainfrom
convert-constraints-f90
Jun 11, 2025
Merged

Convert constraints.f90 to Python#3630
timothy-nunn merged 8 commits intomainfrom
convert-constraints-f90

Conversation

@timothy-nunn
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn commented Apr 11, 2025

Description

Converts constraint_equations.f90 to Python.

Notes

  • I have decided not to convert the additional debugging functions. These can be done by users as required. I only implemented NaN/inf checking on the cc
    if (isnan(cc(i)).or.(abs(cc(i))>9.99D99)) then
    ! Add debugging lines as appropriate...
    select case (icc(i))
    ! Relationship between beta, temperature (keV) and density (consistency equation)
    case (1); call constraint_err_001()
    ! Equation for net electric power lower limit
    case (16); call constraint_err_016()
    ! Equation for injection power upper limit
    case (30); call constraint_err_030()
    ! Limit on rate of change of energy in poloidal field
    case (66); call constraint_err_066()
    end select
  • I could not/did not want to replicate this weird logic of constraint 15
    if (fl_h_threshold > 1.0D0) then
    tmp_symbol = '>'
    else
    tmp_symbol = '<'
    end if
    so I have signed it as >= because it follows the form of other equations with that sign.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 11, 2025

Codecov Report

❌ Patch coverage is 52.16638% with 276 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.40%. Comparing base (c100264) to head (f6d742d).
⚠️ Report is 387 commits behind head on main.

Files with missing lines Patch % Lines
process/constraints.py 51.85% 273 Missing ⚠️
process/final.py 33.33% 2 Missing ⚠️
process/caller.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3630      +/-   ##
==========================================
+ Coverage   37.01%   37.40%   +0.39%     
==========================================
  Files          84       85       +1     
  Lines       21679    22250     +571     
==========================================
+ Hits         8024     8323     +299     
- Misses      13655    13927     +272     

☔ 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 convert-constraints-f90 branch 2 times, most recently from a7c88e6 to 22397e2 Compare April 11, 2025 15:45
@timothy-nunn timothy-nunn self-assigned this Apr 14, 2025
@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch 5 times, most recently from d7a917f to 1842ac5 Compare April 15, 2025 15:21
@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch 7 times, most recently from ed63438 to 6317dfe Compare April 23, 2025 16:02
@timothy-nunn timothy-nunn marked this pull request as ready for review April 23, 2025 16:09
@timothy-nunn timothy-nunn requested a review from jonmaddock April 23, 2025 16:10
@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch from 8d5fe79 to b281443 Compare April 24, 2025 09:13
@timothy-nunn timothy-nunn linked an issue Apr 24, 2025 that may be closed by this pull request
@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch 2 times, most recently from 406e808 to dce76f6 Compare April 30, 2025 09:26
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 a great step, but I'm not totally convinced about the additional complexity of the decorator-based registration idea. What's the advantage of this approach?

Comment thread process/caller.py Outdated
Comment thread process/constraints.py Outdated
Comment thread process/constraints.py
Comment thread process/constraints.py Outdated
Comment thread process/constraints.py
Comment thread examples/single_model_evaluation.ipynb Outdated
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.

Thanks for explaining your approach, I'm happy with this now. Could you add docstrings to your new classes please, and find if there are any "adding a new constraint" docs which are now out of date. Could you deal with my minor "returning an array" comment, and lastly change cc and others to something more descriptive?

@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch 2 times, most recently from 41697fb to e0264d3 Compare May 16, 2025 13:11
@timothy-nunn timothy-nunn requested a review from jonmaddock May 16, 2025 13:11
@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch from e0264d3 to 5c3ccfc Compare June 5, 2025 13:27
@timothy-nunn timothy-nunn force-pushed the convert-constraints-f90 branch from 5c3ccfc to f926ca7 Compare June 5, 2025 13:40
@timothy-nunn timothy-nunn requested a review from clmould June 11, 2025 09:06
@timothy-nunn timothy-nunn merged commit 69adcc0 into main Jun 11, 2025
18 checks passed
@timothy-nunn timothy-nunn deleted the convert-constraints-f90 branch June 11, 2025 12:39
OceanNuclear pushed a commit that referenced this pull request Jul 12, 2025
* Convert constraint_eqns to Python

* Write infrastructure to register Python constraints

* Convert remaining equality constraints to Python

* Convert upper limit constraints to Python

* Convert lower limit constraints to Python

* Convert init_constraint_variables to Python

* Clarify documentation and variable names for Python constraint manager

* Fix list formatting in docs
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 constraint_equations.f90 to Python

4 participants