Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #104 by standardizing the dtype handling in the Solver class. Previously, the anisotropic FEM method was forcing matrices to float32 while the isotropic method used the mesh's dtype (typically float32). This inconsistency caused precision issues in downstream computations.
Changes:
- Added a
dtypeparameter to theSolverclass and all FEM computation methods, defaulting tonp.float64for improved numerical stability - Updated all matrix construction code to respect the specified dtype through proper conversion using
astype(dtype, copy=False) - Updated test expectations for
test_normalize_ev_geometryto reflect the improved precision from float64
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lapy/solver.py | Added dtype parameter throughout the Solver class hierarchy; updated init, _fem_tria, _fem_tria_aniso, fem_tria_mass, _fem_tetra, and _fem_voxels methods to accept and use dtype parameter; added dtype conversion logic to all matrix construction code; improved docstring grammar and clarity |
| lapy/utils/tests/expected_outcomes.json | Updated expected values for test_normalize_ev_geometry test to reflect improved precision from float64 default |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
- Coverage 44.42% 44.41% -0.01%
==========================================
Files 15 15
Lines 2753 2758 +5
Branches 348 348
==========================================
+ Hits 1223 1225 +2
- Misses 1408 1411 +3
Partials 122 122
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Solver was sometimes taking dtype from the mesh (usually float32) and sometimes forcing it to float32 in tria_aniso. This was leading to issues (see #104 ).
We now always allow a dtype argument in solver and default to float64, which will require more memory but will give higher precision. This should be similar to other FEM packages and scipy etc.
Users who run into memory issues should pass dtype=float32 explicitly.
We may break backward compatibility here, by switching to float64 which will change values (should be more accurate now).
Speed should not be affected much.