From 1b8bc59e17b8f8ea75f48baaec1e28dec73366f0 Mon Sep 17 00:00:00 2001 From: wkliao Date: Mon, 8 Dec 2025 14:29:35 -0600 Subject: [PATCH 1/6] ncmpidiff.c print command-line options when first countering a difference --- src/utils/ncmpidiff/ncmpidiff.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/utils/ncmpidiff/ncmpidiff.c b/src/utils/ncmpidiff/ncmpidiff.c index 6eed1bb98..c232a2229 100644 --- a/src/utils/ncmpidiff/ncmpidiff.c +++ b/src/utils/ncmpidiff/ncmpidiff.c @@ -45,6 +45,14 @@ #define uint64 unsigned long long #endif +static int first_diff; +static char cmd_opts[1024]; + +#define PRINT_CMD_OPTS \ + if (first_diff) { \ + printf("%s\n", cmd_opts); \ + first_diff = 0; \ + } #define OOM_ERROR { \ fprintf(stderr, "Error: calloc() out of memory at line %d\n",__LINE__); \ @@ -89,6 +97,7 @@ strcat(msg, str); \ sprintf(str, "value \"%s\" vs \"%s\"\n", b1, b2); \ strcat(msg, str); \ + PRINT_CMD_OPTS \ printf("%s", msg); \ numHeadDIFF++; \ } \ @@ -119,6 +128,7 @@ sprintf(str, "value %g vs %g (difference = %e)\n", \ (double)b1[pos],(double)b2[pos],(double)(b1[pos]-b2[pos])); \ strcat(msg, str); \ + PRINT_CMD_OPTS \ printf("%s", msg); \ numHeadDIFF++; \ } \ @@ -148,6 +158,7 @@ strcat(msg, str); \ sprintf(str, "value \"%s\" vs \"%s\"\n", b1, b2); \ strcat(msg, str); \ + PRINT_CMD_OPTS \ printf("%s", msg); \ numHeadDIFF++; \ } \ @@ -178,6 +189,7 @@ sprintf(str, "value %g vs %g (difference = %e)\n", \ (double)b1[pos],(double)b2[pos],(double)(b1[pos]-b2[pos])); \ strcat(msg, str); \ + PRINT_CMD_OPTS \ printf("%s", msg); \ numHeadDIFF++; \ } \ @@ -225,6 +237,7 @@ if (pos != varsize || worst != -1) { /* diff is found */ \ double v1, v2; \ if (ndims[0] == 0) { /* scalar variable */ \ + PRINT_CMD_OPTS \ if (worst == -1) \ printf("DIFF: scalar variable \"%s\" of type \"%s\"\n", \ name[0], get_type(xtype[0])); \ @@ -245,6 +258,7 @@ diffStart[_i] = pos % shape[_i] + start[_i]; \ pos /= shape[_i]; \ } \ + PRINT_CMD_OPTS \ if (worst == -1) \ printf("DIFF: variable \"%s\" of type \"%s\" at element ["OFFFMT, \ name[0], get_type(xtype[0]), diffStart[0]); \ @@ -378,6 +392,16 @@ int main(int argc, char **argv) MPI_Comm_size(comm, &nprocs); MPI_Comm_rank(comm, &rank); + if (nprocs == 1) + sprintf(cmd_opts, "ncmpidiff", rank); + else + sprintf(cmd_opts, "Rank %d: ncmpidiff", rank); + + for (i=1; i Date: Thu, 11 Dec 2025 17:21:20 -0600 Subject: [PATCH 2/6] bug fix: buf_view.off[] is constructed based on the user buffer, buf --- src/drivers/ncmpio/ncmpio_file_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/ncmpio/ncmpio_file_io.c b/src/drivers/ncmpio/ncmpio_file_io.c index e519a3d96..da855395f 100644 --- a/src/drivers/ncmpio/ncmpio_file_io.c +++ b/src/drivers/ncmpio/ncmpio_file_io.c @@ -576,7 +576,7 @@ printf("%s at %d: buf_view count=%lld off=%lld %lld len=%lld %lld\n",__func__,__ int wkl[21]; #endif for (i=0; i Date: Thu, 11 Dec 2025 17:23:22 -0600 Subject: [PATCH 3/6] print an extra INA profile timer --- src/drivers/ncmpio/ncmpio_close.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/drivers/ncmpio/ncmpio_close.c b/src/drivers/ncmpio/ncmpio_close.c index 6f9cf561c..e93a74a44 100644 --- a/src/drivers/ncmpio/ncmpio_close.c +++ b/src/drivers/ncmpio/ncmpio_close.c @@ -151,7 +151,7 @@ ncmpio_close(void *ncdp) if (max_npairs_put > 0) { /* put npairs > 0 */ put_time = ncp->ina_time_init + ncp->ina_time_flatten; - ntimers = 4; + ntimers = 5; for (i=0; iina_time_put[i]; put_time += tt[i]; @@ -163,12 +163,12 @@ ncmpio_close(void *ncdp) MPI_Reduce(tt, max_t, ntimers+3, MPI_DOUBLE, MPI_MAX, 0, ncp->comm); put_time = max_t[ntimers+2]; if (ncp->rank == 0) - printf("%s: INA put timing %5.2f %5.2f %5.2f %5.2f %5.2f %5.2f = %5.2f\n", - __func__, max_t[ntimers],max_t[ntimers+1],max_t[0],max_t[1],max_t[2],max_t[3],put_time); + printf("%s: INA put timing %5.2f %5.2f %5.2f %5.2f %5.2f %5.2f %5.2f = %5.2f\n", + __func__, max_t[ntimers],max_t[ntimers+1],max_t[0],max_t[1],max_t[2],max_t[3],max_t[4],put_time); } if (max_npairs_get > 0) { /* get npairs > 0 */ get_time = ncp->ina_time_init + ncp->ina_time_flatten; - ntimers = 4; + ntimers = 5; for (i=0; iina_time_get[i]; get_time += tt[i]; @@ -179,8 +179,8 @@ ncmpio_close(void *ncdp) MPI_Reduce(tt, max_t, ntimers+3, MPI_DOUBLE, MPI_MAX, 0, ncp->comm); if (ncp->rank == 0) - printf("%s: INA get timing %5.2f %5.2f %5.2f %5.2f %5.2f %5.2f = %5.2f\n", - __func__, max_t[ntimers],max_t[ntimers+1],max_t[0],max_t[1],max_t[2],max_t[3],max_t[ntimers+2]); + printf("%s: INA get timing %5.2f %5.2f %5.2f %5.2f %5.2f %5.2f %5.2f = %5.2f\n", + __func__, max_t[ntimers],max_t[ntimers+1],max_t[0],max_t[1],max_t[2],max_t[3],max_t[4],max_t[ntimers+2]); } } #endif From b527fcdd8f388d068cf39de35775c3327e86956a Mon Sep 17 00:00:00 2001 From: wkliao Date: Thu, 11 Dec 2025 17:24:09 -0600 Subject: [PATCH 4/6] ncmpio_intra_node.c copying noncontiguous write buffer to contiguous is much faster --- src/drivers/ncmpio/ncmpio_intra_node.c | 107 +++++++++++++++++-------- 1 file changed, 75 insertions(+), 32 deletions(-) diff --git a/src/drivers/ncmpio/ncmpio_intra_node.c b/src/drivers/ncmpio/ncmpio_intra_node.c index 38b8c38dd..dee69a3aa 100644 --- a/src/drivers/ncmpio/ncmpio_intra_node.c +++ b/src/drivers/ncmpio/ncmpio_intra_node.c @@ -1571,7 +1571,7 @@ int ina_put(NC *ncp, { int i, j, err, mpireturn, status=NC_NOERR; char *recv_buf=NULL, *wr_buf = NULL; - MPI_Aint npairs=0, *meta=NULL, *count=NULL; + MPI_Aint npairs=0, *meta=NULL, *count=NULL, *bufAddr=NULL; MPI_Offset wr_amnt=0; #ifdef HAVE_MPI_LARGE_COUNT MPI_Count *off_ptr, *len_ptr; @@ -1738,20 +1738,24 @@ int ina_put(NC *ncp, if (do_sort && indv_sorted) { /* Interleaved offsets are found but individual offsets are already - * sorted. In this case, heap_merge() is called to merge all - * offsets into one single sorted offset list. Note count[] is - * initialized and will be used in heap_merge() + * sorted. This is commonly seen from the checkerboard domain + * partitioning pattern. In this case, heap_merge() must be called + * to merge all individually already-sorted offsets into one single + * sorted offset list. Note count[] is initialized and will be used + * in heap_merge() */ - count = (MPI_Aint*) NCI_Malloc(sizeof(MPI_Aint) *ncp->num_nonaggrs); + count = (MPI_Aint*) NCI_Malloc(sizeof(MPI_Aint)*ncp->num_nonaggrs); for (i=0; inum_nonaggrs; i++) count[i] = meta[i*3]; } /* Construct an array of buffer addresses containing a mapping of the * buffer used to receive write data from non-aggregators and the * buffer used to write to file. bufAddr[] is calculated based on the - * assumption that the write buffer is contiguous. + * assumption that the write buffer of this aggregator is contiguous, + * i.e. buf_view.is_contig being 1. For non-aggregators, their write + * data will always be received into a contiguous buffer. */ - MPI_Aint *bufAddr = (MPI_Aint*)NCI_Malloc(sizeof(MPI_Aint) * npairs); + bufAddr = (MPI_Aint*)NCI_Malloc(sizeof(MPI_Aint) * npairs); bufAddr[0] = 0; for (i=1; irank == ncp->my_aggr) ncp->ina_time_put[3] += endT - startT; + startT = endT; +#endif + /* Now all write data has been collected into recv_buf. In case of any * overlap, we must coalesce recv_buf into wr_buf using off_ptr[], * len_ptr[], and bufAddr[]. For overlapped regions, requests with @@ -1942,13 +1956,47 @@ if (fake_overlap == 0) assert(npairs == i+1); * wr_buf, a contiguous buffer, wr_buf, which will later be used in a * call to MPI-IO/PNCIO file write. */ - if (!do_sort && wr_amnt == recv_amnt) + if (!do_sort && wr_amnt == recv_amnt) { wr_buf = recv_buf; + + if (wr_buf != buf) { + /* If write data has been packed in wr_buf, a contiguous buffer, + * update buf_view before passing it to the MPI-IO/PNCIO file + * write. + */ + buf_view.size = wr_amnt; + buf_view.type = MPI_BYTE; + buf_view.is_contig = 1; + } + /* else case is when user's buffer, buf, can be used to write */ + } +#if 0 + /* Note copying write data into a contiguous buffer in most cases will + * run faster in MPI-IO and PNCIO. + */ + else if (buf_view.is_contig && !overlap) { + /* Note we can reuse bufAddr[] and len_ptr[] as buf_view.off and + * buf_view.len only when buf_view.is_contig is true, because + * bufAddr[] is constructed based on the assumption that the write + * buffer is contiguous. + */ + wr_buf = recv_buf; + buf_view.size = wr_amnt; + buf_view.type = MPI_BYTE; + buf_view.is_contig = (npairs <= 1); + buf_view.off = (MPI_Offset*)bufAddr; /* based on recv_buf */ + buf_view.len = len_ptr; + buf_view.count = npairs; + } +#endif else { /* do_sort means buffer's offsets and lengths have been moved * around in order to make file offset-length pairs monotonically - * non-decreasing. We need to copy write data into a temporary - * buffer, wr_buf, and write it to the file. + * non-decreasing. We need to re-arrange the write buffer + * accordingly by copying write data into a temporary buffer, + * wr_buf, and write it to the file. Copying write data into a + * contiguous buffer in most cases will run faster in MPI-IO and + * PNCIO. */ wr_buf = NCI_Malloc(wr_amnt); ptr = wr_buf; @@ -1957,18 +2005,23 @@ if (fake_overlap == 0) assert(npairs == i+1); memcpy(ptr, recv_buf + bufAddr[j], len_ptr[j]); ptr += len_ptr[j]; } + /* Write data has been packed in wr_buf, a contiguous buffer, + * update buf_view before passing it to the MPI-IO/PNCIO file + * write. + */ + buf_view.size = wr_amnt; + buf_view.type = MPI_BYTE; + buf_view.is_contig = 1; if (recv_buf != buf) NCI_Free(recv_buf); } - - NCI_Free(bufAddr); } /* if (npairs > 0) */ NCI_Free(meta); #if defined(PNETCDF_PROFILING) && (PNETCDF_PROFILING == 1) endT = MPI_Wtime(); - if (ncp->rank == ncp->my_aggr) ncp->ina_time_put[3] += endT - startT; + if (ncp->rank == ncp->my_aggr) ncp->ina_time_put[4] += endT - startT; #endif /* set the fileview */ @@ -1983,22 +2036,12 @@ if (fake_overlap == 0) assert(npairs == i+1); ncp->maxmem_put[4] = MAX(ncp->maxmem_put[4], mem_max); #endif - if (wr_buf != buf) { - /* If write data has been packed in wr_buf, a contiguous buffer, - * buf_view must be updated before passing it to the MPI-IO/PNCIO file - * write. - */ - buf_view.size = wr_amnt; - buf_view.type = MPI_BYTE; - buf_view.is_contig = 1; - } - /* else case is when the user's buffer, buf, can be used to write */ - /* carry out write request to file */ err = ncmpio_read_write(ncp, NC_REQ_WR, 0, buf_view, wr_buf); if (status == NC_NOERR) status = err; if (wr_buf != buf) NCI_Free(wr_buf); + if (bufAddr != NULL) NCI_Free(bufAddr); /* Must free offsets and lengths now, as they may be realloc-ed in * ina_collect_md() From 09398fa0806b5bf8923a5f23722b3a0691054b24 Mon Sep 17 00:00:00 2001 From: wkliao Date: Thu, 11 Dec 2025 17:25:17 -0600 Subject: [PATCH 5/6] testcases/varn_int.c fix varid used in testing --- test/testcases/varn_int.c | 42 +++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/test/testcases/varn_int.c b/test/testcases/varn_int.c index 4f5b2d045..c30ae244a 100644 --- a/test/testcases/varn_int.c +++ b/test/testcases/varn_int.c @@ -55,7 +55,7 @@ #define NDIMS 2 static -int check_contents_for_fail(int *buffer) +int check_contents_for_fail(char *var_name, int *buffer) { int i, nprocs; int expected[NY*NX] = {13, 13, 13, 11, 11, 10, 10, 12, 11, 11, @@ -67,10 +67,10 @@ int check_contents_for_fail(int *buffer) /* check if the contents of buf are expected */ for (i=0; i= nprocs) continue; + if (expected[i] >= nprocs+10) continue; if (buffer[i] != expected[i]) { - printf("Expected read buf[%d]=%d, but got %d\n", - i,expected[i],buffer[i]); + printf("Error: var %s expect read buf[%d]=%d, but got %d\n", + var_name, i,expected[i],buffer[i]); return 1; } } @@ -247,13 +247,24 @@ int main(int argc, char** argv) goto err_out; } } - if (nprocs > 4) MPI_Barrier(MPI_COMM_WORLD); + + if (nprocs > 4) { + /* This program was designed to run on 4 processes. If running on more + * than 4, then we need to sync/flush writes before reading, espacially + * for processes of rank >= 4. + */ + MPI_Barrier(MPI_COMM_WORLD); + err = ncmpi_flush(ncid); + CHECK_ERR + MPI_Barrier(MPI_COMM_WORLD); + } + /* read back and check contents */ memset(r_buffer, 0, NY*NX*sizeof(int)); err = ncmpi_get_var_int_all(ncid, varid[0], r_buffer); CHECK_ERR - nerrs += check_contents_for_fail(r_buffer); + nerrs += check_contents_for_fail("fix_var", r_buffer); if (nerrs > 0) goto err_out; /* permute write order */ @@ -276,16 +287,27 @@ int main(int argc, char** argv) } } + if (nprocs > 4) { + /* This program was designed to run on 4 processes. If running on more + * than 4, then we need to sync/flush writes before reading, espacially + * for processes of rank >= 4. + */ + MPI_Barrier(MPI_COMM_WORLD); + err = ncmpi_flush(ncid); + CHECK_ERR + MPI_Barrier(MPI_COMM_WORLD); + } + /* read back using get_var API and check contents */ memset(r_buffer, 0, NY*NX*sizeof(int)); err = ncmpi_get_var_int_all(ncid, varid[1], r_buffer); CHECK_ERR - nerrs += check_contents_for_fail(r_buffer); + nerrs += check_contents_for_fail("rec_var", r_buffer); if (nerrs > 0) goto err_out; /* read back using get_varn API and check contents */ for (i=0; i Date: Fri, 12 Dec 2025 13:44:45 -0500 Subject: [PATCH 6/6] Pin GHA dependencies by hash Hash-pinned dependencies are an important requirement enforced by the OpenSSF scorecard tool: https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies This commit pins all GitHub Actions workflows to a commit hash. --- .github/workflows/mac_mpich.yml | 2 +- .github/workflows/mac_openmpi.yml | 2 +- .github/workflows/ubuntu_mpich.yml | 2 +- .github/workflows/ubuntu_openmpi.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/mac_mpich.yml b/.github/workflows/mac_mpich.yml index 9a8d45cab..2982e3088 100644 --- a/.github/workflows/mac_mpich.yml +++ b/.github/workflows/mac_mpich.yml @@ -34,7 +34,7 @@ jobs: runs-on: macos-latest timeout-minutes: 60 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Set up dependencies run: | # brew install gcc diff --git a/.github/workflows/mac_openmpi.yml b/.github/workflows/mac_openmpi.yml index 34c4bd5f4..fb4b477e2 100644 --- a/.github/workflows/mac_openmpi.yml +++ b/.github/workflows/mac_openmpi.yml @@ -34,7 +34,7 @@ jobs: runs-on: macos-latest timeout-minutes: 90 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Set up dependencies run: | # brew install gcc diff --git a/.github/workflows/ubuntu_mpich.yml b/.github/workflows/ubuntu_mpich.yml index 311a10025..7e1287213 100644 --- a/.github/workflows/ubuntu_mpich.yml +++ b/.github/workflows/ubuntu_mpich.yml @@ -30,7 +30,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Set up dependencies run: | sudo apt-get update diff --git a/.github/workflows/ubuntu_openmpi.yml b/.github/workflows/ubuntu_openmpi.yml index 4648710af..6f82a2a98 100644 --- a/.github/workflows/ubuntu_openmpi.yml +++ b/.github/workflows/ubuntu_openmpi.yml @@ -30,7 +30,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 90 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Set up dependencies run: | sudo apt-get update