Composite `CeedOperator` collective context operations

[zatkins-dev]

Summary

Add an interface for collective operations over composite CeedOperator context values.

Motivation

Currently, using the CeedOperatorGetContext[Double|Int32|Boolean]Read functions on a composite CeedOperator simply returns the value from the first sub-operator that has the context field defined. While useful for fields that should be the same for all sub-operators, e.g. time or timestep, there are many circumstances where each context would have a separate value which we want to perform a collective operation over. This is especially true for contexts which may have values written within a CeedQFunction (i.e. CeedQFunctionSetContextWritable(ctx, true)).

One place where this would be useful is in checking for element inversion, domain violation, or error checking, where a single boolean value context would be set to true within a CeedQFunction if an error is detected. Right now, in order to determine if any of the sub-operators of the composite operator op set the flag with name name in the context, a user must loop over the sub-operators and do something like the following:

CeedInt       num_sub;
CeedOperator *sub_operators;
bool          value  = false, is_set = false;

CeedOperatorCompositeGetNumSub(op, &num_sub);
CeedOperatorCompositeGetSubList(op, &sub_operators);
for (CeedInt i = 0; i < num_sub; i++) {
  CeedContextFieldLabel label = NULL;
  PetscSizeT            num_values;
  const bool           *values_ceed;

  CeedOperatorGetContextFieldLabel(sub_operators[i], name, &label);
  if (label) {
    CeedOperatorGetContextBooleanRead(sub_operators[i], label, &num_values, &values_ceed);
    value  = value || values_ceed[0];
    is_set = true;
    CeedOperatorRestoreContextBooleanRead(sub_operators[i], label, &values_ceed);
  }
}

Possible Interface

All of this is open to discussion and suggestion, but I think this would be a logical approach.

Phase 1: Composite Operator Context Getter

Much of the user-side difficulty of the above approach can be alleviated by providing functions to get the values associated with a given field of each sub-operator of a composite CeedOperator. This interface would look like (using boolean fields as an example):

CeedOperatorCompositeGetContextBooleanRead(CeedOperator composite_op, CeedContextFieldLabel label, CeedInt* num_suboperator_fields, size_t **num_values, const bool ***values_ceed);
CeedOperatorCompositeRestoreContextBooleanRead(CeedOperator composite_op, CeedContextFieldLabel label, CeedInt* num_suboperator_fields, size_t *num_values, const bool ***values_ceed);

where *num_suboperator_fields would be set to the number of suboperators with the requested field, *num_values would be set to the number of values in the field (already enforced to be constant across suboperators), and *values would be an array of pointers to each of the suboperator field value data (essentially the same as the current interface, just with an extra array over suboperators). That array could easily be cached if desired, though their size likely doesn't justify it.

Using this interface, the above code would be simplified to:

CeedInt num_sub;
size_t  num_values;
bool  **values_ceed;
bool    value  = false, is_set = false;

CeedOperatorCompositeGetContextBooleanRead(op, label, &num_sub, &num_values, &values_ceed);
is_set = num_sub > 0;
for (CeedInt i = 0; i < num_sub; i++) value  = value || values_ceed[i][0];
CeedOperatorCompositeRestoreContextBooleanRead(op, label, &num_sub, &num_values, &values_ceed);

Phase 2: Collective Operation Utilities

I think that as a starting point, a generic backend function like:

CeedOperatorContextCompositeReduceGeneric(CeedOperator op, CeedContextFieldLabel field_label, CeedContextFieldType field_type, CeedCollectiveOperation operation, size_t *num_values, void *values);

where CeedCollectiveOperation is an enum mirroring the default MPI_Op values (excluding MINLOC and MAXLOC):

typedef enum {
  CEED_OPERATION_MIN,
  CEED_OPERATION_MAX,
  CEED_OPERATION_BOR,
  CEED_OPERATION_BXOR,
  CEED_OPERATION_LOR,
  CEED_OPERATION_LXOR,
  CEED_OPERATION_BAND,
  CEED_OPERATION_LAND,
  CEED_OPERATION_SUM,
  CEED_OPERATION_PROD,
} CeedCollectiveOperation;

