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

Documentation for PA #50

Merged
merged 26 commits into from May 18, 2020
Merged

Documentation for PA #50

merged 26 commits into from May 18, 2020

Conversation

YohannDudouit
Copy link
Contributor

@YohannDudouit YohannDudouit commented Apr 16, 2020

This PR adds documentation for Partial Assembly on the MFEM web page (based on the libCEED documentation).

The options on the examples page have been modified slightly, replacing "High-performance" with "Partial assembly" (and listing this option as a discretization option rather than application). Some other minor organizational tweaks to the example page were also made.

Note that this documentation makes reference to the multigrid PR (mfem/mfem/pull/1097), which is currently in next and seems likely to be merged soon. Therefore, this PR should be merged only after #49.

Closes #41.

src/partial-assembly.md Outdated Show resolved Hide resolved
@tzanio
Copy link
Member

tzanio commented Apr 22, 2020

@barker29 and @mlstowell, can you please take a look?

src/partial-assembly.md Outdated Show resolved Hide resolved
src/partial-assembly.md Outdated Show resolved Hide resolved
src/partial-assembly.md Outdated Show resolved Hide resolved
src/partial-assembly.md Outdated Show resolved Hide resolved
src/partial-assembly.md Outdated Show resolved Hide resolved
operator can be applied on the faces. An analogous **D<sub>F</sub>** operator
is then applied at the face quadrature points.

Currently, we support partial assembly only for Gauss-Lobatto and Bernstein
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a the in for the Gauss...

Copy link
Member

@mlstowell mlstowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think it looks very good. One thing I'd like to see added is an explicit statement regarding triangles, tetrahedra, etc.. Support for Quads and Hexes are mentioned a couple times but we should be clear about the current state and/or plans for support of other element types. There's no need to go into great detail.

src/partial-assembly.md Outdated Show resolved Hide resolved
src/partial-assembly.md Outdated Show resolved Hide resolved
is then applied at the face quadrature points.

Currently, we support partial assembly only for Gauss-Lobatto and Bernstein
bases, with integrators that don't require derivatives on the faces.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be a bit more precise: "derivatives" -> "normal derivatives".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, to be completely accurate, we don't support tangential derivatives, but we could support them easily, contrary to normal derivatives that require a bit of work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I thought you added the tangential derivatives. Anyway, the code for computing them is already there, in FaceQuadratureInterpolator, so it is just a matter of storing them if they are requested. The normal derivatives, on the other hand, will require different face-restriction and different face-quadrature-interpolator.

Copy link
Member

@barker29 barker29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nice documentation that will be useful to our users. You should view all my specific comments below as suggestions - I would approve this as is even without any of my changes.

src/partial-assembly.md Outdated Show resolved Hide resolved
src/partial-assembly.md Outdated Show resolved Hide resolved
Comment on lines 120 to 121
and reference geometry. Currently, only fixed order and geometry is supported,
meaning that all the blocks of **B** are identical.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and reference geometry. Currently, only fixed order and geometry is supported,
meaning that all the blocks of **B** are identical.
and reference geometry. In the common case where the mesh has fixed polynomial order
and only one element type, then all the blocks of **B** are identical.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not also mention that this common case is the only one that is currently supported (natively) in MFEM?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping to leave the documentation generic, so that it still applies if this capability is added, but that's probably overly lazy and you're right that we should mention this.

src/partial-assembly.md Outdated Show resolved Hide resolved
@tzanio
Copy link
Member

tzanio commented May 5, 2020

@dylan-copeland do you want to mention the H(div) PA support in Examples 4 and 5 here?

Copy link
Member

@tzanio tzanio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made same small adjustments, but overall this looks great.

Thank you @YohannDudouit, @pazner, @barker29, @mlstowell!

@tzanio tzanio merged commit 7bef4c1 into master May 18, 2020
@tzanio tzanio deleted the pa-documentation branch May 18, 2020 04:54
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.

Add documentation page for Partial Assembly
8 participants