Fix/spl per recursive

[krystophny]

PR Type

Enhancement, Tests, Bug fix

---

Description

• Refactor libneo_coordinates into modular OOP architecture with separate base, VMEC, geoflux, chartmap, validator, and file detection modules

• Add comprehensive regression tests for vector conversion, geoflux, and VMEC coordinate systems with 76 passing tests

• Mark spline subroutines as recursive for OpenMP thread safety with gfortran -fcheck=all

• Fix fpm.toml dependency syntax and add bind.toml for Python binding generation

---

Diagram Walkthrough

flowchart LR
  A["Monolithic libneo_coordinates"] -->|Refactor| B["libneo_coordinates_base"]
  B -->|Abstract type| C["coordinate_system_t"]
  C -->|Extends| D["vmec_coordinate_system_t"]
  C -->|Extends| E["geoflux_coordinate_system_t"]
  C -->|Extends| F["chartmap_coordinate_system_t"]
  B -->|Exports| G["libneo_coordinates.f90"]
  H["libneo_coordinates_validator"] -->|Validates| F
  I["libneo_coordinates_file_detection"] -->|Detects| J["File types"]
  K["Spline subroutines"] -->|Mark recursive| L["OpenMP safe"]
  M["New tests"] -->|Verify| C

Loading

File Walkthrough

	Relevant files
