Merge pull request #339 from ludwig-cf/fix-issue-338

Code quality

.github/workflows/codeql-analysis.yml

@@ -40,7 +40,7 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@v2
+      uses: github/codeql-action/init@v3
       with:
         languages: ${{ matrix.language }}
         queries: +security-and-quality
@@ -53,4 +53,4 @@ jobs:
        make
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v2
+      uses: github/codeql-action/analyze@v3

.github/workflows/regression.yml

@@ -101,6 +101,8 @@ jobs:
       uses: actions/checkout@v4
     - name: Set up MPI
       uses: mpi4py/setup-mpi@v1
+      with:
+        mpi: openmpi
     - run: mpicc --version
     - run: cp config/github-mpicc.mk config.mk
     - run: make -j 2
@@ -118,6 +120,8 @@ jobs:
       uses: actions/checkout@v4
     - name: Set up MPI
       uses: mpi4py/setup-mpi@v1
+      with:
+        mpi: openmpi
     - run: mpicc --version
     - run: cp config/github-mpicc.mk config.mk
     - run: sed -i "s/D2Q9/D3Q15/" config.mk

codecov.yml

@@ -2,8 +2,7 @@ coverage:
   status:
     project:
       default:
-        target: 33%
-        threshold: 2%
+        informational: true
     patch:
       default:
         informational: true

src/colloids.c

