search

FluidityProject / fluidity

Fluidity
http://fluidity-project.org
Other
365 stars 115 forks source link

VTK Quads and Hexas output bug #303

Closed gnikit closed 3 years ago

gnikit commented 3 years ago

Fixes #302 VTK output

Quads and hexahedrals are now drawn correctly when calling VTK.

A unit test has been added testing our 4 main elements triangles, quads, tets and hexas. The test checks that the DG DOFs get correctly reordered from/to Fluidity to/from VTK.

The reordering routines vtk->fluidity and vice versa need to be public 1) for testing 2) to allow greater flexibility when using vtkfortran. Sometimes, it is easier to generate the VTK DG nodes, convert them to Fluidity ordering for the calculations and when the mesh is to be output reorder them back to VTK node order.

I also deleted the AddOneWedge function it is redundant and the AddOneAnything covers it, since wedges are two VTK_TRIANGLES stacked on top of each other. However, I don' think the underlying infrastructure supports (quadrature.F90) wedges.

gnikit commented 3 years ago

The test_inverse unittest is misbehaving again, we might have to drive up the tolerance on that one.

gnikit commented 3 years ago

I think I might also have to delete/change the Python function under vtutools.py:ToVtkNodeOrder and FromVtkNodeOrder, since quad ordering is handled internally in Fortran

gnikit commented 3 years ago

just remove these routines in vtutools.py (and the places that call it).

As far as I could tell there are no places that make use of the vtutools.py code for quads, so all should be good, but I will have another look to be safe.

This might affect checkpointing (i.e. the only time we read in vtu files), but I'm not sure that even works with quads/hexes - you should probably try to see if it does, and if so if it correctly reads things in.

I will have a look, but from what I remembered all looked good on my end. Will push shortly.

gnikit commented 3 years ago

Apologies this took so long. I think this should be good to go now @stephankramer. I also rechecked the chekpoint output and restart using the rename_checkpoint test and the gmsh -2 -algo delquad src/mesh2.geo to get a mixed triangular/quad mesh, then restarted a checkpoint and it all seems good, which makes sense since we simply moved to one location all the node mappings from Fluidity to VTK elements and nothing more.

The Python diagnostic node ordering methods have been deleted since they do nothing other than returning a copy of the nodes array.

gnikit commented 3 years ago

Should this be merged or do we have to wait for the particle tests to be fixed in master?

tmbgreaves commented 3 years ago

Rerunning tests via Actions - @stephankramer assuming no critical bugs come out of that are you happy for this to be merged? Keen to do so before master->main branch renaming on 26th if possible to avoid the mess of retargetting the PR.

tmbgreaves commented 3 years ago

OK, we do have an interesting bug, which is that PRs from outside the Fluidity repo don't have the requisite auth details to do the Actions build and push/pull from docker. @gnikit as a short term fix I can send you a token to add to your fork to fix that, though we need a better solution / policy in the longer term.