Repository navigation
Unify the GMSH readers, repair the triangle path, and keep test artifacts out of the repo - #122
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
==========================================
+ Coverage 55.93% 60.64% +4.70%
==========================================
Files 15 16 +1
Lines 3184 3087 -97
Branches 418 406 -12
==========================================
+ Hits 1781 1872 +91
+ Misses 1246 1046 -200
- Partials 157 169 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… the handle on every path
…rejection of the wrong cell type
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
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.
Tail of #121, which was merged before the last three commits reached the
remote, plus the GMSH work that followed from reviewing them.
The GMSH triangle reader was unreachable and broken
_tria_io.read_gmshhad no caller anywhere and returned a raw 5-tuple ratherthan a mesh. Its binary branch could never have run: the file is opened in text
mode, so
struct.unpack("i", f.read(4))fails on astr, well before thenp.fromstringthat #121 replaced. That branch is removed rather thanrepaired, roughly 70 lines, and now raises
[binary format not implemented],the message
_tet_ioalready used. Real binary support needs the file reopenedin binary mode with the header lines decoded manually, and there is no
.mshfixture to validate it against.
Six
assertstatements did the format validation. Asserts vanish underpython -O, so a truncated file silently produced a wrong mesh; they are nowValueErrorin the same style_tet_ioalready used. The file handle alsoleaked on every error path, so the body moved into a context manager.
One parser for both mesh types
The two readers had drifted into different capabilities for the same format,
and the weaker one was the one people actually use. Parsing now lives once in
lapy/_gmsh_io.py, withTriaMesh.read_gmshandTetMesh.read_gmshas thinwrappers over it.
_tria_io.pyloses 176 lines,_tet_io.pyloses 83.TetMesh.read_gmshgains two fixes from the merge: a$PhysicalNamesblockused to make it fail outright, and its uniform-width assumption meant any
surface triangles gmsh emitted alongside the tetrahedra killed the read. Both
work now, and one file holding both types serves either reader. Node dtype
stays
float32, matching every other reader in the package.Fail early when the file cannot supply what you asked for
Reading a large volume mesh only to discover it has no triangles is wasteful,
so
read_gmsh(filename, want=...)rejects such a file as early as it can:known, since skipping costs 14 ms where parsing costs 464 ms on a 200k-node
file, and the offset is kept so they are read only if the file is usable;
conversion, and only runs when the first record is not already the wanted
type, so an ordinary triangle mesh never pays for it.
On a 62 MB mesh with 1.2M tetrahedra and no triangles: a full read as
TetMeshtakes 2188 ms, rejection as
TriaMeshtakes 493 ms.Element parsing is vectorized
The block is read in one
isliceand converted in onenp.array(...split()).Records share a width only when the block holds one element type with one tag
count, which reshapes in a single step; mixed blocks fall back to walking the
records. On 1M elements: 0.98 s to 0.52 s uniform, 1.00 s to 0.81 s mixed,
identical output in both cases.
Sections we do not use,
$PhysicalNames,$Periodic,$NodeData, are nowskipped rather than rejected, so real gmsh files load that previously did not.
Test artifacts out of the repo
test_visualization_meshes.pywrotedata/cubeTria.evanddata/cubeTetra.evon every run, with values that differ each time because
fem.eigs(k=3)takesno
v0orrng. Nothing read them back and the tests only assert the fileexists. They now write to
tmp_path, the two files are untracked, and*.evis ignored alongside
.agentbridge/,.agent-work/andscratch/..venvwas widened to
.venv*so side-by-side interpreter envs stay untracked.Verification
Ruff clean. 134 passed, 1 skipped on Python 3.13.9 with numpy 2.5.3 and on
3.11.9 with numpy 2.4.0rc1. The skip is
test_cholmod_matches_lu, which needsscikit-sparse and is waiting on #119.
Five new tests in
lapy/utils/tests/test_gmsh_io.pycover the round trip, onefile serving both readers, rejection of the wrong cell type, skipped sections,
and binary rejection. They write their own
.mshintotmp_path, so nofixture is added.
Behaviour changes
TetMesh.read_gmshaccepts files it previously rejected, as described above.TriaMesh.read_gmshis new. Both now require a.mshextension, which wasalready the tet reader's rule.