@@ -109,8 +109,8 @@ __host__ void colloids_info_free(colloids_info_t * info) {
   colloids_info_cell_list_clean(info);
 
   free(info->clist);
-  if (info->map_old) free(info->map_old);
-  if (info->map_new) free(info->map_new);
+  free(info->map_old);
+  free(info->map_new);
 
   if (info->target != info) tdpAssert(tdpFree(info->target));
 
@@ -191,11 +191,16 @@ __host__ int colloids_memcpy(colloids_info_t * info, int flag) {
     assert((info->target == info));
   }
   else {
-    colloid_t * tmp;
-    tdpAssert(tdpMemcpy(&tmp, &info->target->map_new, sizeof(colloid_t **),
-			tdpMemcpyDeviceToHost));
-    tdpAssert(tdpMemcpy(tmp, info->map_new, info->nsites*sizeof(colloid_t *),
-			tdpMemcpyHostToDevice));
+    if (flag == tdpMemcpyHostToDevice) {
+      colloid_t * tmp;
+      tdpAssert(tdpMemcpy(&tmp, &info->target->map_new, sizeof(colloid_t **),
+			  tdpMemcpyDeviceToHost));
+      tdpAssert(tdpMemcpy(tmp, info->map_new, info->nsites*sizeof(colloid_t *),
+			  tdpMemcpyHostToDevice));
+    }
+    else {
+      pe_exit(info->pe, "Bad flag in colloids_memcpy()\n");
+    }
   }
 
   return 0;

src/fe_electro.c

@@ -66,8 +66,9 @@ static fe_vt_t fe_electro_hvt = {
   (fe_hvector_ft)   NULL,
   (fe_htensor_ft)   NULL,
   (fe_htensor_v_ft) NULL,
-  (fe_htensor_v_ft) NULL,
-  (fe_htensor_v_ft) NULL
+  (fe_stress_v_ft)  NULL,
+  (fe_stress_v_ft)  NULL,
+  (fe_stress_v_ft)  NULL
 };
 
 static  __constant__ fe_vt_t fe_electro_dvt = {
@@ -158,7 +159,7 @@ __host__ int fe_electro_free(fe_electro_t * fe) {
   tdpAssert( tdpGetDeviceCount(&ndevice) );
   if (ndevice > 0) tdpAssert(tdpFree(fe->target));
 
-  if (fe->mu_ref) free(fe->mu_ref);
+  free(fe->mu_ref);
   free(fe);
 
   return 0;

src/field.c

@@ -168,7 +168,7 @@ __host__ int field_free(field_t * obj) {
     tdpAssert( tdpFree(obj->target) );
   }
 
-  if (obj->data) free(obj->data);
+  free(obj->data);
 
   field_halo_free(&obj->h);
 
@@ -1672,7 +1672,7 @@ int field_io_write(field_t * field, int timestep, io_event_t * event) {
     }
 
     io->impl->free(&io);
-    io_event_report(event, meta, field->name);
+    io_event_report_write(event, meta, field->name);
   }
 
   return ifail;
@@ -1697,17 +1697,24 @@ int field_io_read(field_t * field, int timestep, io_event_t * event) {
   assert(ifail == 0);
 
   if (ifail == 0) {
+    io_event_record(event, IO_EVENT_READ);
     io->impl->read(io, filename);
+    io_event_record(event, IO_EVENT_DISAGGR);
     field_io_aggr_unpack(field, io->aggr);
     io->impl->free(&io);
+
+    if (meta->options.report) {
+      pe_info(field->pe, "MPIIO read from %s\n", filename);
+      io_event_report_read(event, meta, field->name);
+    }
   }
 
   return ifail;
 }
 
 /*****************************************************************************
  *
- * field_graph_halo_send_create
+ *  field_graph_halo_send_create
  *
  *****************************************************************************/
 

src/field_grad.c

@@ -298,11 +298,11 @@ __host__ void field_grad_free(field_grad_t * obj) {
     tdpAssert( tdpFree(obj->target) );
   }
 
-  if (obj->grad) free(obj->grad);
-  if (obj->delsq) free(obj->delsq);
-  if (obj->grad_delsq) free(obj->grad_delsq);
-  if (obj->delsq_delsq) free(obj->delsq_delsq);
-  if (obj->d_ab) free(obj->d_ab);
+  free(obj->grad);
+  free(obj->delsq);
+  free(obj->grad_delsq);
+  free(obj->delsq_delsq);
+  free(obj->d_ab);
 
   obj->field = NULL;
   free(obj);

src/io_aggregator.c

@@ -8,7 +8,7 @@
  *  Edinburgh Soft Matter and Statistical Physics Group and
  *  Edinburgh Parallel Computing Centre
  *
- *  (c) 2022 The University of Edinburgh
+ *  (c) 2022-2024 The University of Edinburgh
  *
  *  Kevin Stratford ([email protected])
  *
@@ -43,7 +43,7 @@ int io_aggregator_create(io_element_t el, cs_limits_t lim,
   return 0;
 
  err:
-  if (newaggr) free(newaggr);
+  free(newaggr);
   return -1;
 }
 

src/io_event.c

@@ -47,12 +47,45 @@ int io_event_record(io_event_t * event, io_event_record_t iorec) {
  *****************************************************************************/
 
 int io_event_report(io_event_t * event, const io_metadata_t * metadata,
-		    const char * name) {
+		    const char * name, io_event_record_t iorec) {
 
   assert(event);
   assert(metadata);
   assert(name);
 
+  switch (iorec) {
+  case IO_EVENT_READ:
+    io_event_report_read(event, metadata, name);
+    break;
+  case IO_EVENT_WRITE:
+    io_event_report_write(event, metadata, name);
+    break;
+  default:
+    /* Internal error. */
+    pe_exit(metadata->cs->pe, "Bad io event in report\n");
+  }
+
+  return 0;
+}
+
+/*****************************************************************************
+ *
+ *  io_event_report_read
+ *
+ *  Report for an i/o event in parallel.
+ *  The aggregation report might want the local data size (currently total).
+ *  The file write is the total data size of the file.
+ *
+ *  Some refinement might be wanted for multiple files.
+ *
+ *****************************************************************************/
+
+int io_event_report_read(io_event_t * event, const io_metadata_t * metadata,
+			 const char * name) {
+
+  assert(event);
+  assert(metadata);
+  assert(name);
 
   /* End of event (for reporting purposes) */
   event->time[IO_EVENT_REPORT] = MPI_Wtime();
@@ -64,10 +97,79 @@ int io_event_report(io_event_t * event, const io_metadata_t * metadata,
     const char * units = NULL;
     double dunit6 = 1.0e+06; /* Units of data size are MB */
     double dunit9 = 1.0e+09; /* Units of data size are GB */
+
     /* Times (we assume these have been collected correctly! */
+    /* Read, then disaggregate, then report */
+
+    double tr = event->time[IO_EVENT_DISAGGR] - event->time[IO_EVENT_READ];
+    double ta = event->time[IO_EVENT_REPORT] - event->time[IO_EVENT_DISAGGR];
+
+    /* Record size and total file size. */
+
+    double dr = metadata->element.datasize*metadata->element.count;
+    double ds = metadata->subfile.sizes[X]*metadata->subfile.sizes[Y]*
+                metadata->subfile.sizes[Z];
+    double db = dr*ds;
+
+    if (db > dunit9) {
+      /* Use GB */
+      units = "GB";
+      db = db/dunit9;
+    }
+    else {
+      /* Use MB */
+      units = "MB";
+      db = db/dunit6;
+    }
+    pe_info(pe, "- %10s read          %7.3f %2s in %7.3f seconds\n",
+	    name, db, units, tr);
+    pe_info(pe, "- %10s disaggregated %7.3f %2s in %7.3f seconds\n",
+	    name, db, units, ta);
+    pe_info(pe, "- %10s rate          %7.3f GB per second\n",
+	    name, dr*ds/dunit9/tr);
+  }
+
+  return 0;
+}
+
+/*****************************************************************************
+ *
+ *  io_event_report_write
+ *
+ *  The aggregation report might want the local data size (currently total).
+ *  The file write is the total data size of the file.
+ *
+ *  Some refinement might be wanted for multiple files.
+ *
+ *****************************************************************************/
+
+int io_event_report_write(io_event_t * event, const io_metadata_t * metadata,
+			  const char * name) {
+
+  assert(event);
+  assert(metadata);
+  assert(name);
+
+
+  /* End of event (for reporting purposes) */
+  event->time[IO_EVENT_REPORT] = MPI_Wtime();
+
+  if (metadata->options.report) {
+
+    pe_t * pe = metadata->cs->pe;
+
+    const char * units = NULL;
+    double dunit6 = 1.0e+06; /* Units of data size are MB */
+    double dunit9 = 1.0e+09; /* Units of data size are GB */
+
+    /* Times (we assume these have been collected correctly! */
+    /* Write: aggr is first, write is second, report last */
+
     double ta = event->time[IO_EVENT_WRITE]  - event->time[IO_EVENT_AGGR];
     double tw = event->time[IO_EVENT_REPORT] - event->time[IO_EVENT_WRITE];
+
     /* Record size and total file size. */
+
     double dr = metadata->element.datasize*metadata->element.count;
     double ds = metadata->subfile.sizes[X]*metadata->subfile.sizes[Y]*
                 metadata->subfile.sizes[Z];

src/io_event.h

@@ -9,7 +9,7 @@
  *  Edinburgh Soft Matter and Statistical Physics Group and
  *  Edinburgh Parallel Computing Centre
  *
- *  (c) 2022 The University of Edinburgh
+ *  (c) 2022-2024 The University of Edinburgh
  *
  *  Kevin Stratford ([email protected])
  *
@@ -22,6 +22,8 @@
 
 enum io_event_record_enum {
   IO_EVENT_AGGR = 0,
+  IO_EVENT_DISAGGR,
+  IO_EVENT_READ,
   IO_EVENT_WRITE,
   IO_EVENT_REPORT,
   IO_EVENT_MAX
@@ -38,6 +40,10 @@ struct io_event_s {
 
 int io_event_record(io_event_t * event, io_event_record_t iorec);
 int io_event_report(io_event_t * event, const io_metadata_t * metadata,
-		    const char * name);
+		    const char * name, io_event_record_t iorec);
+int io_event_report_read(io_event_t * event, const io_metadata_t * metadata,
+			 const char * name);
+int io_event_report_write(io_event_t * event, const io_metadata_t * metadata,
+			  const char * name);
 
 #endif

src/io_impl_mpio.c

@@ -57,7 +57,7 @@ int io_impl_mpio_create(const io_metadata_t * metadata,
   return 0;
 
  err:
-  if (mpio) free(mpio);
+  free(mpio);
 
   return -1;
 }

src/io_metadata.c

@@ -58,7 +58,7 @@ int io_metadata_create(cs_t * cs,
 
  err:
 
-  if (meta) free(meta);
+  free(meta);
   return -1;
 }
 

src/lb_data.c

@@ -1417,7 +1417,7 @@ int lb_io_write(lb_t * lb, int timestep, io_event_t * event) {
       }
 
       io->impl->free(&io);
-      io_event_report(event, meta, "dist");
+      io_event_report_write(event, meta, "dist");
     }
   }
 

src/lb_model.c

@@ -8,7 +8,7 @@
  *  Edinburgh Soft Matter and Statistical Physics Group and
  *  Edinburgh Parallel Computing Centre
  *
- *  (c) 2021-2022 The University of Edinburgh
+ *  (c) 2021-2024 The University of Edinburgh
  *
  *  Contributing authors:
  *  Kevin Stratford ([email protected])
@@ -108,9 +108,9 @@ int lb_model_free(lb_model_t * model) {
     free(model->ma);
   }
 
-  if (model->na) free(model->na);
-  if (model->cv) free(model->cv);
-  if (model->wv) free(model->wv);
+  free(model->na);
+  free(model->cv);
+  free(model->wv);
 
   *model = (lb_model_t) {0};