Add Fortran API

[awvwgk]

Closes #66

[awvwgk]

Current version fails with OpenMPI due to missing f2c API in mpi_f08 module.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 13 comments.

---

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

[Copilot]

This unconditionally forces GAUXC_HAS_MPI_F08=TRUE whenever MPI is enabled, which can break builds on MPI implementations that do not provide the mpi_f08 module (or do not support the nonstandard %mpi_val access used later). Instead of forcing this flag, detect F08 support (e.g., via CMake/FindMPI variables like MPI_Fortran_HAVE_F08_MODULE, or a try_compile that use mpi_f08) and only define GAUXC_HAS_MPI_F08 when it is actually available; otherwise keep the fallback path that uses use mpi.

Suggested change

	set( GAUXC_HAS_MPI_F08 TRUE CACHE BOOL "GauXC has MPI Fortran 2008 bindings" FORCE )
	if(DEFINED MPI_Fortran_HAVE_F08_MODULE AND MPI_Fortran_HAVE_F08_MODULE)
	set( GAUXC_HAS_MPI_F08 TRUE CACHE BOOL "GauXC has MPI Fortran 2008 bindings" )
	else()
	set( GAUXC_HAS_MPI_F08 FALSE CACHE BOOL "GauXC has MPI Fortran 2008 bindings" )
	endif()

[Copilot]

The test runner always use mpi_f08 when GAUXC_HAS_MPI is set, but the rest of the codebase has conditional support for both mpi and mpi_f08. If GAUXC_HAS_MPI is true but mpi_f08 is not available, this test binary will not compile. Additionally, it calls MPI_Init/MPI_Finalize unconditionally; since MPI_Initialized/MPI_Finalized are already imported, it would be safer to guard initialization/finalization to avoid runtime errors in environments where MPI is already initialized or finalized.

[Copilot]

The test runner always use mpi_f08 when GAUXC_HAS_MPI is set, but the rest of the codebase has conditional support for both mpi and mpi_f08. If GAUXC_HAS_MPI is true but mpi_f08 is not available, this test binary will not compile. Additionally, it calls MPI_Init/MPI_Finalize unconditionally; since MPI_Initialized/MPI_Finalized are already imported, it would be safer to guard initialization/finalization to avoid runtime errors in environments where MPI is already initialized or finalized.

[Copilot]

