Address MPI API inconsistencies. (#76)

Our user-facing APIs have a convention of having the output parameters near or at the end of the function argument list. In the MPI API this wasn't the case with qvi_mpi_context_create(). Signed-off-by: Samuel K. Gutierrez <[email protected]>
hpc · Feb 14, 2024 · 648699d · 648699d
1 parent 8d44be0
commit 648699d
Show file tree

Hide file tree

Showing 20 changed files with 371 additions and 366 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2020-2023 Triad National Security, LLC
+# Copyright (c) 2020-2024 Triad National Security, LLC
 #                         All rights reserved.
 #
 # Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC

diff --git a/cmake/QVFortranSupport.cmake b/cmake/QVFortranSupport.cmake
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2022-2023 Triad National Security, LLC
+# Copyright (c) 2022-2024 Triad National Security, LLC
 #                         All rights reserved.
 #
 # This file is part of the quo-vadis project. See the LICENSE file at the
@@ -32,6 +32,8 @@ if(QV_FORTRAN_SUPPORT)
     FortranCInterface_VERIFY()
     # Make sure we found a Fortran compiler.
     if(NOT CMAKE_Fortran_COMPILER STREQUAL "")
+        # TODO(skg) Improve
+        set(CMAKE_FORTRAN_FLAGS "${CMAKE_FORTRAN_FLAGS} -Wall -Wextra -pedantic")
         set(QV_FORTRAN_HAPPY TRUE)
         set(
             CMAKE_Fortran_MODULE_DIRECTORY

diff --git a/include/quo-vadis-mpi.h b/include/quo-vadis-mpi.h
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2022 Triad National Security, LLC
+ * Copyright (c) 2020-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -34,8 +34,8 @@ extern "C" {
  */
 int
 qv_mpi_context_create(
-    qv_context_t **ctx,
-    MPI_Comm comm
+    MPI_Comm comm,
+    qv_context_t **ctx
 );
 
 /**

diff --git a/include/quo-vadis.h b/include/quo-vadis.h
@@ -223,7 +223,7 @@ qv_scope_nobjs(
     qv_context_t *ctx,
     qv_scope_t *scope,
     qv_hw_obj_type_t obj,
-    int *n
+    int *nobjs
 );
 
 /**

diff --git a/src/fortran/quo-vadis-mpif.f90 b/src/fortran/quo-vadis-mpif.f90
@@ -1,5 +1,5 @@
 !
-! Copyright (c) 2013-2022 Triad National Security, LLC
+! Copyright (c) 2013-2024 Triad National Security, LLC
 !                         All rights reserved.
 !
 ! This file is part of the quo-vadis project. See the LICENSE file at the
@@ -12,7 +12,7 @@ module quo_vadis_mpif
 
 interface
     integer(c_int) &
-    function qv_mpi_context_create_c(ctx, comm) &
+    function qv_mpi_context_create_c(comm, ctx) &
         bind(c, name='qvi_mpi_context_create_f2c')
         use, intrinsic :: iso_c_binding, only: c_ptr, c_int
         implicit none
@@ -41,13 +41,13 @@ end function qv_mpi_context_free_c
 
 contains
 
-    subroutine qv_mpi_context_create(ctx, comm, info)
+    subroutine qv_mpi_context_create(comm, ctx, info)
         use, intrinsic :: iso_c_binding, only: c_ptr, c_int
         implicit none
         type(c_ptr), intent(out) :: ctx
         integer, value :: comm
         integer(c_int), intent(out) :: info
-        info = qv_mpi_context_create_c(ctx, comm)
+        info = qv_mpi_context_create_c(comm, ctx)
     end subroutine qv_mpi_context_create
 
     subroutine qv_mpi_scope_comm_dup(ctx, scope, comm, info)

diff --git a/src/quo-vadis-mpi.cc b/src/quo-vadis-mpi.cc
@@ -33,11 +33,11 @@ extern "C" {
 
 int
 qvi_mpi_context_create_f2c(
-    qv_context_t **ctx,
-    MPI_Fint comm
+    MPI_Fint comm,
+    qv_context_t **ctx
 ) {
     MPI_Comm c_comm = MPI_Comm_f2c(comm);
-    return qv_mpi_context_create(ctx, c_comm);
+    return qv_mpi_context_create(c_comm, ctx);
 }
 
 int
@@ -58,8 +58,8 @@ qvi_mpi_scope_comm_dup_f2c(
 
 int
 qv_mpi_context_create(
-    qv_context_t **ctx,
-    MPI_Comm comm
+    MPI_Comm comm,
+    qv_context_t **ctx
 ) {
     if (!ctx || comm == MPI_COMM_NULL) {
         return QV_ERR_INVLD_ARG;

diff --git a/src/quo-vadis-process.cc b/src/quo-vadis-process.cc
@@ -18,16 +18,6 @@
 #include "qvi-context.h"
 #include "qvi-zgroup-process.h"
 
-int
-qv_process_context_free(
-    qv_context_t *ctx
-) {
-    if (!ctx) return QV_ERR_INVLD_ARG;
-    delete ctx->zgroup;
-    qvi_context_free(&ctx);
-    return QV_SUCCESS;
-}
-
 int
 qv_process_context_create(
     qv_context_t **ctx
@@ -76,6 +66,16 @@ qv_process_context_create(
     return rc;
 }
 
+int
+qv_process_context_free(
+    qv_context_t *ctx
+) {
+    if (!ctx) return QV_ERR_INVLD_ARG;
+    delete ctx->zgroup;
+    qvi_context_free(&ctx);
+    return QV_SUCCESS;
+}
+
 /*
  * vim: ft=cpp ts=4 sts=4 sw=4 expandtab
  */
diff --git a/src/quo-vadis.cc b/src/quo-vadis.cc
@@ -123,13 +123,13 @@ qv_scope_nobjs(
     qv_context_t *ctx,
     qv_scope_t *scope,
     qv_hw_obj_type_t obj,
-    int *n
+    int *nobjs
 ) {
-    if (!ctx || !scope || !n) {
+    if (!ctx || !scope || !nobjs) {
         return QV_ERR_INVLD_ARG;
     }
 
-    return qvi_scope_nobjs(scope, obj, n);
+    return qvi_scope_nobjs(scope, obj, nobjs);
 }
 
 int
@@ -191,25 +191,23 @@ qv_scope_barrier(
     return qvi_scope_barrier(scope);
 }
 
+// TODO(skg) Add Fortran interface.
 int
-qv_scope_split(
+qv_scope_create(
     qv_context_t *ctx,
     qv_scope_t *scope,
-    int npieces,
-    int color,
+    qv_hw_obj_type_t type,
+    int nobjs,
+    qv_scope_create_hint_t hint,
     qv_scope_t **subscope
 ) {
-    if (!ctx || !scope || (npieces <= 0) | !subscope) {
+    if (!ctx || !scope || (nobjs < 0) || !subscope) {
         return QV_ERR_INVLD_ARG;
     }
 
     qv_scope_t *isubscope = nullptr;
-    // We use the sentinel value QV_HW_OBJ_LAST to differentiate between calls
-    // from split() and split_at(). Since this call doesn't have a hardware type
-    // argument, we use QV_HW_OBJ_LAST as the hardware type.
-    int rc = qvi_scope_split(
-        scope, npieces, color,
-        QV_HW_OBJ_LAST, &isubscope
+    int rc = qvi_scope_create(
+        scope, type, nobjs, hint, &isubscope
     );
     if (rc != QV_SUCCESS) {
         qvi_scope_free(&isubscope);
@@ -219,20 +217,24 @@ qv_scope_split(
 }
 
 int
-qv_scope_split_at(
+qv_scope_split(
     qv_context_t *ctx,
     qv_scope_t *scope,
-    qv_hw_obj_type_t type,
-    int group_id,
+    int npieces,
+    int color,
     qv_scope_t **subscope
 ) {
-    if (!ctx || !scope || !subscope) {
+    if (!ctx || !scope || (npieces <= 0) | !subscope) {
         return QV_ERR_INVLD_ARG;
     }
 
     qv_scope_t *isubscope = nullptr;
-    int rc = qvi_scope_split_at(
-        scope, type, group_id, &isubscope
+    // We use the sentinel value QV_HW_OBJ_LAST to differentiate between calls
+    // from split() and split_at(). Since this call doesn't have a hardware type
+    // argument, we use QV_HW_OBJ_LAST as the hardware type.
+    int rc = qvi_scope_split(
+        scope, npieces, color,
+        QV_HW_OBJ_LAST, &isubscope
     );
     if (rc != QV_SUCCESS) {
         qvi_scope_free(&isubscope);
@@ -241,23 +243,21 @@ qv_scope_split_at(
     return rc;
 }
 
-// TODO(skg) Add Fortran interface.
 int
-qv_scope_create(
+qv_scope_split_at(
     qv_context_t *ctx,
     qv_scope_t *scope,
     qv_hw_obj_type_t type,
-    int nobjs,
-    qv_scope_create_hint_t hint,
+    int group_id,
     qv_scope_t **subscope
 ) {
-    if (!ctx || !scope || (nobjs < 0) || !subscope) {
+    if (!ctx || !scope || !subscope) {
         return QV_ERR_INVLD_ARG;
     }
 
     qv_scope_t *isubscope = nullptr;
-    int rc = qvi_scope_create(
-        scope, type, nobjs, hint, &isubscope
+    int rc = qvi_scope_split_at(
+        scope, type, group_id, &isubscope
     );
     if (rc != QV_SUCCESS) {
         qvi_scope_free(&isubscope);

diff --git a/tests/test-mpi-api.c b/tests/test-mpi-api.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2022-2023 Triad National Security, LLC
+ * Copyright (c) 2022-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * This file is part of the quo-vadis project. See the LICENSE file at the
@@ -54,7 +54,7 @@ main(
     }
 
     qv_context_t *ctx = NULL;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_scope_free() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-fortapi.f90 b/tests/test-mpi-fortapi.f90
@@ -1,5 +1,5 @@
 !
-! Copyright (c) 2013-2022 Triad National Security, LLC
+! Copyright (c) 2013-2024 Triad National Security, LLC
 !                         All rights reserved.
 !
 ! This file is part of the quo-vadis project. See the LICENSE file at the
@@ -53,7 +53,7 @@ program mpi_fortapi
         print *, 'cwsize', cwsize
     end if
 
-    call qv_mpi_context_create(ctx, MPI_COMM_WORLD, info)
+    call qv_mpi_context_create(MPI_COMM_WORLD, ctx, info)
     if (info .ne. QV_SUCCESS) then
         error stop
     end if

diff --git a/tests/test-mpi-getdev.c b/tests/test-mpi-getdev.c
@@ -88,7 +88,7 @@ main(
 
     /* Create a QV context */
     qv_context_t *ctx;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-init.c b/tests/test-mpi-init.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2023 Triad National Security, LLC
+ * Copyright (c) 2020-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -46,7 +46,7 @@ main(
     }
 
     qv_context_t *ctx = NULL;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-phases.c b/tests/test-mpi-phases.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2023 Triad National Security, LLC
+ * Copyright (c) 2020-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -70,7 +70,7 @@ main(
 
     /* Create a QV context */
     qv_context_t *ctx;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-pthreads-layout.c b/tests/test-mpi-pthreads-layout.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2023 Triad National Security, LLC
+ * Copyright (c) 2020-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c)      2020 Lawrence Livermore National Security, LLC
@@ -91,7 +91,7 @@ main(void)
     }
 
     qv_context_t *mpi_ctx;
-    rc = qv_mpi_context_create(&mpi_ctx, comm);
+    rc = qv_mpi_context_create(comm, &mpi_ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-scope-create.c b/tests/test-mpi-scope-create.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2023 Triad National Security, LLC
+ * Copyright (c) 2020-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -133,7 +133,7 @@ main(
 
     /* Create a QV context */
     qv_context_t *ctx;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-scopes-affinity-preserving.c b/tests/test-mpi-scopes-affinity-preserving.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2022-2023 Triad National Security, LLC
+ * Copyright (c) 2022-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * This file is part of the quo-vadis project. See the LICENSE file at the
@@ -49,7 +49,7 @@ main(
     }
 
     qv_context_t *ctx;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));

diff --git a/tests/test-mpi-scopes.c b/tests/test-mpi-scopes.c
@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2023 Triad National Security, LLC
+ * Copyright (c) 2020-2024 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -61,7 +61,7 @@ main(
     setbuf(stdout, NULL);
 
     qv_context_t *ctx;
-    rc = qv_mpi_context_create(&ctx, comm);
+    rc = qv_mpi_context_create(comm, &ctx);
     if (rc != QV_SUCCESS) {
         ers = "qv_mpi_context_create() failed";
         qvi_test_panic("%s (rc=%s)", ers, qv_strerr(rc));