CEED / libCEED

CEED Library: Code for Efficient Extensible Discretizations
https://libceed.org
BSD 2-Clause "Simplified" License
196 stars 46 forks source link

CeedOperator constructor #4

Closed v-dobrev closed 6 years ago

v-dobrev commented 6 years ago

In the CeedOperator constructor:

https://github.com/CEED/libCEED/blob/4c2750df89d2249806e99e5d7fcd7c2f83e5bf41/ceed.h#L140-L142

I don't think the parameters dqf and dqfT are needed, since we have the CeedEvalMode parameters in the CeedQFunction.

jedbrown commented 6 years ago

The EvalMode for QFunction is declaring the requirements of that specific QFunction. The dqf and dqfT arguments are intended to provide the action of the Jacobian and action of the transpose of the Jacobian. You can't get those from the QFunction except by finite differencing which is less accurate and relatively inefficient.

v-dobrev commented 6 years ago

Oh, my mistake, I assumed that dqf and dqfT are meant to represent the f_1 part of the integral from this comment:

https://github.com/CEED/libCEED/blob/4c2750df89d2249806e99e5d7fcd7c2f83e5bf41/ceed.h#L110-L112

On a related topic, I think it will be helpful for users if we have a type that represents all Q-functions related to a given integral expression, including the partial/quadrature assembly, the action when given the assembled quadrature data, partial/quadrature assembly of the gradient operator, action of the gradient operator, Q-function that performs the action of the operator without relying on pre-computed quadrature data, etc. Then such an object can be used as an argument to the operator constructor. This way, the operator will have access to all of these operations and will be able to perform them without asking the user to manage the different Q-functions separately, e.g. to perform assembly.

In MFEM, we call such types "Integrators", e.g. "MassIntegrator"; in the templated part of MFEM, they are called "Kernels", e.g. "MassKernel", "DiffusionKernel".

What do you guys think?

jedbrown commented 6 years ago

I'm concerned that this would be brittle. Especially in the context of nonlinear preconditioning, multiphysics, and time integration, there are arbitrarily many variants in terms of what is being stored for which consumers. I don't know how to make it systematic except to have an arbitrarily large collection indexed by some unique label. That could be built on top of libceed or included as a higher level interface later, therefore I see it as out of scope for the moment.

v-dobrev commented 6 years ago

I'm not sure I clearly understand your concern. Is it about the many choices of which input and output fields are active? Can you elaborate?

I think, to keep it simple, one operator object should compute and store at most one version of assembled data plus (for nonlinear operators) one version of assembled gradient operator data, at a time. If the set of active input fields is changed (or any other operation that requires additional assembly), the operator will free any stored data and start clean. If a user needs to have, say multiple versions of the operator at different time stages, then they'll need to manage that - we can easily support operator-duplication without copying any assembled data.

jedbrown commented 6 years ago

You asked for "a type that represents all Q-functions related" but I don't think this is necessarily a small set nor readily enumerated using some unambiguous terminology. I'd rather not introduce such a concept without a clear use case and an ontologoy for the related QFunctions.

tzanio commented 6 years ago

This is a bit difficult to parse so I may be off base here, but I think what @v-dobrev is suggesting is to allow the users to focus on the user-defined portions of D, by abstract-out and handling for them the rest of the D components (the portions that come from derivative and integration).

One use case is Poisson operator with a user-defined coefficient (BP3) -- it will be great if the user just says this is a "Poisson"-type operator and here is my coefficient....

jedbrown commented 6 years ago

How would they say "here"? As a function of physical coordinates x? Or as data that depends on other stuff and thus was computed as part of a different operator?

v-dobrev commented 6 years ago

I think we need to create a new type, e.g. CeedCoefficient, that should support at least a few coefficient sub-types:

Basically, one way to support these different types of coefficients, is to augment the set of input fields of Q-function with "coefficient fields", that will be pre-computed by the CeedOperator kernels (implemented by the backends) that will call the Q-function (e.g. to perform partial/quadrature assembly).

jedbrown commented 6 years ago

These are all expressable using QFunctions. If it's a constant, just put it in your context and use it. If it depends on coordinates, ask for coordinates and use them. If it needs to be computed from other fields, either evaluate the other fields as part of this operator or precompute and put them in qdata.

v-dobrev commented 6 years ago

That is true, you can re-write the Q-function defining the operator you are interested in and embed anything. However, I'm thinking of a user that does not want to mess with the Q-functions that define the operators in order to change, say the diffusion coefficient. I propose that we write the Q-functions defining the operators (mass, diffusion, etc) in terms of a coefficient that will be evaluated, generally, on the fly, and can be defined in the ways I described above.

I think of the coefficients as a simple way to make Q-functions (or rather, their respective operators) customizable without copying-and-modifying the whole Q-function.

jedbrown commented 6 years ago

"The whole Q-function" is just a few lines. Setting up an abstraction for computing coefficients is more complicated than writing a mass or diffusion Q-function using that coefficient. It sounds like what you're asking for is a system more like MOOSE, where kernels and coefficients can be composed and reused. That would be useful, but is way more complicated than a CeedCoefficient. I also think it may be out of scope for Ceed, but we could re-evaluate later.

v-dobrev commented 6 years ago

Anyway, I was just trying to give my take on how a user can say "... here is my coefficient ..." with regard to Tzanio's comment.

jedbrown commented 6 years ago

And I think the proposal as stated is more complicated and less extensible than what already exists so I'd rather not add the concept at this point. I'm more than happy to reconsider. I'm going to close this issue for now, but feel free to reopen if you think it warrents further discussion.

tzanio commented 6 years ago

Lets revisit this after the initial release. Using Q-functions for now is OK, but having a simpler interface for non-experts will be even better.