Using transfer(pmsg(1:slen), \"\") is not portable because the mold \"\" is default character kind, which may differ from c_char (and transfer does not perform kind conversion). This can produce incorrect results or fail on compilers where default character kind is not 1 byte. Prefer constructing msg using a c_char mold (e.g., allocate msg with character(kind=c_char,len=slen) and transfer with msg as the mold, or copy element-wise from pmsg).

Suggested change

	character(kind=c_char, len=:), allocatable :: msg
	character(kind=c_char), pointer :: pmsg(:)
	if (c_associated(status%message)) then
	slen = strlen(status%message)
	call c_f_pointer(status%message, pmsg, [slen])
	msg = transfer(pmsg(1:slen), "")
	else
	msg = ""
	integer(c_int) :: i
	character(kind=c_char, len=:), allocatable :: msg
	character(kind=c_char), pointer :: pmsg(:)
	if (c_associated(status%message)) then
	slen = strlen(status%message)
	call c_f_pointer(status%message, pmsg, [slen])
	allocate(character(kind=c_char, len=slen) :: msg)
	do i = 1, slen
	msg(i:i) = pmsg(i)
	end do
	else
	allocate(character(kind=c_char, len=0) :: msg)

[wavefunction91]

🦆 Rubber Duck Review

I ran a multi-agent rubber-duck review over the Fortran API, deploying five parallel reviewers each focused on a different slice (build/CMake, MPI&ISO_C_BINDING, lifecycle/small wrappers, the big xc_integrator.f90, and functional+tests). The two most consequential findings (svwn3/svwn5 enum bug and the integrator-wrapper contiguity claim) I verified by hand.

Findings are ranked by severity; file:line citations throughout.

Critical — silent wrong results or memory corruption

1. svwn3 / svwn5 enum aliases map to the wrong enum values — src/fortran-api/functional.f90:351-352

   integer(c_int) :: svwn3 = gauxc_functional_vwn3   ! ← should be _svwn3
   integer(c_int) :: svwn5 = gauxc_functional_vwn5   ! ← should be _svwn5

   SVWN = Slater exchange + VWN correlation; VWN = correlation-only. A user who writes gauxc_functional%svwn3 silently gets a correlation-only functional and a physically wrong total energy. The raw gauxc_functional_svwn3 / _svwn5 enumerators are fine; only these two struct aliases are wrong. No test exercises them.

2. strlen interface declared to return c_int, but C strlen returns size_t — src/fortran-api/status.f90:34-35 — 4-byte read of an 8-byte register on LP64. Works by luck when the upper bits happen to be zero. Fix: integer(c_size_t) :: len and matching slen.

3. MPI_Fint ↔ integer(c_int) mismatch — src/fortran-api/runtime_environment.F90:61,118,135 and src/fortran-api/f_runtime_environment.cxx:20,28,36. MPI_Fint is implementation-defined. Works on every common Linux MPI today, but Cray MPICH and MPICH --enable-large-tests can make it 64-bit, in which case the Fortran handle is silently truncated before MPI_Comm_f2c. Either expose the real MPI_Fint kind via a CMake-probed preprocessor define, or add static_assert(sizeof(MPI_Fint) == sizeof(int), …) in f_runtime_environment.cxx to make the assumption explicit.

4. Status message memory leak — no Fortran-accessible gauxc_status_delete — src/fortran-api/status.f90 — Every error path on the C side allocates status->message; the Fortran gauxc_status module never wraps gauxc_status_delete, so messages accumulate over the lifetime of the program. Expose the delete and document that callers must call it (or have the wrappers call it after the user reads the message).

5. GAUXC_HAS_MPI_F08 set TRUE unconditionally when MPI is on — src/fortran-api/CMakeLists.txt:41-43 — No probe that the installed MPI actually provides the mpi_f08 module. MPICH <3.1, OpenMPI 2.x and several vendor MPIs do not. Build fails deep in the compiler with no CMake-level explanation. Use check_fortran_source_compiles("program p; use mpi_f08; end program" HAVE_MPI_F08) and set the flag from that result.

6. Test driver guards use mpi_f08 with the wrong macro — tests/fortran_main.F90:7-8 uses #ifdef GAUXC_HAS_MPI, but the library everywhere else uses the nested #ifdef GAUXC_HAS_MPI_F08. Once [WIP] HIP XC Integrator #5 is fixed, the test will fail to compile on legacy MPI installs.

7. Assumed-shape arrays passed to bind(c) assumed-size dummies without contiguous — src/fortran-api/molecule.f90:111-120 (atoms(:)) and src/fortran-api/basisset.f90 (shells(:)). If a user passes a strided slice (e.g. atoms(1::2)), the C side reads garbage. Add the contiguous attribute. (The xc_integrator wrappers correctly use contiguous already — confirmed at src/fortran-api/xc_integrator.f90:606-612.)

High

#	Issue	Location
8	enable_language(Fortran) in installed gauxc-config.cmake fires for every consumer, including pure C++ projects — hard fails on Fortran-less CI/HPC images	cmake/gauxc-config.cmake.in:32
9	.mod files installed flat to include/gauxc/modules/ — not portable across compilers or even gfortran major versions; silently breaks downstream	src/fortran-api/CMakeLists.txt:54-58
10	hdf5_read.cxx and hdf5_read.f90 (and _write) share leaf names in same target — collide in VS / Xcode generators that key off the leaf	src/external/CMakeLists.txt:35,40
11	Test main runs all assertions on every MPI rank → duplicated output and rank-dependent exit codes	tests/fortran_main.F90:32
12	Multiple tests leak GauXC objects on early-exit error paths, masking real library leaks under Valgrind/ASan	tests/fortran_api_test.f90:170-207, 272-286, 313-397
13	use mpi, only: MPI_Comm (non-f08 branch) — MPI_Comm is not guaranteed exported from MPICH's mpi module; only needed in the f08 branch anyway	src/fortran-api/runtime_environment.F90:22
14	%mpi_val is MPI-4 (lowercase is an OpenMPI extension); not standard in MPI-3 f08	src/fortran-api/runtime_environment.F90:171,196,210,241,255,288
15	gauxc_runtime_environment_new_f symbol arity changes by build flag without renaming → mixed-MPI binaries can link but crash at runtime	src/fortran-api/f_runtime_environment.cxx:18-23
16	gauxc_delete is a per-module generic; user must use every relevant module to see all specifics — undocumented footgun	(multiple modules)
17	hdr.type not cleared after gauxc_*_delete, so zombie handles still report their original tag	(every handle type)

Medium

cmake_dependent_option silently demotes GAUXC_ENABLE_FORTRAN=ON when C is off (no warning); GAUXC_HAS_FORTRAN set in two places that can drift; include/gauxc/gauxc_config.f.in lacks an include guard; cmake/gauxc-test-drive.cmake falls through to a nonexistent test-drive-lib target on miss; no static_assert on GauXCRuntimeEnvironment size to catch GAUXC_HAS_DEVICE skew; HDF5 wrappers declare status as intent(out) inconsistent with the rest of the API (src/external/hdf5_read.f90:33,67, hdf5_write.f90:33,67); use iso_c_binding missing intrinsic qualifier in functional.f90:13, molecular_weights.f90:13, hdf5_read.f90:13, hdf5_write.f90:13; MPI_Initialized imported but never called; no numerical integration test — pipeline test only checks header tags; functional-enum-struct values never verified by tests, so finding #1 is invisible.

Low

value attribute on user-facing Fortran wrappers (unusual style); dead MAKE_DIRECTORY at configure time; gauxc_type_*_factory enums publicly exported but otherwise unused; generic gauxc_delete never exercised by a test.

What the reviewers liked

• src/fortran-api/xc_integrator.f90 (860 LOC) came back essentially clean. All 14 bind(c) interfaces match their C counterparts; c_int vs c_int64_t is consistent throughout; null-termination handled via transfer+c_null_char; GKS 4-component splitting correct; FXC separates P/T with independent leading dims; contiguous used on every assumed-shape wrapper.
• 79-functional enum ordering matches the C side exactly (only the two struct aliases above are wrong).
• gauxc_atom_type struct layout matches C with no padding surprises (int64_t Z then three doubles, natural alignment).
• C side reliably sets ptr = nullptr on factory error paths, so post-failure gauxc_delete is safe.
• MPI_Comm_f2c placed at the correct C/Fortran boundary — textbook design.
• cmake/gauxc-test-drive.cmake pins to a real version (v0.5.0) and tries CONFIG/PkgConfig before fetch.
• .F90 vs .f90 naming correctly applied throughout.

Bottom line

Most of the codebase is solid, careful Fortran. The integrator wrapper — the file most exposed to silent numerical bugs — is exemplary. The defects cluster in three places that genuinely benefit from a careful Fortran review: (a) the enum-struct copy-paste bug, (b) MPI portability assumptions (MPI_Fint, mpi_f08 always present, MPI_Comm export, %mpi_val), and (c) install-time portability (.mod files, downstream enable_language(Fortran)). I'd treat findings 1, 2, 6, 7 as merge-blockers; 3, 4, 5 as required follow-ups; and the High items as issues to file before close.

_{Generated by a multi-agent rubber-duck review (5 parallel code-review agents, one per scope) via GitHub Copilot CLI.}