Skip to content

Conversation

@Joshua-Kloepfer
Copy link
Collaborator

  • Created test called testIntegrator to test triangles and tetrahedron with shape order one and two.
  • Added documentation regarding the ordering of nodes required for integration.
  • Corrected flawed values in integration points.

Copy link
Contributor

@cwsmith cwsmith left a comment

Choose a reason for hiding this comment

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

Looks good. A few comments are below (as discussed offline).

If the self-hosted CI passes would you prefer to merge this or wait until we add the jacobian test on build-box/simmetrix sourced meshes? I'm fine either way.

\filldraw[black] (2.5, 2.5) circle (2pt) node[anchor=south]{V2};
\end{tikzpicture}
\\
Any Counter clockwise ordering is fine ie: V0, V1, V2 and V2, V0, V1.\\
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove the text describing what alternatives could work and just document the one that we currently use.

\filldraw[black] (0, 2.5, 0) circle (2pt) node[anchor=south]{E5};
\end{tikzpicture}
\\
Vertex ordering must follow right hand rule and edge ordering must follow diagram.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep this comment/text.

@Joshua-Kloepfer
Copy link
Collaborator Author

Looks good. A few comments are below (as discussed offline).

If the self-hosted CI passes would you prefer to merge this or wait until we add the jacobian test on build-box/simmetrix sourced meshes? I'm fine either way.

I think we should wait until we add further tests to make sure that everything is working before adding to main branch.

}

int main(int argc, char *argv[]) {
Mesh mesh = Mesh::MakeCartesian3D(100, 100, 100, Element::TETRAHEDRON, 100, 100, 100, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should set the first three args from the command line so we can run a scaling test without having to recompile.

Copy link
Contributor

@cwsmith cwsmith left a comment

Choose a reason for hiding this comment

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

As discussed offline, lets merge the two mfem integrator tests, drop the use of the 'star' mesh, and rely on the cartesian mesh (with user specified entity counts).

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

Successfully merging this pull request may close these issues.

3 participants