702 make reduce return pending send

[nmm0]

Fixes: #702

[codecov]

Codecov Report

Merging #714 into develop will increase coverage by 0.12%.
The diff coverage is 92.46%.

@@             Coverage Diff             @@
##           develop     #714      +/-   ##
===========================================
+ Coverage    77.42%   77.54%   +0.12%     
===========================================
  Files          656      656              
  Lines        25144    25363     +219     
===========================================
+ Hits         19468    19668     +200     
- Misses        5676     5695      +19     

Impacted Files	Coverage Δ
src/vt/collective/reduce/reduce.h	100.00% <ø> (ø)
src/vt/messaging/pending_send.h	100.00% <ø> (ø)
src/vt/objgroup/manager.h	100.00% <ø> (ø)
src/vt/objgroup/proxy/proxy_objgroup.h	100.00% <ø> (ø)
src/vt/vrt/collection/manager.h	100.00% <ø> (ø)
src/vt/vrt/collection/reducable/reducable.h	100.00% <ø> (ø)
tests/unit/termination/test_term_chaining.cc	88.63% <78.46%> (-9.88%)	⬇️
tests/unit/termination/test_term_dep_send_chain.cc	98.65% <95.41%> (-1.35%)	⬇️
src/vt/collective/reduce/reduce.impl.h	100.00% <100.00%> (ø)
src/vt/messaging/collection_chain_set.h	100.00% <100.00%> (ø)
... and 9 more

[lifflander]

@nmm0 Just heads up that PR #757 touches the reduce code and ignores the sequence ID due to lack of reasonable use.

[nmm0]

FYI the build test on mac os x is failing because of an annoying mac os x issue where the hosts file is configured in a certain way that breaks mpich: https://stackoverflow.com/questions/23112515/mpich2-gethostbyname-failed

[lifflander]

FYI the build test on mac os x is failing because of an annoying mac os x issue where the hosts file is configured in a certain way that breaks mpich: https://stackoverflow.com/questions/23112515/mpich2-gethostbyname-failed

I'm trying to debug the failure now. Something changed very recently in Github's configuration.

[nmm0]

FYI the build test on mac os x is failing because of an annoying mac os x issue where the hosts file is configured in a certain way that breaks mpich: https://stackoverflow.com/questions/23112515/mpich2-gethostbyname-failed

I'm trying to debug the failure now. Something changed very recently in Github's configuration.

FWIW this happens to me every couple of months on my mac os machine; it seems to reset the hosts file

[lifflander]

FYI the build test on mac os x is failing because of an annoying mac os x issue where the hosts file is configured in a certain way that breaks mpich: https://stackoverflow.com/questions/23112515/mpich2-gethostbyname-failed

I'm trying to debug the failure now. Something changed very recently in Github's configuration.

FWIW this happens to me every couple of months on my mac os machine; it seems to reset the hosts file

Ok, I have a issue #931/PR #932 open to try to fix it. I'm planning to add some entries to the hosts file and possible explore other options also.

[lifflander]

@PhilMiller Can you review this?
@nmm0 Can you rebase so we get the CI fix?

[PhilMiller]

Should this actually assert that condition? The handler presumably shouldn't be getting run on anywhere other than the root

[lifflander]

This is actually a bare reduce handler (the low-level interface) that is invoked by the newer higher level reduce handlers/callbacks. The low-level interface uses a single handler up the tree (including at the root). See default_op.impl.h for reference. This is the correct way to determine if the handler has made its way to the root

Anyway, @nmm0, we should switch this to a modern handler.

[nmm0]

@lifflander I wasn't aware there was a preferred handler -- which one should I use here?

[lifflander]

The reduction in this case is vt::collective::None.

1. Make the message ChainReduceMsg inherit from vt::collective::ReduceNoneMsg
2. Create a callback to a lambda:

  auto cb = vt::theCB()->makeFunc<ChainReduceMsg>([](ChainReduceMsg*){
    // action to execute on root
  });

3. Call the reduce like this:

vt::NodeType root = 0;
auto r = vt::theCollective()->global(); // if using the global reducer
r->reduce<vt::collective::None>(root, msg.get(), cb);

[nmm0]

Updated to the new interface with the latest commit

[PhilMiller]

All of the titular stuff about making collectives return PendingSend seems fine.

I see nothing in the issue or PR that motivates or explains the merging bits, though. Their logic seems sound on their face, but I don't understand the use case they fulfill.

[nmm0]

All of the titular stuff about making collectives return PendingSend seems fine.

I see nothing in the issue or PR that motivates or explains the merging bits, though. Their logic seems sound on their face, but I don't understand the use case they fulfill.

In my particular use case (contact) they are required for the "CollisionObject" interface.

The main idea is that each object has its own CollectionChainSet representing the operations performed on that object. That includes converting local data to a vt collection, building trees, and evaluating collision. In the case of evaluating collision the idea is that both CollisionObject A and CollisionObject B must have completed their previous operations (i.e. building hierarchies and building the VT collections) in order for the collision to work. So the collision detection can be thought of as dependent on both A and B's chainsets.

[PhilMiller]

All of the titular stuff about making collectives return PendingSend seems fine.
I see nothing in the issue or PR that motivates or explains the merging bits, though. Their logic seems sound on their face, but I don't understand the use case they fulfill.

In my particular use case (contact) they are required for the "CollisionObject" interface.

The main idea is that each object has its own CollectionChainSet representing the operations performed on that object. That includes converting local data to a vt collection, building trees, and evaluating collision. In the case of evaluating collision the idea is that both CollisionObject A and CollisionObject B must have completed their previous operations (i.e. building hierarchies and building the VT collections) in order for the collision to work. So the collision detection can be thought of as dependent on both A and B's chainsets.

Ok, cool, I'll buy that.

When you say "object" here, I assume you mean the physical bodies in the structures being simulated, through their dynamics and collisions?

[nmm0]

All of the titular stuff about making collectives return PendingSend seems fine.
I see nothing in the issue or PR that motivates or explains the merging bits, though. Their logic seems sound on their face, but I don't understand the use case they fulfill.

In my particular use case (contact) they are required for the "CollisionObject" interface.
The main idea is that each object has its own CollectionChainSet representing the operations performed on that object. That includes converting local data to a vt collection, building trees, and evaluating collision. In the case of evaluating collision the idea is that both CollisionObject A and CollisionObject B must have completed their previous operations (i.e. building hierarchies and building the VT collections) in order for the collision to work. So the collision detection can be thought of as dependent on both A and B's chainsets.

Ok, cool, I'll buy that.

When you say "object" here, I assume you mean the physical bodies in the structures being simulated, through their dynamics and collisions?

Exactly -- though they are also referred to as "blocks". Right now in NimbleSM there are two objects, primary and secondary though we want to target arbitrary numbers of objects. Basically in this model there are no self-collision; i.e. elements in one block will not collide with other elements in the block.

[nmm0]

Signed commits and rebased on develop

[lifflander]

Here is an overview of what got changed by this pull request:

Clones added
============
- tests/unit/termination/test_term_chaining.cc  4
- tests/unit/termination/test_term_dep_send_chain.cc  4
- src/vt/collective/reduce/reduce.h  2
- src/vt/vrt/collection/reducable/reducable.impl.h  2
- src/vt/objgroup/proxy/proxy_objgroup.impl.h  2
         

See the complete overview on Codacy