Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have a GrudgeDOFArray? #347

Open
3 tasks
inducer opened this issue May 24, 2024 · 3 comments
Open
3 tasks

Have a GrudgeDOFArray? #347

inducer opened this issue May 24, 2024 · 3 comments

Comments

@inducer
Copy link
Owner

inducer commented May 24, 2024

There's a lot of awkwardness in the grudge interface from having to repeatedly specify DOF descriptors (DOFDescs). This is the case because DOFArrays don't know the DOF descriptor with which they're associated. (They can't. DOFArrays are defined in meshmode, and in order to make sense of a DOF descriptor, you need a catalog of discretizations, something like grudge's DiscretizationCollection.)

The idea here is to fix that by introducing a grudge-specific subclass of the DOFArray that does have that information. This would declutter the interface a lot, in many places. There may be some aspects of easier-said-than-done here:

  • Would we allow arithmetic like DOFArray + GrudgeDOFArray? (I'm thinking no.)
  • What happens if the user passes a GrudgeDOFArray to a meshmode function? (This is still well-defined, since it'd be a subclass. But would meshmode be able to return a GrudgeDOFArray? I'm thinking no. As a result, grudge users probably shouldn't be calling meshmode directly for anything, i.e. we'd possibly need some wrapper code. I don't know how wide-spread this would be.)
  • grudge.op was just in the process of asking users to explicitly specify DOFDescs. We would effectively re-un-deprecate the "bare" versions and re-deprecate the explicit versions. We can keep those around for a while, they just need to check that if a GrudgeDOFArray is passed in, its DOFDesc matches the passed parameter. This is mostly mechanical work, I think.

Anyone opposed?

cc @MTCam @alexfikl @a-alveyblanc @lukeolson @majosm @anderson2981 @matthiasdiener

@majosm
Copy link
Collaborator

majosm commented May 24, 2024

Only other potential annoyance that's coming to mind is having to fish out the dd from array containers for each call. Not sure how much clutter that would add?

@inducer
Copy link
Owner Author

inducer commented May 24, 2024

I think that applies only to very few operations (only divergence comes to mind). In most cases, I think we would just traverse the array container and then apply the operation to each entry separately, without ever figuring whether all entries agree on the same DOFDesc.

For the divergence, I think we will want all $d$ entries of the vector to agree on the DOFDesc, but I think that's a manageable amount of fishing.

@alexfikl
Copy link
Collaborator

What happens if the user passes a GrudgeDOFArray to a meshmode function? (This is still well-defined, since it'd be a subclass. But would meshmode be able to return a GrudgeDOFArray? I'm thinking no. As a result, grudge users probably shouldn't be calling meshmode directly for anything, i.e. we'd possibly need some wrapper code. I don't know how wide-spread this would be.)

Hm, that sounds like a lot of wrapping. I imagine most things in meshmode/discretization/ will have to be wrapped in some fashion for grudge. I'm mostly weary of the wrapping because pytential may want to add a DOF descriptor too and it just adds more boilerplate :(

From a quick grep in meshmode, not that many places initialize a DOFArray by calling the constructor, so maybe we can make more use of container traversal functions to construct things based on a "template" (which would be a GrudgeDOFArray in this case)? Hopefully arithmetic won't cause any subtle issues if we disallow DOFArray + GrudgeDOFArray.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants