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

Provide centralized facilities for quadrature formulas. #4536

Merged
merged 2 commits into from Apr 13, 2022

Conversation

bangerth
Copy link
Contributor

@bangerth bangerth commented Apr 7, 2022

If we ever want to use anything other than hypercube cells, then we must also switch quadrature formulas. In other words, every occurrence of QGauss has to be replaced by something that depends on the cell type.

Rather than having lots of if or ?: statements in the code, I thought I'd use this as an opportunity to provide centralized infrastructure for these cases. That's what the first commit does. I think it makes the code easier to read in general because one no longer has to know that adding one to the polynomial degree is what one should be doing to get an appropriate quadrature formula.

The second commit shows how this can be used. There are some 30 places left that need to be looked at, but I thought I'd put this patch out already for comment, and then just work through the remaining places by hand.

(Right now, I set quadrature objects up in a way that only works for hypercubes. That's because it's not trivial to know up front what kind of cell a mesh is going to use, and the introspection object is set up pretty early. I will have to think about how to address this, but at least it will have to be addressed in only one place, rather than at every location where we currently set up quadrature formulas.)

/rebuild

@tjhei
Copy link
Member

tjhei commented Apr 10, 2022

I have a few questions/comments:

  • Why do you copy the quadratures when using them instead of taking a reference?
  • Why are tests breaking, was there a wrong degree used somewhere?
  • I like the idea of a central location instead of the manual, repeated logic like velocity+1
  • It is not clear to me yet how you will be able to change the quadrature type for different cell types this way. 😉

@bangerth
Copy link
Contributor Author

Good question about the copy -- I guess I should make it a reference. The copy of these objects is cheap (a call to memcpy on a modest amount of memory), but you're right that that is unnecessary. Will fix this.

About the failing tests: Unsure. Let me remove one of the changes and see whether that fixes anything.

As for how I plan on doing this in the end: We have a function setup_introspection() which is called from setup_dof(), which is itself called only after we have created the mesh. I'm currently initializing the quadrature formulas early on, but the initialization will have to move to a point until after the mesh is available. I imagine that that should work. (That's also where the initialization of the finite element has to move.) But I think the current patch is already a good step by itself, I think it makes the code easier to read.

@tjhei
Copy link
Member

tjhei commented Apr 12, 2022

But I think the current patch is already a good step by itself, I think it makes the code easier to read.

I agree.

@bangerth
Copy link
Contributor Author

I don't know what the issue was with the changes in the melt postprocessor I had previously changed, so I dropped this and converted a couple of other places for now. This patch now tests cleanly; I will start converting all of the other places in steps once this is merged.

@tjhei tjhei merged commit 09e4f4b into geodynamics:main Apr 13, 2022
@tjhei
Copy link
Member

tjhei commented Apr 13, 2022

👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants