Refactor as library [5]

[zandivx]

User description

To preserve backwards compatibility, the first argument to the built executable is just the base file name and the actual config has ".in" appended. But it makes no sense that this behavior should be in the library API, but rather in the main function of the standalone executable. There's no good reason why calling the API init should automatically append ".in".

---

PR Type

Enhancement

---

Description

• Remove automatic ".in" suffix appending from library API

• Reorder config struct fields (m0, mph) for consistency

• Update all callers to explicitly pass full config filenames

• Preserve misplaced comment at correct location in code

---

Diagram Walkthrough

flowchart LR
  A["Library API<br/>read_and_set_config"] -->|"Now expects<br/>full filename"| B["Config File<br/>with .in extension"]
  C["Main executable<br/>& diagnostics"] -->|"Append .in<br/>explicitly"| B
  D["Config struct<br/>reordering"] -->|"m0 then mph"| E["Consistent field<br/>ordering"]

Loading

File Walkthrough

	Relevant files
Enhancement	7 files config.f90 Reorder config fields and update function signature            +5/-6      diag_atten_map.f90 Append .in extension to config filename                                    +1/-1      diag_bounce_debug.f90 Append .in extension to config filename                                    +1/-1      diag_bounce_nonlin.f90 Append .in extension to config filename                                    +1/-1      diag_contrib_map.f90 Append .in extension to config filename                                    +1/-1      lib.f90 Remove .in suffix handling from library API                            +5/-5      main.f90 Append .in extension to config filename                                    +1/-1
Miscellaneous	1 files do_magfie_standalone.f90 Relocate comment to correct location                                          +0/-1
Tests	5 files test_multi_init.f90 Append .in extension to config filename                                    +1/-1      test_neort_lib.f90 Append .in extension to config filename                                    +3/-3      test_omega_prime.f90 Append .in extension to config filename                                    +1/-1      test_parallel.f90 Append .in extension to config filename                                    +1/-1      test_torque.f90 Append .in extension to config filename                                    +1/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Arbitrary file read Description: The new read_and_set_config(config_file) opens and reads an arbitrary path provided by callers (often derived from command-line runname), which can enable unintended local file read/path traversal (e.g., passing ../../secret.in) if the executable/library is invoked in a privileged or sensitive context. config.f90 [67-87] Referred Code subroutine read_and_set_config(config_file) ! Set global control parameters directly from a file use do_magfie_mod, only: s, bfac, inp_swi use do_magfie_pert_mod, only: mph, set_mph use driftorbit, only: epsmn, m0, comptorque, magdrift, nopassing, pertfile, nonlin, efac use logger, only: set_log_level use neort, only: vsteps use neort_orbit, only: noshear use neort_profiles, only: M_t, vth use util, only: qe, mu, qi, mi character(len=*), intent(in) :: config_file real(dp) :: qs, ms integer :: log_level namelist /params/ s, M_t, qs, ms, vth, epsmn, m0, mph, comptorque, magdrift, & nopassing, noshear, pertfile, nonlin, bfac, efac, inp_swi, vsteps, log_level open (unit=9, file=config_file, status="old", form="formatted") read (9, nml=params) close (unit=9)
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: Unhandled I/O errors: The new read_and_set_config path opens and reads config_file without iostat/iomsg checks, so missing/invalid files or namelist parse errors can fail without actionable context or graceful handling. Referred Code open (unit=9, file=config_file, status="old", form="formatted") read (9, nml=params) close (unit=9) 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: No file input validation: read_and_set_config uses the externally-provided config_file directly in open/read without validating existence or handling invalid paths/namelist content via checked error codes/messages. Referred Code character(len=*), intent(in) :: config_file real(dp) :: qs, ms integer :: log_level namelist /params/ s, M_t, qs, ms, vth, epsmn, m0, mph, comptorque, magdrift, & nopassing, noshear, pertfile, nonlin, bfac, efac, inp_swi, vsteps, log_level open (unit=9, file=config_file, status="old", form="formatted") read (9, nml=params) close (unit=9) Learn more about managing compliance generic rules or creating your own custom rules
Update

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
General	Add error handling for file opening Add iostat error handling to the open statement in the read_and_set_config subroutine to gracefully handle cases where the configuration file cannot be opened. src/config.f90 [67-94] subroutine read_and_set_config(config_file) ! Set global control parameters directly from a file use do_magfie_mod, only: s, bfac, inp_swi use do_magfie_pert_mod, only: mph, set_mph use driftorbit, only: epsmn, m0, comptorque, magdrift, nopassing, pertfile, nonlin, efac use logger, only: set_log_level use neort, only: vsteps use neort_orbit, only: noshear use neort_profiles, only: M_t, vth use util, only: qe, mu, qi, mi character(len=*), intent(in) :: config_file real(dp) :: qs, ms integer :: log_level + integer :: io_status namelist /params/ s, M_t, qs, ms, vth, epsmn, m0, mph, comptorque, magdrift, & nopassing, noshear, pertfile, nonlin, bfac, efac, inp_swi, vsteps, log_level - open (unit=9, file=config_file, status="old", form="formatted") + open (unit=9, file=config_file, status="old", form="formatted", iostat=io_status) + if (io_status /= 0) then + print *, "Error: Could not open config file: ", trim(config_file) + stop 1 + end if read (9, nml=params) close (unit=9) M_t = M_t * efac / bfac qi = qs * qe mi = ms * mu call set_mph(mph) call set_log_level(log_level) end subroutine read_and_set_config Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly points out missing error handling for file I/O and proposes adding an iostat check, which is a critical improvement for robustness, especially since many call sites lack pre-checks.	Medium
	Include filename in error message Improve the error message in the check_file call within init_from_config_file to include the name of the missing configuration file. src/lib.f90 [41] -call check_file(config_file, .true., " could not be found") +call check_file(config_file, .true., "Config file "//trim(config_file)//" could not be found") Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion improves user experience by making the error message more specific, which helps in debugging. It's a good practice for maintainability and usability.	Low
Possible issue	Validate all input files upfront In init_from_config_file, validate the existence of boozer_pert_file before reading the main configuration file to prevent a race condition and improve error reporting. src/lib.f90 [29-53] subroutine init_from_config_file(config_file, boozer_file, boozer_pert_file) use do_magfie_mod, only: read_boozer_file use do_magfie_pert_mod, only: read_boozer_pert_file use driftorbit, only: pertfile, nonlin use neort_config, only: read_and_set_config use thetadata_mod, only: init_attenuation_data use util, only: check_file character(len=*), intent(in) :: config_file character(len=*), intent(in) :: boozer_file character(len=*), intent(in), optional :: boozer_pert_file call check_file(config_file, .true., " could not be found") call check_file(boozer_file, .true., " could not be found") + if (present(boozer_pert_file)) then + call check_file(boozer_pert_file, .true., " could not be found") + end if call read_and_set_config(config_file) call read_boozer_file(boozer_file) if (present(boozer_pert_file) .and. pertfile) then - call check_file(boozer_pert_file, .true., " required if pertfile = .true.") call read_boozer_pert_file(boozer_pert_file) + else if (pertfile) then + ! This case should be caught by the check above if boozer_pert_file is required + ! but we can add a stop for logical consistency. + stop "pertfile is true but perturbation file was not provided." end if if (nonlin) call init_attenuation_data() end subroutine init_from_config_file Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential race condition and improves error handling by checking for a required file earlier, preventing a less informative crash later on.	Low
Update