Add unit tests for BoutMesh::load #2245

ZedThree · 2021-03-01T09:52:46Z

Adds various unit tests for BoutMesh::load by splitting it into smaller helper methods and testing those, plus unit tests for some other BoutMesh methods. This enables parameterised tests, for instance, checking a double-null configuration can be split across various processor combinations. Each of these tests takes less than 3ms, and it's very easy to add a new combination to test, so it would be interesting to get some more variations in.

There's still some more bits left to pull out into helper functions and test, so this is WIP till then.

It should also be possible to test the communication routines to some extent as well, as we've got MpiWrapper that allows us to use GoogleMock to test routines that use MPI.

Helps clarify which numbers are the grid sizes and which are the decomposition indices

Wrong value when called with `yind == ny`: this should be out-of-range. Never called with this value, so should not be a problem

This test is really not very clever, but it's difficult to see how to write a better one

Clean up the existing tests with helper functions, expand to more configurations

ZedThree · 2021-03-22T17:56:25Z

Now also fixes a whole host of clang-tidy warnings

github-actions

clang-tidy made some suggestions

src/mesh/impls/bout/boutmesh.cxx

github-actions · 2021-03-22T18:01:57Z

src/mesh/impls/bout/boutmesh.cxx

@@ -1181,7 +1257,7 @@ int BoutMesh::wait(comm_handle handle) {
  int ind, len;
  MPI_Status status;

-  if (ch->var_list.size() == 0) {
+  if (ch->var_list.empty()) {

    // Just waiting for a single MPI request
    mpi->MPI_Wait(ch->request, &status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->request, &status); ^ src/mesh/impls/bout/boutmesh.cxx:1242:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1242:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:7: note: Assuming the condition is true if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:3: note: Taking true branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1263:5: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->request, &status); ^

github-actions · 2021-03-22T18:01:57Z

src/mesh/impls/bout/boutmesh.cxx

  } while (ind != MPI_UNDEFINED);

  if (async_send) {
    /// Asyncronous sending: Need to check if sends have completed (frees MPI memory)
    MPI_Status async_status;

-    if (UDATA_INDEST != -1)
+    if (UDATA_INDEST != -1) {
      mpi->MPI_Wait(ch->sendreq, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1242:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1242:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1271:5: note: 'Default' branch taken. Execution continues on line 1308 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1269:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1313:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1313:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:9: note: Assuming the condition is true if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:5: note: Taking true branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1318:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq, &async_status); ^

github-actions · 2021-03-22T18:01:58Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq, &async_status);
-    if (UDATA_OUTDEST != -1)
+    }
+    if (UDATA_OUTDEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 1, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 1, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1242:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1242:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1271:5: note: 'Default' branch taken. Execution continues on line 1308 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1269:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1313:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1313:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:9: note: Assuming the condition is true if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:5: note: Taking true branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 1, &async_status); ^

github-actions · 2021-03-22T18:01:58Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq + 2, &async_status);
-    if (DDATA_OUTDEST != -1)
+    }
+    if (DDATA_OUTDEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 3, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 3, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1242:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1242:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1271:5: note: 'Default' branch taken. Execution continues on line 1308 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1269:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1313:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1313:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:9: note: Assuming the condition is false if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:5: note: Taking false branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1323:9: note: Assuming the condition is false if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1323:5: note: Taking false branch if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1326:9: note: Assuming the condition is true if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1326:5: note: Taking true branch if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1327:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 3, &async_status); ^

github-actions · 2021-03-22T18:01:58Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq + 3, &async_status);
-    if (IDATA_DEST != -1)
+    }
+    if (IDATA_DEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 4, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 4, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1242:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1242:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1271:5: note: 'Default' branch taken. Execution continues on line 1308 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1269:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1313:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1313:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:9: note: Assuming the condition is false if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:5: note: Taking false branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1323:9: note: Assuming the condition is false if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1323:5: note: Taking false branch if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1326:9: note: Assuming the condition is false if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1326:5: note: Taking false branch if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1329:9: note: Assuming the condition is true if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1329:5: note: Taking true branch if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1330:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 4, &async_status); ^

