Skip to content

Commit ca2958b

Browse files
garth-wellsclaude
andcommitted
Address review: infinity norm, drop dead tolerance term, trim comments
- test_matrix_assembly_rectangular compares element-wise now, with PETSc.NormType.INFINITY on both the difference and the norm the tolerance scales with, and SAME_NONZERO_PATTERN in the axpy: the nest submatrix and the monolithic matrix are assembled from the same form, so a differing pattern would itself be a failure worth seeing. - The 'max(1.0e-10, 1.0e6 * eps**2)' in the interpolation tests is gone. The reviewer is right that it does no work: the term wins only in single precision, where it gives 1.4e-8, but the worst residual measured there is 9.75e-12, already inside the plain 1e-10. The original constants are restored unchanged -- the parametrisation over the scalar type is what those tests gained, not a new tolerance. 174 pass. - The vtkhdf comment is replaced by a reference to issue #4415, which records the degree-3 round-trip discrepancy properly. - Comments trimmed throughout, per AGENTS.md: an explanation that runs to five lines beside a one-line assertion is noise, and the reasoning belongs in the pull request or an issue. mypy still passes on all three targets, verified in a container matching the CI leg. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d895810 commit ca2958b

21 files changed

Lines changed: 1464 additions & 102 deletions

.github/workflows/ci.yml

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -214,16 +214,10 @@ jobs:
214214
spack env activate ci
215215
python -c "import dolfinx; assert dolfinx.has_petsc is ${{ matrix.petsc && 'True' || 'False' }}, dolfinx.has_petsc"
216216
217-
# mypy runs in ccpp.yml only in a build without PETSc, where it
218-
# excludes 'fem/petsc.py' and 'io/utils.py' because they need
219-
# petsc4py. Those modules are therefore type-checked nowhere. This
220-
# leg has petsc4py, gmsh and pyvista installed, so it checks them.
221-
#
222-
# The two disabled codes are the accepted limitation recorded in
223-
# AGENTS.md: petsc4py ships no stubs, so every signature touching it
224-
# degrades to 'Any'.
225-
#
226-
# One leg only: the result does not vary with the scalar type.
217+
# ccpp.yml runs mypy only in a build without PETSc, which excludes
218+
# the modules needing petsc4py. This leg has petsc4py, gmsh and
219+
# pyvista, so it checks them, on the project config alone. One leg
220+
# only: the result does not vary with the scalar type.
227221
- name: mypy checks (with petsc4py)
228222
if: ${{ matrix.petsc && matrix.mpi == 'openmpi' && matrix.scalar == 'float64' && matrix.index == 'int32' }}
229223
shell: spack-bash {0}

.github/workflows/spack-config/gh-actions-env-test.yml

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,13 @@ spack:
1313
all:
1414
require:
1515
- target=x86_64_v3
16-
# Use the reference BLAS/LAPACK rather than OpenBLAS.
17-
#
1816
# OpenBLAS 0.3.34 built by the container's gcc 15.2 faults in its Zen
19-
# kernels: on an AMD EPYC 9V74 runner the dispatcher selects
20-
# 'Core: Zen' and the first large dgemm of high-degree hexahedral
21-
# H(curl) element creation jumps to address zero. The same binary
22-
# selects 'Core: Haswell' on an Intel host and is fine, and valgrind
23-
# reports no memory error, so the fault is a miscompilation of those
24-
# kernels rather than a bug in the caller. netlib-lapack is the
25-
# implementation the crash was bisected against: LD_PRELOADing it on
26-
# the affected runner builds the element that OpenBLAS dies on.
27-
#
28-
# It is also the only provider that survives this toolchain. Building
29-
# OpenBLAS with gcc 13 is not an option -- the concretizer resolves
30-
# one compiler for most of the DAG, and mixing gcc 13 with cached
31-
# gcc 15 C++ libraries fails to link ('undefined reference to
32-
# __cxa_call_terminate' from llvm) -- and BLIS/libflame does not
33-
# build either, as libflame 5.2.0's f2c output is rejected by gcc
34-
# 15's C23 default ('too many arguments to function').
35-
#
36-
# The cost is speed: this is unblocked reference code with no tuned
37-
# kernels. Revisit when OpenBLAS is fixed for gcc 15.
17+
# kernels: high-degree hexahedral H(curl) element creation jumps to
18+
# address zero on an AMD EPYC runner, while the same binary picks
19+
# 'Core: Haswell' on Intel and is fine. netlib-lapack is the
20+
# implementation the crash was bisected against. Revisit when
21+
# OpenBLAS is fixed for gcc 15; see PR #4414 for the alternatives
22+
# that do not build under this toolchain.
3823
blas:
3924
require:
4025
- netlib-lapack

0 commit comments

Comments
 (0)