Make the toroidal mode number (mph) an integer

[zandivx]

User description

The toroidal mode number should logically be an integer. This PR adapts all code paths using mph to expect an integer. However, with these changes the golden record test fails, as integer to real conversions performed by the compiler now whenever mph is multiplied onto a real seem to yield slightly different results. I claim that this is okay anyway as the tolerances used for the golden record test are very strict (rtol=1e-8, atol=1e-15).

---

PR Type

Enhancement

---

Description

• Convert toroidal mode number mph from real to integer type

• Update all code paths to handle mph as integer throughout codebase

• Adjust format specifiers in output statements from ES12.5 to I0

• Add explicit type conversions where mph is used in real arithmetic

---

Diagram Walkthrough

flowchart LR
  A["mph: real64"] -->|"Type conversion"| B["mph: integer"]
  B -->|"Config & modules"| C["Type declarations"]
  B -->|"Calculations"| D["Explicit conversions with dble/nint"]
  B -->|"Output"| E["Format specifiers I0"]

Loading

File Walkthrough

Relevant files

Enhancement

9 files config.f90 Change mph type from real to integer                                          +1/-1      diag_atten_map.f90 Update mph output format to integer                                            +1/-1      diag_bounce_nonlin.f90 Update mph output format to integer                                            +1/-1      do_magfie_neo.f90 Declare mph as integer, convert with nint                                +2/-2      do_magfie_standalone.f90 Change mph and mph_shared to integer type                                +5/-5      freq.f90 Remove int() cast, mph already integer                                      +2/-2      nonlin.f90 Add explicit dble() conversion for real arithmetic              +1/-1      orbit.f90 Update mph format specifier to I0                                                +2/-2      test_omega_prime.f90 Add explicit dble() conversion for real arithmetic              +1/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Division by zero Description: Potential division-by-zero/DoS: the new integer mph is used as a divisor in me = -dble(mth) / mph and mf = 1.0_dp / mph, and since mph can be user-controlled (e.g., defaulting to 0 in config/namelist paths) this can trigger a runtime crash or undefined behavior if mph == 0. nonlin.f90 [59-61] Referred Code me = -dble(mth) / mph mf = 1.0_dp / mph
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Division by zero: New code divides by mph without validating that mph is non-zero, which can crash or produce undefined results for default/invalid inputs. Referred Code me = -dble(mth) / mph mf = 1.0_dp / mph Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Missing mph validation: mph is now an integer defaulting to 0 but downstream code paths use it as a divisor, so external/namelist-derived mph should be validated (e.g., mph > 0) before being accepted/used. Referred Code integer :: mph = 0 ! toroidal perturbation mode (if pertfile==F, n>0!) logical :: comptorque = .false. ! compute torque Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Update golden record instead of ignoring test failures Instead of ignoring the failing golden record test, the test should be updated. The new results should be analyzed to confirm they are expected, and then the golden record should be regenerated and included in the PR. Solution Walkthrough: Before: // PR Process State: Before // 1. Code is modified to use integer `mph`. // 2. Golden record test is executed. // 3. Test fails due to small numerical differences. // 4. PR is proposed with the failing test being ignored. test_suite.run() // Result: FAILED // Action: Ignore failure and merge. After: // PR Process State: After // 1. Code is modified to use integer `mph`. // 2. New results are generated. // 3. Differences from the old golden record are analyzed and confirmed to be expected. // 4. The golden record is updated with the new results. // 5. PR is proposed with the updated golden record. test_suite.run() // Result: PASSED // Action: Merge with confidence. Suggestion importance[1-10]: 9 __ Why: This suggestion addresses a critical flaw in the PR's validation process; ignoring a failing golden record test undermines its purpose, and the proper action is to verify the new results and update the record.	High
Possible issue	Refactor to prevent division by zero Refactor the calculations for dvdJ and detadJ to algebraically eliminate division by mph, preventing potential division-by-zero errors and improving numerical stability. src/nonlin.f90 [59-63] -me = -dble(mth) / mph -mf = 1.0_dp / mph +real(dp) :: common_den -dvdJ = mb * me / (ma * md * mf - mb * mc * mf) -detadJ = ma * me / (mb * mc * mf - ma * md * mf) +common_den = ma * md - mb * mc +if (common_den == 0.0_dp) then + ! Avoid division by zero, though this case may be unphysical + dvdJ = 0.0_dp + detadJ = 0.0_dp +else + dvdJ = -mb * dble(mth) / common_den + detadJ = ma * dble(mth) / common_den +end if Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential division-by-zero error and proposes an algebraic refactoring that not only resolves the issue but also improves numerical stability, which is a significant improvement.	Medium
	Prevent division by zero in tests Add a check to ensure mph is not zero before performing division in test/test_omega_prime.f90 to prevent potential division-by-zero errors during tests. test/test_omega_prime.f90 [149-150] -f%Jbar(2) = f%J(2) - dble(mth) / mph * f%J(3) -f%Jbar(3) = f%J(3) / mph +if (mph /= 0) then + f%Jbar(2) = f%J(2) - dble(mth) / mph * f%J(3) + f%Jbar(3) = f%J(3) / mph +else + f%Jbar(2) = f%J(2) + f%Jbar(3) = 0.0_dp ! Or some other sensible default for mph=0 +end if Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential division-by-zero error in a test file and proposes adding a guard clause, which is a good practice for making tests more robust.	Medium
