Improve aesthetics of completions in API prototypes

[Dan Bonachea]

We've had feedback from a few spec readers that the completions goop in each API Reference prototype is aesthetically distracting.

The boilerplate that completions adds to each prototype is the same for every function with an explicit/defaulted completion argument, but the presentation is very long-winded and obscures the more interesting (mandatory) arguments that actually differ from one function to another, which is usually what the reader consulting the reference is looking for.

Moreover, a large class of users (including all novices) never exercise generalized completion and rely solely upon the default future-based completion. For those readers, the completions boilerplate in the presentation is just distraction.

We discussed this somewhat in our 2020-09-16 meeting. We did not reach a firm resolution, so I'm opening this issue to solicit feedback on how to improve accessibility/readability of the presentation, without sacrificing correctness/precision.

My current proposal:

Define a normative(?) type alias, in the completions chapter:

using operation_cx::future = decltype(operation_cx::as_future());

and use it along with some variable renaming and slight color changes to reduce the "clutter", as follows:

Example 1: Broadcast

Before:

After:

Example 2: Reduce

Before:

After:

Discussion

I believe my proposed replacement is equally correct/precise, but (at least to my eyes) the completion boilerplate seems rather less distracting/obtrusive.

Thoughts?

CC: @Rob Egan

[Dan Bonachea]

It's worth noting that both the current and proposed declarations above are not 100% technically accurate. Specifically, consider the current form of declaration:

template <typename Completions=decltype(operation_cx::as_future())>
RType whatever(...., Completions cxs=Completions{});

The defaulted actual argument here is making the assumption that decltype(operation_cx::as_future()) yields a default-constructible type, where the default-constructed value is equivalent to operation_cx::as_future(). This property is not required by the completions specification, nor is it provided by the current implementation; if a user actually invokes decltype(operation_cx::as_future()){}, the result is currently a type error.

A second problem, as recently discussed, is we really don't want users explicitly instantiating the completions template argument; in particular, any code not relying on inference for that parameter is likely to break in the next release (we expect/hope this is an empty set). So it would be better if we did not suggest/imply a valid concrete default template parameter.

Alternate Proposal

In light of these factors, here is an alternate proposal that tries to address these shortcomings:

This form discourages explicit template argument instantiation, and provides a default argument value which is actually specified and typechecks. It also avoids the need to introduce a new alias.

Thoughts?

[Rob Egan]

I think this Alternate Proposal is more helpful than the current and first proposal. I agree that having the spec propound that a specific Completion type is required in the declaration is both confusing to the user and implies a falsehood that it must be that specific type/composition of Cxs.

Leaving it as “typename Cx=/**/” leaves it undefined, but so does “typename Cx”… As long as it is clearly covered in the Completions section that “typename Cx” means something specific here – any valid Completion type for the operation given the inputs, output, remote ops, etc. of the function. So I still recommend just leave it as “typename Cx”.

For example I wouldn’t propose to include “typename BinaryOp &&op=/**/” in the template declaration because there is no need. The BinaryOp type cannot be anything and must conform to include specific set of operator() function implementations being performed upon the type T, but otherwise doesn’t need to be of any specific type or even inherit from some other specific class.

Cx is an oddball here though. I believe it is the case that Cx must inherit from the Completions opaque class though, which by its very nature means that the users should not be building their own, though it should be fine for building their own BinaryOp. But even if the user is not encouraged to write their own Completion class, they are given the tools necessary to compose a customized Completion of their own through the operator|() functions. But there be dragons in that API which is the reason why I have chosen not to delve deep into that aspect of upcxx (yet), and have found the much simpler promise/future APIs to be fully sufficient and capable for my tasks so far.

[Dan Bonachea]

Hi @Rob Egan - thanks for the feedback.

The reason I used template<typename Cx=/**/> is because <typename Cx> alone does not type check - C++11 cannot infer template parameters from function argument default values, so the latter is not technically correct for that reason. Also we want to strongly discourage explicit instantiation of that template parameter (as the concrete types are deliberately unspecified), so the /**/ was intended to emphasize that.

But there be dragons in that API which is the reason why I have chosen not to delve deep into that aspect of upcxx (yet)

I encourage you to spend a few minutes learning about completions. I assure you it's a stable and mature feature that is ready for production use. The polishing we are doing here stems from some very minor performance optimizations we are deploying, but they really can offer some nice benefits when used correctly.

I've fleshed out my current proposal in PR #62.

[Dan Bonachea]

Factor Completions boilerplate in API declarations

• Rework to improve aesthetics and reduce clutter

Resolves issue #172

→ <<cset b60513b6514f>>