Note, only some of the operations are valid for a given CeedContextFieldType. Specifically, following MPI convention:

[ MAX, MIN] CEED_CONTEXT_FIELD_DOUBLE, CEED_CONTEXT_FIELD_INT32
[ SUM, PROD] CEED_CONTEXT_FIELD_DOUBLE, CEED_CONTEXT_FIELD_INT32
[ LAND, LOR, LXOR] CEED_CONTEXT_FIELD_INT32, CEED_CONTEXT_FIELD_BOOL
[ BAND, BOR, BXOR] CEED_CONTEXT_FIELD_INT32

Then, the user-facing functions (note that the values array would be allocated by libCEED and freed by the user, or we could use a Get/Restore interface):

CeedOperatorContextCompositeReduceBoolean(CeedOperator op, CeedContextFieldLabel field_label, CeedCollectiveOperation operation, size_t *num_values, bool **values);
CeedOperatorContextCompositeReduceDouble(CeedOperator op, CeedContextFieldLabel field_label, CeedCollectiveOperation operation, size_t *num_values, double **values);
CeedOperatorContextCompositeReduceInt32(CeedOperator op, CeedContextFieldLabel field_label, CeedCollectiveOperation operation, size_t *num_values, int32_t **values);

These functions should check that the requested operation is valid, then apply it over each of the field_label->num_values values and store the results in a new array, which is returned to the user as *values.

[jeremylt]

I would start with a version of the interface that only does single values at first.

I think we can implement purely on the interface level at first and add backend specific implementations later if needed.

[jrwrigh]

Why not have an interface that returns the context values for every suboperator (that has the context label)? That seems like a simpler interface than having to explicit specify and implement the available reduction operations; users can do the reduction themselves or do whatever else they want to do with that information.

add backend specific implementations later if needed.

I'm not sure why that would be necessary? The data being reduced will be, at max, in the dozens, so doing the reduction on device isn't a huge benefit. Given the small size, latency will dominate over bandwidth, so transferring a few dozen values D2H would effectively be the same as a single scalar value.

[jeremylt]

Its a question of data locality. Doing it host side means pulling everything down to the host. Not great if the user wants this every KSP iteration, if that's not where they will use the value. And it would me one transfer for each sub operator, which can add up.

But honestly.... an interface to get all values of a label probably would be a cleaner fix. Just need to decide how to handle operators without the labeled context entry. Probably just skip those so the returned array of values is shorter.

[jrwrigh]

And it would me one transfer for each sub operator, which can add up.

Ah, I was imagining one transfer-per composite operator, but I realize now that would require device-side code to collate the data together before sending.

Just need to decide how to handle operators without the labeled context entry. Probably just skip those so the returned array of values is shorter.

That was my thought. If users need to know how to the values map onto specific operators, then the for loops Zach posted above works fine since they probably need to query the suboperator to determine which one exactly it is anyways.

[zatkins-dev]

These are fair points. I think there's some value to having utilities to make the context easier to work with, but I can understand not wanting to support all of these features.

[jeremylt]

I think maybe phase 1 is just a utility that gets all values of all sub operators that have the matching label, then phase 2 is later adding utility functions to do reductions for users. Phase 1 feels like enough work for a student to bite off, then Phase 2 as well?

[zatkins-dev]

Somewhat tangential, but I also think that we need to make the context registration a bit more flexible; at least allow float, int64_t, and char (read: byte) values to be registered, and add a CEED_CONTEXT_FIELD_INT and CEED_CONTEXT_FIELD_SCALAR to better support the common types of CeedInt and CeedScalar.

[jeremylt]

Honestly, sounds like a good one to add to the student queue, and we can add it ourselves if it becomes pressing

[zatkins-dev]

I think maybe phase 1 is just a utility that gets all values of all sub operators that have the matching label, then phase 2 is later adding utility functions to do reductions for users. Phase 1 feels like enough work for a student to bite off, then Phase 2 as well?

Updated the issue to reflect this separation.

[zatkins-dev]

Honestly, sounds like a good one to add to the student queue, and we can add it ourselves if it becomes pressing

Added a new issue: #2004

Should we add a label specifically for student issues?

[jeremylt]

I've been using 'good first issue', I think?