Hdf5 #101

guj · 2017-05-02T17:39:02Z

added timestep and read write.
looks like I need to merge with master? structure was changed

for using reader in current write-read test, see helloHDF5Writer.cpp at the end of write section, there is a few lines to read what was written.

chuckatkins

Needs most member variables renamed. Also please fix the HDF5 Write test

chuckatkins · 2017-05-03T00:07:35Z

examples/hello/hdf5Writer/helloHDF5Writer.cpp

-                          myCLongDoubles.data() + complexOffset);
+        int ts = 0;
+        int totalts = 2;
+        while (true)


while(true) is clunky here. Should be a for loop. Try:

for(int ts = 0; ts < totalts; ++ts)

chuckatkins · 2017-05-03T00:08:45Z

source/adios2/ADIOS.cpp

-
+    else if (type == "HDF5Reader") // -Junmin
+    {
+#if defined(ADIOS_HAVE_PHDF5) && defined(ADIOS_HAVE_MPI)


Should just be #ifdef ADIOS2_HAVE_HDF5

chuckatkins · 2017-05-03T00:10:49Z

source/adios2/engine/hdf5/HDF5Common.cpp

+    H5Pclose(_plist_id);
+}
+
+HDF5Common::~HDF5Common() {}


The empty destructor can be left out completely and a default destructor will be inserted

chuckatkins · 2017-05-03T00:12:27Z

source/adios2/engine/hdf5/HDF5Common.h

+     */
+    HDF5Common();
+
+    virtual ~HDF5Common();


No inheritance so no need to make this virtual. Also can be removed entirely since the implementation is empty and just let the compiler generate a default destructor.

chuckatkins · 2017-05-03T00:13:26Z

source/adios2/engine/hdf5/HDF5Common.h

+    int GetNumTimeSteps();
+    void WriteTimeSteps();
+
+    hid_t _plist_id, _file_id;


All member variables should be named as m_VarName not _var_name

chuckatkins · 2017-05-03T00:15:03Z

source/adios2/engine/hdf5/HDF5ReaderP.cpp

+Variable<void> *HDF5Reader::InquireVariable(const std::string &variableName,
+                                            const bool readIn)
+{
+    std::cout << "Not implemented: HDF5Reder::InquireVariable()" << std::endl;


Missing return value. Add return nullptr

chuckatkins · 2017-05-03T00:16:30Z

source/adios2/engine/hdf5/HDF5ReaderP.cpp

+}
+
+Variable<double> *
+HDF5Reader::InquireVariableDouble(const std::string &variableName, const bool)


This doesn't actually return anything

I added null return, for now.

This function has a body now, because I noticed ReadMe(), which is not inherited from any parent class, was not touched during compilation, unless I call it from a parent class function, like InquireVariableDouble().

Do you know why?

chuckatkins · 2017-05-03T00:24:42Z

source/adios2/engine/hdf5/HDF5ReaderP.h

+
+    virtual ~HDF5Reader();
+
+    Variable<void> *InquireVariable(const std::string &variableName,


All of these InquireVariableTypeName member functions would be better suited to a single template function with specializations for each type.

chuckatkins · 2017-05-03T00:25:25Z

source/adios2/engine/hdf5/HDF5ReaderP.h

+    void ReadMe(Variable<T> variable, T *values, hid_t h5type);
+
+private:
+    HDF5Common _H5File;


Use m_VarName for all data members, not _var_name

in Variable.h and VariableBase.h

re-examining with updated commits

chuckatkins

All access in user-facing code to the engines should be through the abstract engine class, not through the HDF5Writer class directly.

I think you misunderstood what I meant in the test. For the ADIOS2HDF5WriteHDF5Read tests, we want to use the ADIOS2 HDF5Writer engine to write the data but then use HDF5 directly to read the data, i.e. not the HDF5Reader engine. The ADIOS2 Read API is still in quite a bit of flux so I don't want to build tests on it yet. Besides, those will be seperate tests: Write with hdf5 -> read with adios2+hdf5, write with adios2+hdf5 -> read with adios2+hdf5, but don't bother with those yet. Currently we jsut want write with adios2+hdf5 -> read with hdf5.

Also, most of the comments from the previos review remain unaddressed.

guj · 2017-05-04T20:01:39Z

Hi Chuck, I understood your request. I just put the function ReadMe() in HDF5Reader for convenience. I assume this hdf5 direct access check will disappear after ADIOS2 read is in place. At the time, I will just simply remove that function.

guj · 2017-05-04T20:06:09Z

To be precise, ReadMe() is invoked directly through HDR5Reader, not going through the ADIOS engine at all.

chuckatkins · 2017-05-04T21:26:15Z

It will remain as a permanent test. It's part of ensuring correctness for the interoperability aspect. We need to make sure that all of the following situations work:

Write with adios2, read with hdf5
Write with adios2, read with adios2
Write with hdf5, read with adios2

Currently the second two will remain empty until the adios2 read API settles, but the first one needs to be in place now. Having the read validation test not using adios2 at all ensures that we don't create hdf5 files that are incompatible with apps only using hdf5 and not adios2.

guj · 2017-05-04T22:07:20Z

I will put a read function in the HDF5Common class. It is better?

chuckatkins · 2017-05-05T00:39:28Z

The read validation should be directly in the test and directly using hdf5, not as any component of the engine. Basically you should be able to read the data back and verify it's dimensions and contents without using anything from adios2.

guj · 2017-05-05T03:17:06Z

hdf5 read engine will call hdf5 interface too, and that is what HDF5Common is for. It has nothing to do with engine, just make hdf5 calls.
so sounds like you want to see a duplicate of HDF5Common in the test/ directory?

to read hdf directly

guj · 2017-05-05T06:15:03Z

never mind. I put a simple hdf5 read class in the TestHDF5WriteRead.cpp
to do direct hdf5 access. It should serve the purpose.

Re-examining with new changes

chuckatkins · 2017-05-05T18:56:47Z

The ADIOSWriteInterfcace test now fails. I assume this has to do with the swapping for global and local dimension parameters in the variable class.

$ ./bin/TestADIOSInterfaceWrite
[==========] Running 20 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 20 tests from ADIOSInterfaceWriteTest
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarChar1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:36: Failure
      Expected: var_i8.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarChar1x10 (1 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarShort1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:53: Failure
      Expected: var_i16.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarShort1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarInt1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:67: Failure
      Expected: var_i32.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarInt1x10 (1 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarLong1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:81: Failure
      Expected: var_i64.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarLong1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarUChar1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:97: Failure
      Expected: var_u8.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUChar1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarUShort1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:114: Failure
      Expected: var_u16.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUShort1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarUInt1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:130: Failure
      Expected: var_u32.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUInt1x10 (1 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarULong1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:146: Failure
      Expected: var_u64.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarULong1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarFloat1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:162: Failure
      Expected: var_r32.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarFloat1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarDouble1x10
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:178: Failure
      Expected: var_r64.m_LocalDimensions.size()
      Which is: 0
To be equal to: 1
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarDouble1x10 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarChar2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:193: Failure
      Expected: var_i8.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarChar2x5 (1 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarShort2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:210: Failure
      Expected: var_i16.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarShort2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarInt2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:226: Failure
      Expected: var_i32.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarInt2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarLong2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:242: Failure
      Expected: var_i64.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarLong2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarUChar2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:259: Failure
      Expected: var_u8.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUChar2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarUShort2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:277: Failure
      Expected: var_u16.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUShort2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarUInt2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:295: Failure
      Expected: var_u32.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUInt2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarULong2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:313: Failure
      Expected: var_u64.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarULong2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarFloat2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:330: Failure
      Expected: var_r32.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarFloat2x5 (0 ms)
[ RUN      ] ADIOSInterfaceWriteTest.DefineVarDouble2x5
/home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:347: Failure
      Expected: var_r64.m_LocalDimensions.size()
      Which is: 0
To be equal to: 2
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarDouble2x5 (0 ms)
[----------] 20 tests from ADIOSInterfaceWriteTest (5 ms total)

[----------] Global test environment tear-down
[==========] 20 tests from 1 test case ran. (5 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 20 tests, listed below:
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarChar1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarShort1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarInt1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarLong1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUChar1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUShort1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUInt1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarULong1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarFloat1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarDouble1x10
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarChar2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarShort2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarInt2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarLong2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUChar2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUShort2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarUInt2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarULong2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarFloat2x5
[  FAILED  ] ADIOSInterfaceWriteTest.DefineVarDouble2x5

20 FAILED TESTS
$

pnorbert · 2017-05-05T19:21:15Z

I am fixing that test in my pull request. Norbert

…

On Fri, May 5, 2017 at 2:56 PM, Chuck Atkins ***@***.***> wrote: The ADIOSWriteInterfcace test now fails. I assume this has to do with the swapping for global and local dimension parameters in the variable class. $ ./bin/TestADIOSInterfaceWrite [==========] Running 20 tests from 1 test case. [----------] Global test environment set-up. [----------] 20 tests from ADIOSInterfaceWriteTest [ RUN ] ADIOSInterfaceWriteTest.DefineVarChar1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:36: Failure Expected: var_i8.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarChar1x10 (1 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarShort1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:53: Failure Expected: var_i16.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarShort1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarInt1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:67: Failure Expected: var_i32.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarInt1x10 (1 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarLong1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:81: Failure Expected: var_i64.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarLong1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarUChar1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:97: Failure Expected: var_u8.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUChar1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarUShort1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:114: Failure Expected: var_u16.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUShort1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarUInt1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:130: Failure Expected: var_u32.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUInt1x10 (1 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarULong1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:146: Failure Expected: var_u64.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarULong1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarFloat1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:162: Failure Expected: var_r32.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarFloat1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarDouble1x10 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:178: Failure Expected: var_r64.m_LocalDimensions.size() Which is: 0 To be equal to: 1 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarDouble1x10 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarChar2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:193: Failure Expected: var_i8.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarChar2x5 (1 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarShort2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:210: Failure Expected: var_i16.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarShort2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarInt2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:226: Failure Expected: var_i32.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarInt2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarLong2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:242: Failure Expected: var_i64.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarLong2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarUChar2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:259: Failure Expected: var_u8.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUChar2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarUShort2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:277: Failure Expected: var_u16.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUShort2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarUInt2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:295: Failure Expected: var_u32.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUInt2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarULong2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:313: Failure Expected: var_u64.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarULong2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarFloat2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:330: Failure Expected: var_r32.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarFloat2x5 (0 ms) [ RUN ] ADIOSInterfaceWriteTest.DefineVarDouble2x5 /home/khq.kitware.com/chuck.atkins/Code/adios/source/hdf5/testing/adios2/interface/TestADIOSInterfaceWrite.cpp:347: Failure Expected: var_r64.m_LocalDimensions.size() Which is: 0 To be equal to: 2 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarDouble2x5 (0 ms) [----------] 20 tests from ADIOSInterfaceWriteTest (5 ms total) [----------] Global test environment tear-down [==========] 20 tests from 1 test case ran. (5 ms total) [ PASSED ] 0 tests. [ FAILED ] 20 tests, listed below: [ FAILED ] ADIOSInterfaceWriteTest.DefineVarChar1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarShort1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarInt1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarLong1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUChar1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUShort1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUInt1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarULong1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarFloat1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarDouble1x10 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarChar2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarShort2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarInt2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarLong2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUChar2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUShort2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarUInt2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarULong2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarFloat2x5 [ FAILED ] ADIOSInterfaceWriteTest.DefineVarDouble2x5 20 FAILED TESTS $ — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#101 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADGMLWXH51aVsIDHhXoge0lywydUuhulks5r23DwgaJpZM4NOdKj> .

chuckatkins

I know it looks like a lot, but really it's mostly just formatting a changes for variable names and removing large blocks of disabled code. Almost there!

chuckatkins · 2017-05-05T19:01:28Z

examples/hello/hdf5Writer/helloHDF5Writer.cpp

@@ -10,7 +10,10 @@

 #include <mpi.h>

-#include <adios2.h>
+#define ADIOS_HAVE_PHDF5 // so hdf5 related items are loaded in ADIOS_CPP.h


User-code doesn't see the internal ADIOS headers anymore. This whole include block should just be #include <adios2.h>

chuckatkins · 2017-05-05T19:03:02Z

examples/hello/hdf5Writer/helloHDF5Writer.cpp

+*/
+
+#ifndef NEVER
+        adios::HDF5Common myReader;


Remove #ifdef'd out code

chuckatkins · 2017-05-05T19:04:09Z

source/adios2/ADIOS.cpp

+    else if (type == "HDF5Reader") // -Junmin
+    {
+//#if defined(ADIOS_HAVE_PHDF5) && defined(ADIOS_HAVE_MPI)
+#ifdef ADIOS_HAVE_PHDF5


Use ADIADIOS2_HAVE_HDF5, not ADIOS_HAVE_PHDF5

chuckatkins · 2017-05-05T19:05:51Z

source/adios2/core/VariableBase.h

-                 const std::size_t elementSize, const Dims localDimensions,
-                 const Dims globalDimensions, const Dims offsets,
+                 const std::size_t elementSize, const Dims globalDimensions,
+                 const Dims localDimensions, const Dims offsets,


I think this ordering makes more sense but now it's not consistent with Variable.h. Please revert.

chuckatkins · 2017-05-05T19:06:44Z

source/adios2/engine/hdf5/HDF5Common.cpp

+}
+
+#ifdef NEVER
+template <class T>


Remove #ifdef'd code block

chuckatkins · 2017-05-05T20:16:25Z

source/adios2/engine/hdf5/HDF5WriterP.cpp

-    // H5Sclose(_memspace);
-    // H5Pclose(_plist_id);
-    H5Fclose(_file_id);
+    /*


Remove commented out code

chuckatkins · 2017-05-05T20:17:42Z

source/adios2/engine/hdf5/HDF5WriterP.cpp

    }

-    _filespace = H5Screate_simple(dimSize, dimsf.data(), NULL);
+    hid_t _filespace = H5Screate_simple(dimSize, dimsf.data(), NULL);


See https://github.com/ornladios/ADIOS2/wiki/Variables-Scope,-Functions,-and-Namespaces for local variables using camelCase

chuckatkins · 2017-05-05T20:18:19Z

source/adios2/engine/hdf5/HDF5WriterP.h

@@ -86,22 +85,30 @@ class HDF5Writer : public Engine
    void Write(const std::string variableName,
               const std::complex<long double> *values);

+    void Advance(float timeout_sec = 0.0);


See https://github.com/ornladios/ADIOS2/wiki/Variables-Scope,-Functions,-and-Namespaces for function parameters using camelCase

chuckatkins · 2017-05-05T20:18:43Z

source/adios2/engine/hdf5/HDF5WriterP.h


+    HDF5Common _H5File;


_H5File -> m_H5File

chuckatkins · 2017-05-05T20:18:56Z

source/adios2/engine/hdf5/HDF5WriterP.h


+    HDF5Common _H5File;
+
+    /*
    hid_t _plist_id, _file_id, _dset_id;
    hid_t _memspace, _filespace;


Remove commented out code

chuckatkins · 2017-05-05T20:23:57Z

Fixes #96

guj and others added 5 commits May 2, 2017 17:10

added to handle common hdf5 operations for both read/write

2f9ad95

added timestep support for read/write

ec2b621

added HDF5Reader flag for engine

6a1095a

added support to compile HDF5Common.*

16293d3

Fix formatting issues

bc34140

chuckatkins force-pushed the hdf5 branch from 929c452 to bc34140 Compare May 2, 2017 21:14

chuckatkins previously requested changes May 3, 2017

View reviewed changes

guj added 3 commits May 4, 2017 10:49

the order GlobalDimension/LocalDimension/Offset needs to be consistant

5e2ad29

in Variable.h and VariableBase.h

Updated to handle the case when local dimension is not provided

9017b90

added HDF5 native read test in TestHDF5WriterRead.cpp

59b2869

chuckatkins previously requested changes May 4, 2017

View reviewed changes

changes to conform with coding standard

1a53375

added a class in testhdf5writeread.cpp

611e0b2

to read hdf directly

chuckatkins force-pushed the hdf5 branch from ab17fe9 to 2fa03b1 Compare May 5, 2017 18:57

chuckatkins suggested changes May 5, 2017

View reviewed changes

guj added 3 commits May 6, 2017 13:39

Fix cormatting

29644df

removed used method ReadMe()

f5fe330

clang format

afafd03

Chuck Atkins and others added 7 commits May 6, 2017 13:39

HDF5: Added more tests for type information

62eecee

revert back the change on variablebase

9f24419

revised for coding standard

3614a28

H5Tequal should be called instead of =

bcbc979

removed commented out session

f80dba4

clang

c7b068a

Fix outstanding formatting issues

30e9a89

chuckatkins force-pushed the hdf5 branch from 39fe5d8 to 30e9a89 Compare May 6, 2017 17:41

chuckatkins approved these changes May 6, 2017

View reviewed changes

chuckatkins merged commit f1f1d83 into ornladios:master May 6, 2017

chuckatkins mentioned this pull request May 6, 2017

Tests for HDF5Writer engine are missing native HDF5 read validation #96

Closed


		virtual ~HDF5Reader();

		Variable<void> *InquireVariable(const std::string &variableName,

Hdf5 #101

Hdf5 #101

Conversation

guj commented May 2, 2017

chuckatkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chuckatkins left a comment

Choose a reason for hiding this comment

guj commented May 4, 2017

guj commented May 4, 2017

chuckatkins commented May 4, 2017

guj commented May 4, 2017

chuckatkins commented May 5, 2017

guj commented May 5, 2017

guj commented May 5, 2017

chuckatkins commented May 5, 2017

pnorbert commented May 5, 2017 via email

chuckatkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chuckatkins commented May 5, 2017