Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing: Make hdf5 test able to run in parallel and guard BP engine tests #258

Merged

Conversation

HaochengLIU
Copy link
Collaborator

@HaochengLIU HaochengLIU commented Sep 5, 2017

In this PR, I extend HDF5 native reader so that now hdf5 tests can read data
by selection(so called hyperslab selection in hdf5).
Close #262

@HaochengLIU
Copy link
Collaborator Author

@chuckatkins
Hello Chuck, would you please kindly review this PR? I extend hdf5 native reader so that hdf5 test is parallel compatible!

@chuckatkins
Copy link
Contributor

@HaochengLIU Please rebase on master to get updated CI information

io.SetEngine("HDF5Writer");

#ifdef ADIOS2_HAVE_MPI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guj @williamfgc Does the HDF5 engine actually use any transports or is the engine making calls directly to hdf5?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chuckatkins we need this if we want to reuse the HDF5 common object inside an application specific Engine (something more complex) that supports several transports. Today's engines are not first-class citizens, they are test engines for the toolkit components, that's why the logic is lightweight.

@HaochengLIU HaochengLIU force-pushed the Make-hdf5-test-run-in-parallel branch from 032ad30 to 24534f0 Compare September 5, 2017 13:00
@HaochengLIU
Copy link
Collaborator Author

Done.

@@ -605,48 +751,85 @@ TEST_F(HDF5WriteReadTest, DISABLED_HDF5WriteADIOS2HDF5Read2D2x4)
//******************************************************************************

// ADIOS2 write, native HDF5 read
TEST_F(HDF5WriteReadTest, ADIOS2HDF5WriteHDF5Read2D4x2)
TEST_F(HDF5WriteReadTest, DISABLED_ADIOS2HDF5WriteHDF5Read2D4x2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be disabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Fixed.

In this PR, I extend HDF5 native reader so that now hdf5 tests can read data
by selection(so called hyperslab selection in hdf5).
@HaochengLIU HaochengLIU force-pushed the Make-hdf5-test-run-in-parallel branch from 24534f0 to 0d1ee65 Compare September 5, 2017 14:23
@HaochengLIU HaochengLIU changed the title Testing: Make hdf5 test able to run in parallel Testing: Make hdf5 test able to run in parallel and guard BP engine tests Sep 5, 2017
@HaochengLIU HaochengLIU force-pushed the Make-hdf5-test-run-in-parallel branch 2 times, most recently from 905f066 to 6ea77d3 Compare September 6, 2017 14:30
@HaochengLIU
Copy link
Collaborator Author

@scottwittenburg
Hi Scott, it seems that CI/appveyor is not happy with me. Any ideas?

@scottwittenburg
Copy link
Collaborator

At the moment I can't understand why appveyor is building/testing your branch, as so far it's something I have only set up on my own fork of the project. Maybe @chuckatkins did some midnight work while I wasn't looking though? At any rate, I'll look into it.

@scottwittenburg
Copy link
Collaborator

I have revoked all appveyors authorizations which I control. Can you push again on this branch and see if it still tries to run that check?

@scottwittenburg
Copy link
Collaborator

Now I have found out how to log in to AppVeyor on behalf of the ornladios user, and then turn off a) building of feature branches which are part of a PR, and b) building of branches without an appveyor.yml, so I think you should be good to go for now.

Just doing an empty git commit --amend and then force pushing should be enough to check.

Remove meaningless code(AddTransport which is not implemented yet) in
HDF5 test as well.
@HaochengLIU HaochengLIU force-pushed the Make-hdf5-test-run-in-parallel branch from 6ea77d3 to 257bbe7 Compare September 6, 2017 16:00
@scottwittenburg
Copy link
Collaborator

Awesome @HaochengLIU, looks like you're all set again. Sorry for the disruption.

@HaochengLIU
Copy link
Collaborator Author

HaochengLIU commented Sep 6, 2017

@scottwittenburg
Hi Scott, never mind. I think your fix works. Thanks!

Btw, the rebuild with SSH action is grey in CI for me, saying that You need write permission to trigger build, I guess I need to be granted to some level?