github-actions · 2021-03-22T18:01:58Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq + 4, &async_status);
-    if (ODATA_DEST != -1)
+    }
+    if (ODATA_DEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 5, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 5, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1242:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1242:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1248:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1260:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1271:5: note: 'Default' branch taken. Execution continues on line 1308 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1308:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1269:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1313:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1313:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1317:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:9: note: Assuming the condition is false if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1320:5: note: Taking false branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1323:9: note: Assuming the condition is false if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1323:5: note: Taking false branch if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1326:9: note: Assuming the condition is false if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1326:5: note: Taking false branch if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1329:9: note: Assuming the condition is false if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1329:5: note: Taking false branch if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1332:9: note: Assuming the condition is true if (ODATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1332:5: note: Taking true branch if (ODATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1333:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 5, &async_status); ^

src/mesh/impls/bout/boutmesh.cxx

src/mesh/impls/bout/boutmesh.hxx

MPI types are `int_t` in MPICH, not opaque pointers like in OpenMPI

ZedThree · 2021-03-23T09:45:55Z

clang-tidy is worried about a few MPI calls, mostly when async_send is true. I've not managed to follow the logic through myself yet, but could be something we need to look at.

ZedThree · 2021-03-23T12:17:27Z

Not really related to this PR, but I think we can now squash BoutMesh::load into the constructor and remove Mesh::load entirely. It looks like we need to remove the call to Mesh::getCoordinates at the end of load to initialise the Coordinates, and rely on fields creating them as needed. All the tests pass, but maybe there's still some cases that this would break.

johnomotani · 2021-03-23T12:50:27Z

maybe there's still some cases that this would break.

I can't think of any immediately. Anything that uses Coordinates members will need to get a pointer to some Coordinates using Mesh::getCoordinates() (either directly or through Field*::getCoordinates()), so I think Coordinates should always be created when they are needed.

ZedThree · 2021-03-23T12:54:18Z

I guess I was thinking of if we create a staggered field before anything else -- but that issue is sorted now, right?

I'll make another PR for that change, this one is already way, way bigger than I was expecting/hoping!

johnomotani · 2021-03-23T13:19:23Z

I guess I was thinking of if we create a staggered field before anything else -- but that issue is sorted now, right?

Good point, I hadn't thought about that! But I think it's OK, when we create staggered Coordinates, they'll just get/create the CELL_CENTRE ones first - I think we fixed the problem with initialising things in the coords_map.

BOUT-dev/src/mesh/mesh.cxx

Lines 433 to 444 in e7920b7

    
           std::shared_ptr<Coordinates> Mesh::createDefaultCoordinates(const CELL_LOC location, 
        
               bool force_interpolate_from_centre) { 
        
             if (location == CELL_CENTRE || location == CELL_DEFAULT) { 
        
               // Initialize coordinates from input 
        
               return std::make_shared<Coordinates>(this, options); 
        
             } else { 
        
               // Interpolate coordinates from CELL_CENTRE version 
        
               return std::make_shared<Coordinates>(this, options, location, 
        
                   getCoordinates(CELL_CENTRE), force_interpolate_from_centre); 
        
             } 
        
           }

…aries Add Mesh method to get all possible boundary regions

github-actions

clang-tidy made some suggestions

github-actions · 2021-05-07T14:12:13Z

include/bout/mesh.hxx

@@ -942,7 +948,7 @@ class Mesh {
  // The maxregionblocksize to use when creating the default regions.
  // Can be set in the input file and the global default is set by,
  // MAXREGIONBLOCKSIZE in include/bout/region.hxx
-  int maxregionblocksize;
+  int maxregionblocksize{MAXREGIONBLOCKSIZE};


warning: member variable maxregionblocksize has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]

int maxregionblocksize{MAXREGIONBLOCKSIZE}; ^

github-actions · 2021-05-07T14:12:14Z

src/mesh/impls/bout/boutmesh.cxx

@@ -73,113 +77,42 @@ BoutMesh::~BoutMesh() {
  clear_handles();

  // Delete the boundary regions
-  for (const auto &bndry : boundary)
+  for (const auto& bndry : boundary) {
    delete bndry;


warning: deleting a pointer through a type that is not marked gsl::owner<>; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

delete bndry; ^ src/mesh/impls/bout/boutmesh.cxx:80:8: note: variable declared here for (const auto& bndry : boundary) { ^

github-actions · 2021-05-07T14:12:14Z

src/mesh/impls/bout/boutmesh.cxx

    delete bndry;
-  for (const auto &bndry : par_boundary)
+  }
+  for (const auto& bndry : par_boundary) {
    delete bndry;


warning: deleting a pointer through a type that is not marked gsl::owner<>; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

delete bndry; ^ src/mesh/impls/bout/boutmesh.cxx:83:8: note: variable declared here for (const auto& bndry : par_boundary) { ^

github-actions · 2021-05-07T14:12:14Z

src/mesh/impls/bout/boutmesh.cxx

@@ -1181,7 +1318,7 @@ int BoutMesh::wait(comm_handle handle) {
  int ind, len;
  MPI_Status status;

-  if (ch->var_list.size() == 0) {
+  if (ch->var_list.empty()) {

    // Just waiting for a single MPI request
    mpi->MPI_Wait(ch->request, &status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->request, &status); ^ src/mesh/impls/bout/boutmesh.cxx:1303:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1303:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:7: note: Assuming the condition is true if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:3: note: Taking true branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1324:5: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->request, &status); ^

github-actions · 2021-05-07T14:12:14Z

src/mesh/impls/bout/boutmesh.cxx

  } while (ind != MPI_UNDEFINED);

  if (async_send) {
    /// Asyncronous sending: Need to check if sends have completed (frees MPI memory)
    MPI_Status async_status;

-    if (UDATA_INDEST != -1)
+    if (UDATA_INDEST != -1) {
      mpi->MPI_Wait(ch->sendreq, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1303:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1303:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1332:5: note: 'Default' branch taken. Execution continues on line 1369 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1330:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1374:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1374:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:9: note: Assuming the condition is true if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:5: note: Taking true branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1379:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq, &async_status); ^

github-actions · 2021-05-07T14:12:15Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq + 2, &async_status);
-    if (DDATA_OUTDEST != -1)
+    }
+    if (DDATA_OUTDEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 3, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 3, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1303:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1303:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1332:5: note: 'Default' branch taken. Execution continues on line 1369 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1330:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1374:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1374:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1381:9: note: Assuming the condition is false if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1381:5: note: Taking false branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1384:9: note: Assuming the condition is false if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1384:5: note: Taking false branch if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1387:9: note: Assuming the condition is true if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1387:5: note: Taking true branch if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1388:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 3, &async_status); ^

github-actions · 2021-05-07T14:12:15Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq + 3, &async_status);
-    if (IDATA_DEST != -1)
+    }
+    if (IDATA_DEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 4, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 4, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1303:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1303:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1332:5: note: 'Default' branch taken. Execution continues on line 1369 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1330:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1374:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1374:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1381:9: note: Assuming the condition is false if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1381:5: note: Taking false branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1384:9: note: Assuming the condition is false if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1384:5: note: Taking false branch if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1387:9: note: Assuming the condition is false if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1387:5: note: Taking false branch if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1390:9: note: Assuming the condition is true if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1390:5: note: Taking true branch if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1391:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 4, &async_status); ^

github-actions · 2021-05-07T14:12:15Z

src/mesh/impls/bout/boutmesh.cxx

      mpi->MPI_Wait(ch->sendreq + 4, &async_status);
-    if (ODATA_DEST != -1)
+    }
+    if (ODATA_DEST != -1) {
      mpi->MPI_Wait(ch->sendreq + 5, &async_status);


warning: Request has no matching nonblocking call. [clang-analyzer-optin.mpi.MPI-Checker]

mpi->MPI_Wait(ch->sendreq + 5, &async_status); ^ src/mesh/impls/bout/boutmesh.cxx:1303:7: note: Assuming the condition is false if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1303:3: note: Taking false branch if (handle == nullptr) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:7: note: Assuming field 'in_progress' is true if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1309:3: note: Taking false branch if (!ch->in_progress) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:7: note: Assuming the condition is false if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1321:3: note: Taking false branch if (ch->var_list.empty()) { ^ src/mesh/impls/bout/boutmesh.cxx:1332:5: note: 'Default' branch taken. Execution continues on line 1369 switch (ind) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:9: note: Assuming the condition is false if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1369:5: note: Taking false branch if (ind != MPI_UNDEFINED) { ^ src/mesh/impls/bout/boutmesh.cxx:1330:3: note: Loop condition is false. Exiting loop do { ^ src/mesh/impls/bout/boutmesh.cxx:1374:7: note: Assuming field 'async_send' is true if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1374:3: note: Taking true branch if (async_send) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:9: note: Assuming the condition is false if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1378:5: note: Taking false branch if (UDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1381:9: note: Assuming the condition is false if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1381:5: note: Taking false branch if (UDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1384:9: note: Assuming the condition is false if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1384:5: note: Taking false branch if (DDATA_INDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1387:9: note: Assuming the condition is false if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1387:5: note: Taking false branch if (DDATA_OUTDEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1390:9: note: Assuming the condition is false if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1390:5: note: Taking false branch if (IDATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1393:9: note: Assuming the condition is true if (ODATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1393:5: note: Taking true branch if (ODATA_DEST != -1) { ^ src/mesh/impls/bout/boutmesh.cxx:1394:7: note: Request has no matching nonblocking call. mpi->MPI_Wait(ch->sendreq + 5, &async_status); ^

github-actions · 2021-05-07T14:12:15Z

src/mesh/impls/bout/boutmesh.cxx

@@ -1806,11 +1907,27 @@ BoutMesh::BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, i
  addBoundaryRegions();
 }

+BoutMesh::BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, int nxpe,


warning: constructor does not initialize these fields: MX, MY, MZ, MYSUB, MXSUB, MZSUB, jyseps1_1, jyseps2_1, jyseps1_2, jyseps2_2, ixseps_inner, ixseps_outer, ixseps_upper, ixseps_lower, ny_inner, TS_up_in, TS_up_out, TS_down_in, TS_down_out, UDATA_INDEST, UDATA_OUTDEST, UDATA_XSPLIT, DDATA_INDEST, DDATA_OUTDEST, DDATA_XSPLIT, IDATA_DEST, ODATA_DEST, TwistShift, zperiod, ZMIN, ZMAX [cppcoreguidelines-pro-type-member-init]

BoutMesh::BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, int nxpe, ^

github-actions · 2021-05-07T14:12:16Z

src/mesh/impls/bout/boutmesh.hxx

-           int pe_xind, int pe_yind);
+           int pe_xind, int pe_yind, bool create_topology = true, bool symmetric_X = true,
+           bool symmetric_Y = true);
+  BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, int nxpe, int nype,


warning: function BoutMesh::BoutMesh has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, int nxpe, int nype, ^ /github/workspace/src/mesh/impls/bout/boutmesh.cxx:1910:11: note: the definition seen here BoutMesh::BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, int nxpe, ^ src/mesh/impls/bout/boutmesh.hxx:214:3: note: differing parameters are named here: ('periodic_X'), in definition: ('periodicX_') BoutMesh(int input_nx, int input_ny, int input_nz, int mxg, int myg, int nxpe, int nype, ^

johnomotani

Looks good to me, although I only skimmed over the unit test files very quickly.

Having test coverage of BoutMesh is awesome, thanks @ZedThree! 🎉

src/mesh/impls/bout/boutmesh.cxx

tests/unit/mesh/test_boutmesh.cxx

tests/unit/mesh/test_mesh.cxx

* next: (102 commits) Docs: Fix sphinx conf.py for breathe 4.30 CMake: Print link to generated docs after completion CMake: Fix typo in docs comment Move constraint creation to utility method CMake: Default to not building the docs CMake: Be explicit that docs aren't built in `all` CMake: Fix typo Make method parameter names consistent (clang-tidy fix) Check version of SUNDIALS is new enough for constraints Option to set positivity constraints in CVODE Option to change max nonlinear iterations in CVODE GHA: Don't build the docs for other Github actions either GHA: Don't build docs on github CMake: Build documentation [skip ci] Apply black changes Update installation suggestions for Marconi/gnu fix order of date and time Fix compilation Faster recompile Remove checks for old behaviour of bool conversion ...

ZedThree · 2021-05-10T13:26:01Z

Thanks @johnomotani , sorted your comments

johnomotani

Looks good to me 👍

ZedThree · 2021-05-10T15:04:39Z

Failing test is just readthedocs, should be fixed in #2310

ZedThree added 30 commits February 26, 2021 15:00

Pull out duplicated BoutMesh::load Y-decomposition code

5f9227b

Use local variable for num_local_y_points

a369678

Parameterise BoutMesh decomposition test

675abca

Split out routine from BoutMesh for setting y-decomposition indices

6788767

Use BoutMesh::DecompositionIndices in tests

7c8bf91

Helps clarify which numbers are the grid sizes and which are the decomposition indices

Break out BoutMesh methods for selecting processor split

f147199

Apply clang-format to BoutMesh test

ef392fa

Parameterise BoutMesh::findProcessorSplit test

3c676d6

Add tests for BoutMesh::PROC_NUM

35c22cb

Remove some duplicated docstrings

30ea472

Make BoutMesh::YPROC,XPROC const

6d45b83

Fix minor bug in BoutMesh::YPROC

f1c4fd9

Wrong value when called with `yind == ny`: this should be out-of-range. Never called with this value, so should not be a problem

Add unit tests for BoutMesh::XPROC,YPROC

9600985

Add unit tests for BoutMesh::get{Local,Global}{X,Y,Z}Index

9e3de9b

Add some basic tests for first{X,Y}/last{X,Y}

68a48ef

Add a unit test for Mesh::msg_len

f4bf849

Add unit test for BoutMesh::default_connections

62680e9

This test is really not very clever, but it's difficult to see how to write a better one

Add unit tests for Mesh::hasRegion*

b1777fc

Check connection information directly in BoutMesh unit tests

6a57194

Rename BoutMesh::DecompositionIndices

42de82b

Add unit test for BoutMesh::topology() on a simple single null

8959d19

Move some BoutMesh docstrings to header

2e2a950

Add SVG for topology schematic

2103328

Expand on BOUT++ topology documentation

d394c21

Add setter for BoutMesh separatrix indices

e6865cf

Parameterise BoutMesh setYDecompositionIndices tests

e6387e8

Add test helper functions for constructing various mesh topologies

2a9de12

Add better BoutMesh::topology tests

7cfe450

Clean up the existing tests with helper functions, expand to more configurations

Pull out repeated code from BoutMesh constructor/load

fe5b42a

Pull out BoutMesh methods for creating the boundary regions

32f757c

ZedThree added 2 commits March 22, 2021 17:55

Use default initialisation of member

f7a765a

Use same argument name in declaration and definition

073ce7c

github-actions bot reviewed Mar 22, 2021

View reviewed changes

ZedThree added 2 commits March 23, 2021 09:32

Fix initialisation of MPI types

85a4ec3

MPI types are `int_t` in MPICH, not opaque pointers like in OpenMPI

Avoid warning about null this in GCC11

6706f78

ZedThree mentioned this pull request Mar 23, 2021

Make input Options case sensitive and error on unused Options #2210

Merged

ZedThree mentioned this pull request Apr 19, 2021

Mesh branch cut documentation #25

Closed

ZedThree and others added 3 commits May 7, 2021 11:05

Add Mesh method to get the set of all possible boundaries

1f1a79a

Print possible boundary regions during startup

3f1c454

[skip ci] Apply black changes

9bdcf0c

ZedThree mentioned this pull request May 7, 2021

Add Mesh method to get all possible boundary regions #2307

Merged

Merge pull request #2307 from boutproject/boutmesh-get-possible-bound…

3f0a33b

…aries Add Mesh method to get all possible boundary regions

github-actions bot reviewed May 7, 2021

View reviewed changes

johnomotani previously approved these changes May 9, 2021

View reviewed changes

src/mesh/impls/bout/boutmesh.cxx Outdated Show resolved Hide resolved

src/mesh/impls/bout/boutmesh.cxx Outdated Show resolved Hide resolved

tests/unit/mesh/test_boutmesh.cxx Outdated Show resolved Hide resolved

tests/unit/mesh/test_mesh.cxx Outdated Show resolved Hide resolved

ZedThree added 4 commits May 10, 2021 12:05

Only check local y points if we have multiple processors in Y

da98145

Fix a couple of typos in tests

10c9e1d

Remove extraneous this-> when setting member variables

45e9af6

ZedThree dismissed johnomotani’s stale review via 45e9af6 May 10, 2021 13:21

johnomotani approved these changes May 10, 2021

View reviewed changes

ZedThree merged commit 5800806 into next May 10, 2021

ZedThree deleted the split-boutmesh-load branch May 10, 2021 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit tests for BoutMesh::load #2245

Add unit tests for BoutMesh::load #2245

ZedThree commented Mar 1, 2021

ZedThree commented Mar 22, 2021

github-actions bot left a comment

github-actions bot Mar 22, 2021

github-actions bot Mar 22, 2021

github-actions bot Mar 22, 2021

github-actions bot Mar 22, 2021

github-actions bot Mar 22, 2021

github-actions bot Mar 22, 2021

ZedThree commented Mar 23, 2021

ZedThree commented Mar 23, 2021

johnomotani commented Mar 23, 2021

ZedThree commented Mar 23, 2021

johnomotani commented Mar 23, 2021

github-actions bot left a comment

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

github-actions bot May 7, 2021

johnomotani left a comment

ZedThree commented May 10, 2021

johnomotani left a comment

ZedThree commented May 10, 2021

Add unit tests for BoutMesh::load #2245

Add unit tests for BoutMesh::load #2245

Conversation

ZedThree commented Mar 1, 2021

ZedThree commented Mar 22, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Mar 22, 2021

Choose a reason for hiding this comment

github-actions bot Mar 22, 2021

Choose a reason for hiding this comment

github-actions bot Mar 22, 2021

Choose a reason for hiding this comment

github-actions bot Mar 22, 2021

Choose a reason for hiding this comment

github-actions bot Mar 22, 2021

Choose a reason for hiding this comment

github-actions bot Mar 22, 2021

Choose a reason for hiding this comment

ZedThree commented Mar 23, 2021

ZedThree commented Mar 23, 2021

johnomotani commented Mar 23, 2021

ZedThree commented Mar 23, 2021

johnomotani commented Mar 23, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

github-actions bot May 7, 2021

Choose a reason for hiding this comment

johnomotani left a comment

Choose a reason for hiding this comment

ZedThree commented May 10, 2021

johnomotani left a comment

Choose a reason for hiding this comment

ZedThree commented May 10, 2021