General	Validate m_phi rounding After converting m_phi to the integer mph, add a check to verify that the fractional part of m_phi was negligible and raise an error if not. src/do_magfie_pert_mod.f90 [102] mph = nint(m_phi) +if (abs(m_phi - real(mph, kind=dp)) > 1.0e-6_dp) then + call error('Perturbation m_phi not integer: '//trim(adjustl(to_str(m_phi)))) +end if [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: This suggestion proposes adding a validation check to ensure that converting m_phi to an integer does not discard a significant fractional part, which improves the robustness of the code by catching potentially invalid input values.	Low
	Check standalone mode rounding Add a validation check after calculating mph_shared to ensure the value being rounded to an integer is within a small tolerance of that integer, and raise an error if not. src/do_magfie_standalone.f90 [404] -mph_shared = nint(nfp * modes(1, 1, 2)) +mph_shared = nint(nfp * modes(1,1,2)) +if (abs(nfp*modes(1,1,2) - real(mph_shared, kind=dp)) > 1.0e-6_dp) then + call error('Standalone perturbation mode non-integer: '//trim(adjustl(to_str(nfp*modes(1,1,2)))) +end if [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: This suggestion correctly proposes applying the same validation logic from a previous suggestion to a similar calculation in another file, ensuring consistent robustness against unexpected non-integer mode numbers.	Low
More

[Copilot]

Pull request overview

This PR changes the type of the toroidal mode number mph from a floating-point real to an integer throughout the codebase, which is conceptually correct since mode numbers are inherently discrete integer values.

Changes:

• Changed mph type declarations from real(dp) to integer in config, module declarations, and function signatures
• Updated format specifiers from ES12.5 (scientific notation for reals) to I0 (integer) in diagnostic output statements
• Added explicit type conversions using dble() where integer-to-real conversion clarity is needed, and nint() where real mode numbers from files need to be converted to integers

Reviewed changes

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

Show a summary per file

File	Description
test/test_omega_prime.f90	Added dble() conversion for mth in division operation to ensure proper real arithmetic
src/orbit.f90	Updated format specifier from ES12.5 to I0 for mph in error diagnostic outputs
src/nonlin.f90	Added dble() conversion for mth in the omega_prime function to ensure proper real arithmetic
src/freq.f90	Updated format specifiers and removed unnecessary int() cast since mph is now natively integer
src/do_magfie_standalone.f90	Changed mph and mph_shared declarations to integer, added nint() for converting real mode values from file
src/do_magfie_neo.f90	Changed mph declaration to integer, added nint() for converting m_phi from external module
src/diag/diag_bounce_nonlin.f90	Updated format specifier from ES12.5 to I0 for mph in diagnostic output
src/diag/diag_atten_map.f90	Updated format specifier from ES12.5 to I0 for mph in diagnostic output
src/config.f90	Changed mph type in config struct from real(dp) to integer

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.