@scottwittenburg
Copy link
Collaborator

scottwittenburg commented Sep 6, 2017

Yes, now that you mention it, I had a similar issue until @chuckatkins granted some extra level of access for me. I think I needed write access to the ornladios/adios2 repo. And even then, it seems it didn't update that greyed out button right away. I guess we'll need to look more deeply into how we can allow developers to rebuild with ssh without granting these write permissions.

Just to see what happens, I triggered a rebuild with ssh on this PR. Can you see if you're able to go to the test results dashboard of CircleCI, get the ssh login info, and use it to login to the build?

It may not work, as granting you write permission is what may add your public key, granting you access to ssh to the image.

@HaochengLIU
Copy link
Collaborator Author

I tried ssh -i ~/.ssh/id_rsaGithub -p 64538 34.203.38.1677 with my github ssh key from the cl website but it says that Permission denied(publickey). May I ask did I miss something here?

@scottwittenburg
Copy link
Collaborator

Well, I mentioned that it may not work, so no, you did not miss anything. I was just interested to see if it was possible for me to rebuild with ssh on your behalf. But it seems we'll need to look into the permissions issue instead, so I'll ask @chuckatkins about it when I chat with him later.

@HaochengLIU
Copy link
Collaborator Author

Ok. Thanks Scott!

@chuckatkins chuckatkins merged commit 1f8a3f3 into ornladios:master Sep 6, 2017
pnorbert added a commit to pnorbert/ADIOS2 that referenced this pull request Aug 17, 2021
…st definition at a given step. BP4 file reading provides the last definition even if BeginStep/EndStep is attempted. BP3 does not support changing attributes, it always presents the first definition of the attribute (although, in the encoded data blocks newer attributes appear but the metadata does not change).

Caveat: Modifying an attribute on non-zero rank process will have no effect as the change is not registered on the metadata aggregator.

$ ctest -R Engine.BP.BPWriteReadAttributes.WriteReadStreamMutable.BP4.Serial
Test project /home/adios/ADIOS2/build.debug
    Start 258: Engine.BP.BPWriteReadAttributes.WriteReadStreamMutable.BP4.Serial
1/1 Test ornladios#258: Engine.BP.BPWriteReadAttributes.WriteReadStreamMutable.BP4.Serial ...   Passed    0.01 sec

Stream reading:
$ ./bin/bpls -lta testing/adios2/engine/bp/bp4/foo/AttributesWriteReadMutable.bp/
Step 0:
  int32_t   var1           scalar
  double    var1\dArray    attr   = {0.1, 0.2, 0.3}
  uint32_t  var1\i32Value  attr   = 0
  int32_t   var2           {8}
  double    var2\dArray    attr   = {0.1, 0.2, 0.3}
  uint32_t  var2\i32Value  attr   = 0
Step 1:
  int32_t   var1           scalar
  double    var1\dArray    attr   = {1.1, 1.2, 1.3}    -- modified value of attribute
  uint32_t  var1\i32Value  attr   = 1                  -- modified value of attribute
  double    var2\dArray    attr   = {0.1, 0.2, 0.3}    -- attribute was not modified so last value appears
  uint32_t  var2\i32Value  attr   = 0                  -- attribute was not modified so last value appears
Step 2:
  int32_t   var1           scalar
  double    var1\dArray    attr   = {2.1, 2.2, 2.3}
  uint32_t  var1\i32Value  attr   = 2
  int32_t   var2           {8}
  double    var2\dArray    attr   = {2.1, 2.2, 2.3}
  uint32_t  var2\i32Value  attr   = 2

File reading:
$ ./bin/bpls -lA testing/adios2/engine/bp/bp4/foo/AttributesWriteReadMutable.bp/
  double    var1\dArray    attr   = {2.1, 2.2, 2.3}  -- last definition appears
  uint32_t  var1\i32Value  attr   = 2
  double    var2\dArray    attr   = {2.1, 2.2, 2.3}
  uint32_t  var2\i32Value  attr   = 2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants