Elem::orient(), MeshTools::Modification::orient_elements()

[roystgnr]

This ought to close #3424

@GiudGiud - you've got a MOOSE MeshGenerator test case you can try this out on, make sure it handles bad extrusion vectors properly, before we merge here?

[moosebuild]

Job Coverage on 3c4e048 wanted to post the following:

Coverage

	7bbcc3	#3435 3c4e04
	Total	Total	+/-	New
Rate	59.63%	59.74%	+0.12%	83.38%
Hits	48033	48340	+307	296
Misses	32525	32574	+49	59

Diff coverage report

Full coverage report

Warnings

• New new line coverage rate 83.38% is less than the suggested 90.0%

This comment will be updated on new commits.

[GiudGiud]

Sure. I have the symmetry and the extrusion to try. I ll report back

[GiudGiud]

so if you leave this without a message we'll need to catch it upstream in moose

[GiudGiud]

The symmetry generator works well with the set and gives the right solution on a diffusion problem with QUADs.

The (advanced)extruder returns different sidelists with this flipping compared to the manual flipping logic we had.
Flipping the elements seems to have flipped the sides. We still get the bottom/left sides right, what is different now is the existing 1D sides that were turned into 2D sides.

Figures show the case. The extrusion direction is in the depth of the figure. The left and right (brown, green) are the sidesets that got broken. left is reference.

Left and right exist as 1D sidesets, and becomes 2D sidesets in the extruded mesh, probably because the nodes are in the relevant nodeset and the sideset is created off of this nodeset

I think it s just a convention in the nodes selected for the swaps at this point?
The swaps selected in AdvancedExtruder 'preserved' these sidesets on a Z-axis extrusion.

[roystgnr]

Left and right exist as 1D sidesets, and becomes 2D sidesets in the extruded mesh, probably because the nodes are in the relevant nodeset and the sideset is created off of this nodeset

Ah! I swapped the BoundaryInfo side and edge entries but not node entries! Thanks for catching that.

[roystgnr]

Huh. Actually, I'm still not understanding the bug. I was already swapping both side and edge entries ... and for node entries there's nothing to swap - we associate those with the Node *, which doesn't change when the element node ordering is swapped.

[GiudGiud]

the nodeset seems wrong, and the sideset for that nodeset is wrong as well
I m plotting both left/right sidesets but only one (right) nodeset.

[roystgnr]

Can you point me to the branch + input file to reproduce this?

[GiudGiud]

https://github.com/RocksSavage/moose/tree/create-SymmetryTransformerGenerator
the input is
test/tests/meshgenerators/advanced_extruder_generator/elem_flip.i

I complexified it locally but it s not needed to show the problem

[GiudGiud]

is this still correct with an element flip?
It s copying the old boundary id to the new mesh

            // For 2D->3D extrusion, we give the boundary IDs
            // for side s on the old element to side s+1 on the
            // new element.  This is just a happy coincidence as
            // far as I can tell...
            if (_boundary_swap_pairs.empty())
              boundary_info.add_side(new_elem, cast_int<unsigned short>(s + 1), ids_to_copy);

[roystgnr]

Okay, that is the problem. That add_side() would be correct if it were done before the flip, but in the branch it's being done after the flip. So we flip the element, the sides identified by each side number change accordingly, but then we add boundary info to the side number that would have been appropriate for the original inverted element, not the side number that's appropriate for the flipped element.

Why do volume() tests and an individual flip() call on each element? We ought to be able to just orient_elements() once at the end before returning. Shorter code, wouldn't have this bug, the test for inversion in orient() (and thereby in orient_elements() is cheaper than volume() ... and possibly more future-proofed, if we ever make volume() return an absolute value on cells the way it currently does on faces.

[GiudGiud]

doing it that other way works.

Thanks for looking into it.