Skip to content

Conversation

@GiudGiud
Copy link
Contributor

as MOOSE has a test using edge3
refs #4257

@moosebuild
Copy link

moosebuild commented Oct 27, 2025

Job Coverage, step Generate coverage on 63dba73 wanted to post the following:

Coverage

739e36 #4292 63dba7
Total Total +/- New
Rate 65.00% 65.01% +0.00% 100.00%
Hits 76957 76963 +6 1
Misses 41434 41429 -5 0

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@roystgnr
Copy link
Member

The "Test mac" failure is a PETSc environment issue and unrelated. The "Distributed make check sweep (odd)" failure is that weird intermittent MetaPhysicL error, unrelated.

I'm glad they got my suspicions up, though, because this code is wrong in its new context! Consider the Edge3 from point(0)==(0,0) to point(1)==(1,0) through point(2)==(0.5,0.25): normalizing (and possibly sign flipping) point(1)-point(0) would have been correct (e.g. returning (1,0) at s==1) for an Edge2 without point(2), but on this Edge3 it will still return (1,0) on side 1, even though with that curvature the new normal is (1/sqrt(2),-1/sqrt(2)) there.

@GiudGiud
Copy link
Contributor Author

yeah I m still in finite volume land here. We have one test using Edge3, but mind you it s a straight one.

Ok I ll find something that works better, at least for a lagrange mapping

@moosebuild
Copy link

Job Min gcc on 9f31363 : invalidated by @GiudGiud

fluke?

@GiudGiud
Copy link
Contributor Author

This should be better now! Thanks for catching that.

@GiudGiud
Copy link
Contributor Author

forgot to register the test..

@GiudGiud
Copy link
Contributor Author

should be good now.

@GiudGiud
Copy link
Contributor Author

I did this geometrically with the mid point.
I m going to check this using the FE now

@GiudGiud
Copy link
Contributor Author

ok no luck. I m seeing that the computation with FE disagrees with this simple approximation.
I should have expected it though, the FE quadratic functions dont have 0 derivatives at the edge nodes

@GiudGiud
Copy link
Contributor Author

ok this works!

Copy link
Member

@jwpeterson jwpeterson left a comment

Choose a reason for hiding this comment

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

Just a couple minor comments but if @roystgnr wants to merge as-is that's fine.

"n/a # processor id specification file\n"
"n/a # p-level specification file\n"
"1 # n_elem at level 0, [ type (n0 ... nN-1) ]\n"
"27 0\n"
Copy link
Member

Choose a reason for hiding this comment

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

(General comment which I can address separately from this PR.)

So 27 is the ElemType enumeration value here. @roystgnr maybe we should update the xda comment that we typically write to make it clear that the first number is an ElemType rather than the vague "type" and the node ids aren't optional? At least I think that's what we were trying to imply with the parentheses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw I generated this manually. I could not figure out the script (generated an empty file)

@moosebuild
Copy link

Job Min gcc on 8a1623c : invalidated by @jwpeterson

Invalidate now that the PETSc on Min gcc has been updated

@moosebuild
Copy link

Job Test mac on 8a1623c : invalidated by @jwpeterson

- delete extra line deletion
- rename reference nodeelem to nodeelem instead of node
@roystgnr roystgnr merged commit bb3a8ff into libMesh:devel Nov 3, 2025
21 checks passed
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.

4 participants