Enhancement	7 files libneo_coordinates_base.f90 New abstract base module with coordinate system type          +135/-0  libneo_coordinates_vmec.f90 Standalone VMEC coordinate system implementation module    +26/-6    libneo_coordinates_geoflux.f90 Standalone geoflux coordinate system implementation module +34/-16  libneo_coordinates_chartmap.f90 Standalone chartmap coordinate system implementation module +48/-27  libneo_coordinates_validator.f90 Extracted chartmap file validation into separate module    +7/-3      libneo_coordinates_file_detection.f90 Extracted reference coordinate file type detection module +10/-3    libneo_coordinates.f90 Facade module re-exporting public API from submodules        +18/-256
Tests	4 files test_vector_conversion.f90 New comprehensive vector conversion regression tests          +332/-0  test_geoflux_coordinate_system.f90 New geoflux coordinate system regression test suite            +310/-0  test_vmec_coordinate_system.f90 Enhanced VMEC tests with consistency and property checks  +196/-34 CMakeLists.txt Add new coordinate system test executables and targets      +22/-0
Bug fix	1 files spl_three_to_five.f90 Mark spline subroutines as recursive for thread safety      +9/-9
Configuration changes	3 files CMakeSources.in Update build sources for modular coordinate system files  +7/-1      fpm.toml Fix dependency syntax and add external modules list            +7/-5      bind.toml New fpm-bind configuration for Python binding generation  +7/-0
Additional files	1 files libneo_coordinates_merged.f90 +0/-1171

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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 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 error path: The newly added geoflux_from_cyl unconditionally calls error stop "geoflux_from_cyl: not implemented" (and even sets ierr = 0 beforehand), which is not graceful error handling and prevents callers from handling the failure via ierr. Referred Code real(dp), intent(out) :: u(3) integer, intent(out) :: ierr associate(dummy => self) end associate associate(dummy2 => xcyl) end associate u = 0.0_dp ierr = 0 error stop "geoflux_from_cyl: not implemented" end subroutine geoflux_from_cyl 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: User-facing stop message: The error stop message includes internal implementation detail (geoflux_from_cyl: not implemented), and it is unclear from the diff whether this can reach end users vs. being restricted to internal/test usage. Referred Code real(dp), intent(out) :: u(3) integer, intent(out) :: ierr associate(dummy => self) end associate associate(dummy2 => xcyl) end associate u = 0.0_dp ierr = 0 error stop "geoflux_from_cyl: not implemented" end subroutine geoflux_from_cyl 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: External file input: The test accepts a file path from command-line/environment (LIBNEO_TEST_GEQDSK) and passes it into spline_geoflux_data without visible validation, and it is unclear whether downstream routines robustly validate/sanitize the file input. Referred Code character(len=*), parameter :: fallback_geqdsk = '../../python/tests/test.geqdsk' character(len=*), parameter :: env_geqdsk = 'LIBNEO_TEST_GEQDSK' character(len=512) :: geqdsk_file character(len=512) :: arg_buffer integer :: arg_status, arg_len integer :: nerrors class(coordinate_system_t), allocatable :: cs nerrors = 0 geqdsk_file = fallback_geqdsk call get_command_argument(1, value=arg_buffer, length=arg_len, status=arg_status) if (arg_status == 0 .and. arg_len > 0 .and. len_trim(arg_buffer) > 0) then geqdsk_file = trim(arg_buffer) else call get_environment_variable(env_geqdsk, value=arg_buffer, & length=arg_len, status=arg_status) if (arg_status == 0 .and. arg_len > 0 .and. len_trim(arg_buffer) > 0) then geqdsk_file = trim(arg_buffer) end if ... (clipped 4 lines) 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
Possible issue	Add file validation before allocation Add a call to validate_chartmap_file at the beginning of make_chartmap_coordinate_system to validate the input file before allocating memory. src/coordinates/libneo_coordinates_chartmap.f90 [338-351] subroutine make_chartmap_coordinate_system(cs, filename) use libneo_coordinates_validator, only: validate_chartmap_file class(coordinate_system_t), allocatable, intent(out) :: cs character(len=*), intent(in) :: filename + integer :: ierr + character(len=512) :: msg + + call validate_chartmap_file(filename, ierr, msg) + if (ierr /= 0) then + error stop "Invalid chartmap file: "//trim(msg) + end if allocate (chartmap_coordinate_system_t :: cs) - select type (ccs => cs) type is (chartmap_coordinate_system_t) call initialize_chartmap(ccs, filename) class default error stop "make_chartmap_coordinate_system: allocation failed" end select end subroutine make_chartmap_coordinate_system Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that file validation should happen before resource allocation, preventing crashes on invalid files and providing clearer error messages, which is a significant improvement in robustness.	Medium
General	Remove unused code from stub Simplify the geoflux_from_cyl stub by removing redundant associate blocks and dead code that initializes variables before the error stop statement. src/coordinates/libneo_coordinates_geoflux.f90 [124-138] subroutine geoflux_from_cyl(self, xcyl, u, ierr) class(geoflux_coordinate_system_t), intent(in) :: self real(dp), intent(in) :: xcyl(3) real(dp), intent(out) :: u(3) integer, intent(out) :: ierr - associate(dummy => self) - end associate - associate(dummy2 => xcyl) - end associate - - u = 0.0_dp - ierr = 0 error stop "geoflux_from_cyl: not implemented" end subroutine geoflux_from_cyl [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies and removes redundant associate blocks and dead code from a stub function, which improves code clarity and removes clutter.	Low
	Remove no-op associate blocks Remove the empty associate(dummy => self) blocks from geoflux_evaluate_cyl and geoflux_covariant_basis to improve code clarity. src/coordinates/libneo_coordinates_geoflux.f90 [27-61] subroutine geoflux_evaluate_cyl(self, u, x) class(geoflux_coordinate_system_t), intent(in) :: self real(dp), intent(in) :: u(3) real(dp), intent(out) :: x(3) - - associate(dummy => self) - end associate call geoflux_to_cyl(u, x) end subroutine geoflux_evaluate_cyl subroutine geoflux_covariant_basis(self, u, e_cov) class(geoflux_coordinate_system_t), intent(in) :: self real(dp), intent(in) :: u(3) real(dp), intent(out) :: e_cov(3, 3) ... - associate(dummy => self) - end associate + ! no associate block here ... end subroutine geoflux_covariant_basis [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies and removes empty associate blocks that serve no purpose, improving code readability and removing clutter.	Low
Update