Merge pull request #3787 from eisenhauer/LocalVarFix

Fixup local var reading by block with test, master branch
ornladios · Sep 5, 2023 · c289aa4 · c289aa4
2 parents fe61f77 + 65bd06c
commit c289aa4
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 17 deletions.
diff --git a/source/adios2/core/Variable.tcc b/source/adios2/core/Variable.tcc
@@ -56,15 +56,23 @@ Dims Variable<T>::DoCount() const
                         ", in call to Variable<T>::Count()");
             }
 
-            size_t *DimsPtr = (MVI->BlocksInfo)[m_BlockID].Count;
-            Dims D;
-            D.resize(MVI->Dims);
-            for (int i = 0; i < MVI->Dims; i++)
+            if (!MVI->WasLocalValue)
             {
-                D[i] = DimsPtr[i];
+                size_t *DimsPtr = (MVI->BlocksInfo)[m_BlockID].Count;
+                Dims D;
+                D.resize(MVI->Dims);
+                for (int i = 0; i < MVI->Dims; i++)
+                {
+                    D[i] = DimsPtr[i];
+                }
+                delete MVI;
+                return D;
+            }
+            else
+            {
+                delete MVI;
+                return {1};
             }
-            delete MVI;
-            return D;
         }
 
         const size_t step = !m_FirstStreamingStep ? m_Engine->CurrentStep() : lf_Step();

diff --git a/source/adios2/toolkit/format/bp5/BP5Deserializer.cpp b/source/adios2/toolkit/format/bp5/BP5Deserializer.cpp
@@ -1312,14 +1312,27 @@ bool BP5Deserializer::QueueGetSingle(core::VariableBase &variable, void *DestDat
     if (VarRec->OrigShapeID == ShapeID::LocalValue)
     {
         // Shows up as global array with one element per writer rank
-        for (size_t WriterRank = variable.m_Start[0];
-             WriterRank < variable.m_Count[0] + variable.m_Start[0]; WriterRank++)
+        if (variable.m_SelectionType == adios2::SelectionType::BoundingBox)
+        {
+            for (size_t WriterRank = variable.m_Start[0];
+                 WriterRank < variable.m_Count[0] + variable.m_Start[0]; WriterRank++)
+            {
+                (void)GetSingleValueFromMetadata(variable, VarRec, DestData, AbsStep, WriterRank);
+                DestData = (char *)DestData + variable.m_ElementSize; // use variable.m_ElementSize
+                // because it's the size in local
+                // memory, VarRec->ElementSize is
+                // the size in metadata
+            }
+        }
+        else if (variable.m_SelectionType == adios2::SelectionType::WriteBlock)
         {
+            size_t WriterRank = variable.m_BlockID;
             (void)GetSingleValueFromMetadata(variable, VarRec, DestData, AbsStep, WriterRank);
-            DestData = (char *)DestData + variable.m_ElementSize; // use variable.m_ElementSize
-                                                                  // because it's the size in local
-                                                                  // memory, VarRec->ElementSize is
-                                                                  // the size in metadata
+        }
+        else
+        {
+            helper::Throw<std::invalid_argument>("Toolkit", "format::bp::BP5Deserializer",
+                                                 "QueueGetSingle", "Unexpected selection type");
         }
         return false;
     }

diff --git a/testing/adios2/engine/bp/TestBPWriteReadLocalVariables.cpp b/testing/adios2/engine/bp/TestBPWriteReadLocalVariables.cpp
@@ -210,14 +210,34 @@ TEST_F(BPWriteReadLocalVariables, ADIOS2BPWriteReadLocal1D)
                 EXPECT_EQ(rankLocalValueData[r], static_cast<int32_t>(r));
             }
 
+            if ((engineName != "BP3") && (engineName != "BP4") && (mpiSize > 1))
+            {
+                // This test exposes a difference in behaviour between
+                // BP3/4 and BP5.  BP5 resets the variables to their
+                // default state at each BeginStep where prior engines
+                // do not.  Therefore setting the block selection in
+                // this portion causes failures in future timesteps in
+                // prior engines, but not in BP5.  So we'll just test
+                // BP5 for the moment.
+                for (size_t r = 0; r < rankLocalValueData.size(); ++r)
+                {
+                    int32_t val;
+                    var_RanksLocalValue.SetBlockSelection(r);
+                    bpReader.Get(var_RanksLocalValue, &val);
+                    EXPECT_EQ(val, static_cast<int32_t>(r));
+                }
+            }
+
+            bpReader.Get(var_RanksLocalValue, rankLocalValueData);
+
             EXPECT_TRUE(var_RanksLocalValueString);
             EXPECT_EQ(var_RanksLocalValueString.ShapeID(), adios2::ShapeID::GlobalArray);
-            EXPECT_EQ(var_RanksLocalValue.Shape().size(), 1);
-            EXPECT_EQ(var_RanksLocalValue.Shape()[0], mpiSize);
+            EXPECT_EQ(var_RanksLocalValueString.Shape().size(), 1);
+            EXPECT_EQ(var_RanksLocalValueString.Shape()[0], mpiSize);
             std::vector<std::string> rankLocalValueDataString;
             bpReader.Get(var_RanksLocalValueString, rankLocalValueDataString, adios2::Mode::Sync);
-            EXPECT_EQ(rankLocalValueData.size(), mpiSize);
-            for (size_t r = 0; r < rankLocalValueData.size(); ++r)
+            EXPECT_EQ(rankLocalValueDataString.size(), mpiSize);
+            for (size_t r = 0; r < rankLocalValueDataString.size(); ++r)
             {
                 EXPECT_EQ(rankLocalValueDataString[r], std::to_string(r));
             }