Skip to content

Fixes to Integration - #75

Merged
cwsmith merged 44 commits into
mainfrom
jk/FixIntegration
Oct 28, 2026
Merged

cwsmith merged 44 commits into
mainfrom
jk/FixIntegration

Conversation

@Joshua-Kloepfer

Copy link
Copy Markdown
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.

@cwsmith cwsmith left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread docs/Ordering.tex Outdated
\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
Copy Markdown
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.

Comment thread docs/Ordering.tex Outdated
\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
Copy Markdown
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
Copy Markdown
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.

Comment thread mfemTest/README.md Resolved
Comment thread mfemTest/IntegratorMfemLargeTest.cpp Outdated
}

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

Copy link
Copy Markdown
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.

@cwsmith cwsmith left a comment

Copy link
Copy Markdown
Contributor

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).

Comment thread src/MeshField_Element.hpp Fixed
Comment thread src/MeshField_Element.hpp Fixed
Comment thread src/MeshField_Element.hpp Fixed
@cwsmith
cwsmith merged commit f7c97cf into main Oct 28, 2026
6 checks passed
@cwsmith
cwsmith deleted the jk/FixIntegration branch October 28, 2026 14:13

This branch was successfully deployed

1 active deployment
asics — c7dc624f Deployed Oct 28, 2026 by cwsmith via build_and_test #27
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