Communicator Convenience Functions

[friedmud]

Alright @permcody let's do this. You've been trash talking gatherSum() for years (including here: #7803 )

"blah, blah, Derek is an idiot for wrapping common libMesh functionality up into nice, easy to digest functions that are easy to find and insulate most people from needing to ever interact with a libMesh::Parallel::Communicator"

Tired of it. Let's make a decision here.

Let me make my arguments for keeping the convenience functions: gatherSum(), gatherMin(), gatherMax(), gatherProxyValue():

1. Let's look at the data (specifically: https://docs.google.com/spreadsheets/d/1b8vYnDsJCy1I_crmUWljGhj1IVJzfi65wYhXkdHMk68/edit?usp=sharing )

Looking at UserObject derived objects in Framework, Modules, BISON and YAK (because I happen to have those handy):

Convenience function calls: 104
Communicator call throughs: 25

So, 80% of the parallel communication in UO derived objects is captured by these convenience functions. So why wouldn't we provide simplified access to them?

BTW: Nearly 100% of the communicator call throughs in Modules are made by you ( @permcody ) in GrainTracker related objects.

So: essentially you're the only one (there are a couple of exceptions) NOT using the convenience functions!
2. We wrap libMesh functionality all the time to keep from having to hop through libMesh objects and provide a more consistent interface. For instance: MooseMesh::nNodes(), SystemBase::nVariables(), etc., etc. etc (thousands of examples). All of these could be dealt with just by getting the underlying libMesh object and calling the functions directly... but for common workflows it makes sense to provide a consistent MOOSE interface.
3. In #7803 (review) you ( @permcody ) say:

I'd rather see that the communicator object is involved and just go look at the available methods

Great idea! Let's send our users (who often don't have much programming experience, normally aren't C++ experts and most often don't have any MPI experience) deep (line 611 !!) into parallel.h ( https://github.com/libMesh/libmesh/blob/master/include/parallel/parallel.h#L611 ) to have them wrangle with what's going on there.

I can see it now on moose-users:

Guys, I went into parallel.h and got overwhelmed. All I'm trying to do is add all of the numbers together on each processor. I implemented it by doing a broadcast() then an allgather() a loop and then another broadcast()... unfortunately I can't get it to compile. Something about a Request and a Status object not working together or something. Help?

Or maybe we should send them into parallel_implementation.h to make sure they perfectly understand all of the intricacies of the parallel operations on their types? I mean, it can't be helpful at all to assume that 99.9999% of people are just working with doubles... we should make them read all of parallel_implementation.h and do a book report (including a Youtube video) before we allow them to do silly things like add numbers together.
4. The convenience functions have already been useful for insulating our users from framework changes.

Remember that _communicator is actually rather new! It's only ~2 years old or so. Before that gatherSum() etc. had different implementations. If it weren't for the convenience functions, when we implemented MultiApps and created _communicator subtle bugs would have existed in many applications for a LONG time. As it is, we changed the convenience functions in one place in MOOSE and immediately fixed everyone's applications.

This may happen again. All of this noise about shared memory MPI stuff and hierarchical intercommunicators, etc. may force more changes in the future (for instance - a gatherSum() may require summing over a local, shared-memory communicator and then a global distributed memory communicator). Do you think our users that just want to sum some numbers should have to learn all about that too?
5. The convenience functions are more "MOOSE-like". In MOOSE we are ALL ABOUT convenience functions! All of the *Interface classes are just huge collections of convenience wrappers. People are used to having everything they need rolled up through inheritance.

I mean: why even provide coupledValue()? Why not just make our users do _coupled_vars[var_name][comp]->sln()? Forcing them to do that would obviously make them understand MOOSE better...

In general: what is the purpose of MOOSE? Why do we exist? Everything you can do with MOOSE you can do with libMesh. MOOSE is supposed to be simplified and targeted. It should allow scientists and engineers stay on task by providing streamlined interfaces that make it straight forward (as possible) to create multiphysics applications.

We should always target the 80% work flows with pared down interfaces to make things easier. Including in this case.

Only the people that need more should be subjected to the complexity of parallel.h, Communicator, etc.

But whatevs. Rip it out if you want to. Let's just do it already....

And (to beat @dschwen to it)...

[dschwen]

#ImWithHim

[permcody]

Nice arguments, I mean seriously.

If I'm the only one, I'll concede. I think each of these wrappers should be
thought about a bit. None of them are perfect. We have the partial PETSc
wrapping, the partial InputParameters wrapping and the partial Communicator
wrapper. I've probably complained about all of them at one point or
another.

Let's get some other opinions from the growing team.

On Wed, Oct 5, 2016 at 8:23 PM Derek Gaston notifications@github.com
wrote:

Alright @permcody https://github.com/permcody let's do this. You've
been trash talking gatherSum() for years (including here: #7803 (comment)
#7803 (review) )

"blah, blah, Derek is an idiot for wrapping common libMesh functionality
up into nice, easy to digest functions that are easy to find and insulate
most people from needing to ever interact with a
libMesh::Parallel::Communicator"

Tired of it. Let's make a decision here.

Let me make my arguments for keeping the convenience functions:
gatherSum(), gatherMin(), gatherMax(), gatherProxyValue():

Let's look at the data (specifically:
https://docs.google.com/spreadsheets/d/1b8vYnDsJCy1I_crmUWljGhj1IVJzfi65wYhXkdHMk68/edit?usp=sharing
)

Looking at UserObject derived objects in Framework, Modules, BISON and
YAK (because I happen to have those handy):

Convenience function calls: 104
Communicator call throughs: 25

So, 80% of the parallel communication in UO derived objects is
captured by these convenience functions. So why wouldn't we provide
simplified access to them?

BTW: Nearly 100% of the communicator call throughs are made by you (
@permcody https://github.com/permcody ) in GrainTracker related
objects.

So: essentially you're the only one (there are a couple of exceptions)
NOT using the convenience functions!
2.

We wrap libMesh functionality all the time to keep from having to hop
through libMesh objects and provide a more consistent interface. For
instance: MooseMesh::nNodes(), SystemBase::nVariables(), etc., etc.
etc (thousands of examples). All of these could be dealt with just by
getting the underlying libMesh object and calling the functions directly...
but for common workflows it makes sense to provide a consistent MOOSE
interface.
3.

In #7803 (comment)
#7803 (review)
you ( @permcody https://github.com/permcody ) say:

I'd rather see that the communicator object is involved and just go
look at the available methods

Great idea! Let's send our users (who often don't have much
programming experience, normally aren't C++ experts and most often don't
have any MPI experience) deep (line 611 !!) into parallel.h (
https://github.com/libMesh/libmesh/blob/master/include/parallel/parallel.h#L611
) to have them wrangle with what's going on there.

I can see it now on moose-users:

Guys, I went into parallel.h and got overwhelmed. All I'm trying to do
is add all of the numbers together on each processor. I implemented it by
doing a broadcast() then an allgather() a loop and then another
broadcast()... unfortunately I can't get it to compile. Something about a
Request and a Status object not working together or something. Help?

Or maybe we should send them into parallel_implementation.h to make
sure they perfectly understand all of the intricacies of the parallel
operations on their types? I mean, it can't be helpful at all to assume
that 99.9999% of people are just working with doubles... we should
make them read all of parallel_implementation.h and do a book report
(including a Youtube video) before we allow them to do silly things like
add numbers together.
4.

The convenience functions have already been useful for insulating our
users from framework changes.

Remember that _communicator is actually rather new! It's only ~2 years
old or so. Before that gatherSum() etc. had different implementations.
If it weren't for the convenience functions, when we implemented MultiApps
and created _communicator subtle bugs would have existed in many
applications for a LONG time. As it is, we changed the convenience
functions in one place in MOOSE and immediately fixed everyone's
applications.

This may happen again. All of this noise about shared memory MPI stuff
and hierarchical intercommunicators, etc. may force more changes in the
future (for instance - a gatherSum() may require summing over a local,
shared-memory communicator and then a global distributed memory
communicator). Do you think our users that just want to sum some numbers
should have to learn all about that too?
5.

The convenience functions are more "MOOSE-like". In MOOSE we are ALL
ABOUT convenience functions! All of the *Interface classes are just
huge collections of convenience wrappers. People are used to having
everything they need rolled up through inheritance.

I mean: why even provide coupledValue()? Why not just make our users
do _coupled_vars[var_name][comp]->sln()? Forcing them to do that would
obviously make them understand MOOSE better...

In general: what is the purpose of MOOSE? Why do we exist? Everything you
can do with MOOSE you can do with libMesh. MOOSE is supposed to be
simplified and targeted. It should allow scientists and engineers stay
on task by providing streamlined interfaces that make it straight forward
(as possible) to create multiphysics applications.

We should always target the 80% work flows with pared down interfaces to
make things easier. Including in this case.

Only the people that need more should be subjected to the complexity of
parallel.h, Communicator, etc.

But whatevs. Rip it out if you want to. Let's just do it already....

And (to beat @dschwen https://github.com/dschwen to it)...

[image: 4a3c3e9ddb89babe1ae32689dd958171]
https://cloud.githubusercontent.com/assets/870705/19138432/24f8549e-8b4a-11e6-9e95-f8daef9fcfd4.jpg

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7820, or mute the thread
https://github.com/notifications/unsubscribe-auth/AC5XIJ0L5LP3ICvqQ7-7PywYAQh81s28ks5qxFuzgaJpZM4KPeuZ
.

[friedmud]

I just updated my comment about how much of the communicator based communication comes from grain tracker objects. It should have said "in Modules" (because that's where the grain tracker stuff is)... but it didn't.

Specifically: 11/12 of the communicator based calls in Modules come from grain tracker related objects. Those account for nearly half of the 25 communicator based calls I counted across Framework/Modules/Bison/Yak

In MOOSE the big ones that use the communicator are the Samplers (especially PointSamplerBase).

So - you're not the only one ;-) But I do think you may have a skewed perspective about how often people need to go outside the convenience functions.

We're experts at this stuff (and we deal in parallel.h all the time) so it seems like no big deal. But I truly believe that these nice, friendly, convenience functions do make it easier for our users to get their Postprocessors working properly in parallel with a minimal amount of thrashing about...

[permcody]

Any comments @idaholab/moose-team? Derek makes plenty of compelling arguments here so I'm willing to drop this and all of the other existing partial wrapper implementations in MOOSE.

[jwpeterson]

I'm willing to drop this and all of the other existing partial wrapper implementations

I think you mean you're willing to keep the wrapper implementations? Otherwise I didn't understand the thread I just read... I'm fine with the wrappers because they don't preclude using more advanced interfaces if you know how/want to.

[permcody]

Doh!
Yes, I mean keep the existing implementations.

[aeslaughter]

I vote keep them.

[permcody]

Done - we're keeping them.