More code modernization - #96
Open
davschneller wants to merge 23 commits into
Open
davschneller wants to merge 23 commits into
davschneller wants to merge 23 commits into
Conversation
davschneller
commented
Sep 24, 2026
davschneller
commented
Contributor
- better testing
- fix more bugs with old formats
- VTKHDF/Xdmf views
- better mesh support
All sources except pumgen.cpp now form the static library pumgen-core, which pumgen and the tests link. The reader test compiled its own copies of the parser sources. The release flags were set to "-O2", which dropped -DNDEBUG, so the asserts were active in release builds; they are "-O2 -DNDEBUG" now. The sanitizers of Debug builds were added to CMAKE_LINKER_FLAGS_DEBUG, which is not a CMake variable; they only reached the linker because CMake passes the compile flags along. They are now set with add_compile_options and add_link_options for Debug builds, and SANITIZERS chooses them (address by default, as before; empty for none), so that CI can add undefined. install() was given two destinations, the second one from CMAKE_INSTALL_BINDIR without GNUInstallDirs; it now uses that variable only, with GNUInstallDirs included. CMake 3.13 is required for add_link_options. AI-generated. Model: Claude Opus 5.5
doctest (v2.5.3) comes in as a submodule, together with its MPI extension: a TEST_CASE runs on every rank, an MPI_TEST_CASE on a communicator of the ranks it asks for, and it is skipped when the test is started with fewer ranks. CTest runs the unit tests on 1 and on 3 ranks. The first test checks the three functions of the chunk distribution against each other, for fewer items than ranks as well. AI-generated. Model: Claude Opus 5.5
ParallelVertexFilter picks its splitters from the sorted local vertices, which a rank without vertices does not have: it read the first entry of an empty index vector. Such a rank now proposes splitters behind those of all other ranks, which leaves the choice of the others unchanged. The filter is used for SimModSuite and netCDF meshes. The new test filters on three ranks, one of them without vertices, and checks that duplicates get the same global id and distinct vertices distinct ones; before, the test aborted in the assert on the splitter index. AI-generated. Model: Claude Opus 5.5
The partitions of a netCDF mesh are split into blocks of ceil(partitions / ranks). A rank completely behind the last block kept the full block size instead of none, so with 5 partitions on 4 ranks the last rank read the partitions 6 and 7, which do not exist. getBlockRange computes the range of a rank, and its test covers that case. The ranks with partitions were split off MPI_COMM_WORLD instead of the communicator given to NetCDFMesh, and the resulting communicator was never freed. Partition keeps its arrays in vectors instead of owning raw arrays behind an implicit copy constructor. This path is not compiled in CI yet: the netCDF of Ubuntu is built without parallel support and has no netcdf_par.h. It was checked for syntax against a stand-in of that header. AI-generated. Model: Claude Opus 5.5
The groups were distributed as if the group sections listed exactly one entry per element. When they list fewer, readGroups walked past the last section and dereferenced its end iterator; a mesh without any group section crashed right away. Now only the listed entries are read, the other elements keep group 0, and a warning says how many there are, as for GMSH meshes. More entries than elements are reported as well; the last group listing an element wins, as before. The serial readGroups(int*) and readBoundaries(int*) of GambitReader were not used and are gone; the latter did not check the element and face it indexed with. The test derives a mesh without groups from the layered one when CMake configures the tests. AI-generated. Model: Claude Opus 5.5
The number of rows written per round is --chunksize divided by the size of a row; a chunk size smaller than a row made it zero and the number of rounds a division by zero. The test writes the layered mesh with --chunksize=1 on 3 ranks, one row per round. AI-generated. Model: Claude Opus 5.5
The XDMF file described the connectivity as four columns of eight byte integers whatever was written. For --order=2 the dataset has ten columns; VTK's XDMF reader crashed on such a file (segmentation fault). The higher-order nodes follow the vertices, so the topology now selects the first four columns with a hyperslab and shows the linear cells. The precisions come from the values actually written, rounded up to the sizes XDMF knows, so that --compactify-datatypes is described correctly (HDF5 widens the values on reading, which is why the old files could still be read). The name of the XDMF file had the suffix replaced twice: the second replacement of ".h5" hit the directory for an output such as dir.h5/mesh.puml.h5, and the XDMF file went to dir.xdmf/mesh.xdmf. The tests check the XDMF files of these three cases; VTK reads the first two with 320 tetrahedra. AI-generated. Model: Claude Opus 5.5
When the command line could not be parsed, and for --help, main returned without MPI_Finalize, which mpiexec reports as an abnormal end; --help now exits with 0. The mesh input is held in a unique_ptr instead of a raw pointer deleted at the end, and writeH5Data loses the two parameters it never used. AI-generated. Model: Claude Opus 5.5
The workflow runs on pull requests as well, not only on pushes. Four configurations build and test PUMGen: GCC in Release; GCC and Clang in Debug with AddressSanitizer and UndefinedBehaviorSanitizer, where undefined behaviour aborts the test; and GCC in Release with easi, which builds pumgen-sizefield and runs the tests of the velocity-aware sizing. Those tests had never run in CI. easi is built from a pinned commit, without ASAGI, Lua and ImpalaJIT. libnetcdf-dev is not installed anymore: NETCDF was never switched on, and the netCDF of Ubuntu lacks the parallel support the netCDF reader needs. All four configurations pass locally. AI-generated. Model: Claude Opus 5.5
The table of node counts per element type left out the three type numbers 76 to 78, which gmsh does not define, so every type from 76 on was given the node count of the type three numbers further. A hexahedron of order 3 (type 92) was read with 343 nodes instead of 64, and the parsers lost their place in the file. The table is now written from the element properties gmsh 4.15.2 reports, with 0 for the numbers it does not define; only the prisms 90 and 91, which gmsh defines but cannot report, are taken from GmshDefines.h. A unit test checks a sample of the table against gmsh. AI-generated. Model: Claude Opus 5.5
--compactify-datatypes chose the smallest number of whole bytes, so a dataset could end up with integers of 3, 5, 6 or 7 bytes. HDF5 converts them on reading, but XDMF declares precisions of 1, 2, 4 and 8 bytes only, VTK maps HDF5 integer types to its arrays by these sizes, and SeisSol tells the packed boundary formats apart by the width of a value. The width is now rounded up to the next of these sizes; the saving over the exact width is at most a byte per value. The reduced types are not committed to the file as named datatypes anymore: nothing reads them, and they only added objects next to the datasets. AI-generated. Model: Claude Opus 5.5
The node counts of the MSH element types were taken from gmsh 4.15.2, which cannot report the prisms of order 5 to 9 (types 106 to 110) and the incomplete prisms (111 to 117) any more than the prisms of order 3 and 4. They were left at 0, so a file holding such an element was refused as having an unknown type. They are now taken from GmshDefines.h as well. AI-generated. Model: Claude Opus 5.5
CellType names the four kinds of cells by their VTK cell types, which a VTKHDF file stores and PUML reads. Their shapes list the vertices in the order of gmsh and the faces in the numbering of PUML, oriented outwards, together with the edges; the tetrahedron is the one the readers use throughout. gmshElementType classifies the MSH element types: cells by kind and order and whether they hold all nodes of a Lagrange cell, faces as triangles or quadrilaterals of any order, whose first nodes are their vertices, and everything else as other. The tests check the shapes (Euler's formula, every edge in two faces, all faces pointing out of the reference cells of gmsh) and the node counts of the complete cells against the table of the parsers. AI-generated. Model: Claude Opus 5.5
writeH5Data copied the data of every rank into a buffer before writing it, in rounds of --chunksize, which is 1 GiB by default, so that the whole connectivity was held twice while it was written. Data which is already of the memory type is now written from where it is; only data of another type is still converted into a buffer round by round. The inspheres of all cells are also let go once their minimum is known, instead of being kept until the end. AI-generated. Model: Claude Opus 5.5
The mesh data keeps the kind of every cell, the vertices of all cells one after the other, and the boundary conditions of all faces, as many per cell as its kind has. The GMSH readers take hexahedra, wedges and pyramids besides tetrahedra, of any order: the vertices of the mesh are the vertices of its cells, numbered in the order of the nodes, and the other nodes of every cell of higher order are kept with it. The order of a cell is the one of its element type, so --order is gone. Element types without all nodes of a Lagrange cell are refused. The binary MSH 4.1 reader with all ranks reads linear cells of every kind; a file with cells of higher order is read by rank 0. The writer lays out a mesh of one kind as before, with a rectangular connect and an XDMF file for that kind. A mesh of several kinds is written as the unstructured grid of VTKHDF: connect holds the vertices of all cells, connect_offsets where every cell begins in it, and cell_type the VTK cell type of every cell. The group /VTKHDF links these datasets under the names VTK expects, so that VTK, ParaView and PUML read the file directly; XDMF cannot describe such a mesh, and none is written. Cells of higher order add geometry_ho, the nodes of every cell except its vertices in the order of gmsh, geometry_ho_offsets and order. Boundary conditions are packed (i32, i64) for cells of up to four faces only; for cells with more faces, one integer per face is the default, and --boundarytype=i32x4 always means one integer per face. The file names its kind of cells, a format version, the generator with its git version, the command, the source format and the time of creation. The insphere radius is three times the volume over the surface area, which for a tetrahedron is the radius of its insphere; the velocity check measures the edges of each kind. Ranks without vertices no longer decide on their own that the mesh identifies no vertices, which let them skip the collective write of /identify. The readers reserve the sizes they can know, and a single rank takes the mesh over instead of copying it. On a box of 617,044 tetrahedra in Release on one core, against the previous commit: ASCII MSH 4.1 on one rank 0.55 s and 70 MB instead of 0.56 s and 89 MB; binary on one rank 0.51 s and 85 MB instead of 0.51 s and 91 MB; binary on four ranks a median of 0.98 s instead of 0.96 s and at most 60 MB per rank instead of 63 MB. The written datasets are the same as before. The reference of the mesh of order 2 holds the vertices of the linear mesh and the geometry of higher order, written by fixtures/generate.py from the node tags in the file. AI-generated. Model: Claude Opus 5.5
fixtures/generate.py builds a conforming mesh of every kind of cell, cube by cube: a hexahedron, six pyramids around the centre of the next cube, two wedges in the third, and Kuhn's six tetrahedra in a cube next to the triangles of the wedges. The boundary conditions follow from the position of the faces. The references are computed from the cells the script builds, in the order of the files; gmsh writes the MSH 2.2 file by element type, so it has a reference of its own. The mesh is converted from ASCII MSH 4.1 on one to three ranks, from binary MSH 4.1 by all ranks and by rank 0, from MSH 2.2, and with compacted datatypes; a packed boundary format is refused for its hexahedron. With MIXED, a test also compares the offsets and kinds of the cells and the links of the VTKHDF view. AI-generated. Model: Claude Opus 5.5
On more ranks than a mesh has vertices, the ranks without vertices decided on their own that the mesh identifies no vertices and skipped the collective write of /identify, which the other ranks entered, so the conversion hung. fixtures/generate.py now makes a periodic cube of its eight corners, which is converted on nine ranks by the reader on rank 0 and by the one of all ranks. Before "Read and write cells of every kind and order", both conversions hang. AI-generated. Model: Claude Opus 5.5
Both MSH versions can name their physical groups in a section $PhysicalNames, which the parsers skipped. They now read it, and the names travel with the mesh to all ranks. The writer names the values of /group by the names of dimension 3, and those of /boundary by the names of dimension 2, as two attributes of the dataset: ids, the values as the dataset holds them, so for a boundary condition without 100, and names, in the same order. A file without names gets no attributes. The groups of the mesh of all kinds of cells are named now, one surface with a space in its name, and the references hold the attributes, which h5diff compares with the datasets. The reader test checks that the names are the same with a tiny buffer. AI-generated. Model: Claude Opus 5.5
fixtures/generate.py raises the mesh of all kinds of cells to order 2 with gmsh and keeps the hexahedron and the wedges of order 2, while the pyramids and tetrahedra are taken as linear cells, so that the file also holds nodes no cell uses. gmsh numbers the nodes anew, so the vertices are found by their coordinates. The reference is read from the file itself: the vertices are the nodes which are a vertex of a cell, in the order of their tags, and the other nodes of a cell of order 2 are its geometry of higher order. The mesh is converted from ASCII MSH 4.1 on one and three ranks, and from binary MSH 4.1, which rank 0 reads. AI-generated. Model: Claude Opus 5.5
The reader of all ranks read linear cells only, so a binary MSH 4.1 file with cells of higher order went to rank 0. The reader now keeps the vertices of every cell apart from its other nodes. The nodes which are a vertex of a cell are marked in a bit set of all nodes, which the ranks share and which numbers the vertices in the order of the nodes; the vertices go from the ranks which read their nodes to the ranks which own them. Every rank fetches the coordinates of the other nodes of its cells from the ranks which read them. Periodic classes are represented by their smallest vertex, and nodes which are no vertices drop out of them. A mesh of linear cells is read as before. Binary files are therefore always read by all ranks, and the check for cells of higher order before choosing the reader is gone. The meshes of order 2 and of several orders are converted by the reader of all ranks on up to four ranks. A periodic box of order 2 gives the same datasets with this reader on three and four ranks as with the reader on rank 0. AI-generated. Model: Claude Opus 5.5
A mesh of several kinds of cells is a VTKHDF file already. With --vtkhdf, a mesh of one kind becomes one as well: /VTKHDF links the datasets as for several kinds, while its connectivity is a virtual dataset which reads the rectangle of /connect one row after the other, so that the vertices are not stored twice. The offsets and kinds of the cells are written, 9 bytes per cell, fewer with compacted datatypes. Virtual datasets came with HDF5 1.10, so such a file is written in the format of 1.10 and needs HDF5 1.10 to be read; without the option, nothing changes. The XDMF file is written all the same. The tests compare the view, also with compacted datatypes, with a reference which fixtures/generate.py derives from the one of the mesh. VTK 9.7 reads both files: 320 cells, all of positive volume, which add up to the volume of the box. AI-generated. Model: Claude Opus 5.5
Given a checkout of PUML2 as PUML_DIR, the tests build a program which reads a converted mesh with PUML, through the reader of meshes of several kinds of cells, and checks what PUML makes of it against the file: the numbers of cells, vertices, faces and faces on the surface, which it counts in the file from the faces of the cells, and the Euler characteristic of a mesh of a ball. Every face on the surface has to have a boundary condition in the file; faces inside may have one too. The program reads the outputs of the tests of tetrahedra, of order 2, of all kinds of cells from both MSH versions, and of several kinds and orders, each on one and three ranks, and fails when the conditions of a cell are removed. A new CI job builds PUMGen with PUML2 from TUM-I5/PUML2 and runs these tests, with the conversions they need. The branch, PUML_REF, has to read the kinds of the cells from an array of their own, which the PUML2 patch "Read the kinds of a mixed mesh from an array of their own" brings. The job itself cannot run here; the tests ran against PUML2 with that patch. AI-generated. Model: Claude Opus 5.5
The program which reads a converted mesh with PUML counted the faces on the surface in the file and checked their boundary conditions in the file alone, as PUML2 could not give the faces of a cell of a mixed mesh. It now walks down from every cell PUML read: the cell has to be of the kind the file gives it, the face on each of its sides has to have the vertices the file gives that side, so that PUML and PUMGen number the sides alike, and a face PUML finds on the surface has to have a boundary condition at that side in the file. The numbers of the faces and of the faces on the surface are still compared with the file. The program fails when the conditions of a cell are removed from a file, and when two sides of a tetrahedron are swapped in its table. It needs the PUML2 patches up to "Take the vertices of a face from its first cell", which the branch of the CI job has to contain. AI-generated. Model: Claude Opus 5.5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.