Single-precision (fp32) build support#5033
Conversation
|
Thanks for this.
What about single+complex? Isn't that a valid configuration?
This isn't necessary. That's all going to be taken care of when I release the next major version in the next 24 hours.
Can you get this fixed upstream? Clearly Claude already knows what to do. |
0dc7472 to
dd517a9
Compare
Added ScalarType.SINGLE_COMPLEX (--arch single-complex) to firedrake-configure for all four platform targets: single_mode (from PETSC_PRECISION) and complex_mode (from PETSC_SCALAR) are detected independently, and ScalarType already resolves to numpy.complex64 for a single+complex build.
Removed from the PR description — thanks.
Opened a PR here: https://gitlab.com/petsc/petsc/-/merge_requests/9272 |
connorjward
left a comment
There was a problem hiding this comment.
I've just looked at the CI/install related side of this (at a glance everything else seems pretty good).
We just have to be careful about adding new test builds to Firedrake. I will try and figure out a solution soon.
I have a solution for this in #5117 (merged). Once we update |
…spatialindex float64 coercion
- mesh.py: clarify why PETSc.RealType is correct for plex_from_cell_list - mg/kernels.py: fix to_reference_coords_kernel signature (PetscScalar *X -> %(RealType)s *X) and add RealType to the template substitution dict - evaluate.h: comment explaining why int/double is intentional for evaluate() - test_stokes_mini.py: replace fp32 solver workaround with @pytest.mark.skipsingle; fieldsplit path needs a dedicated fp32 test - conftest.py: remove trailing blank line
…nd skip VertexOnlyMesh tests in fp32
cf73186 to
3cd08b8
Compare
…p32 skip refinements
Co-authored-by: Connor Ward <c.ward20@imperial.ac.uk>
connorjward
left a comment
There was a problem hiding this comment.
In general I think this is good - thank you!
We have to think about testing this. I don't mind having partial test coverage and skipping a lot of things provided that it is clearly recorded that that is only happening because we haven't gotten around to fixing things. For example an issue discussing this should definitely be opened. It might even be a "good first issue".
|
|
||
| @pytest.mark.skipcomplex | ||
| @pytest.mark.parallel([1, 2]) | ||
| @pytest.mark.skipsingle # VertexOnlyMesh point location has fp32 precision issues |
There was a problem hiding this comment.
Is this still true? You look to have done some work on point location
| assert errornorm(w, wcheck) < tol | ||
|
|
||
|
|
||
| @pytest.mark.skipsingle # asserts ksp.its == 1 (exact algebraic inverse); not achievable in fp32 |
There was a problem hiding this comment.
@JHopeCollins is there a way we can run this test in single precision? What is the right way to loosen things?
| "unitsquare_from_high_order", | ||
| "unitsquare_to_high_order", | ||
| "extrudedcube", | ||
| "extrudedcube", # petsc/petsc!9272 fixes fp32 DMSwarm.getField; re-add skipsingle if that MR is not in the shipped PETSc |
There was a problem hiding this comment.
Can this comment go? Has the PETSc MR been merged?
| return errornorm(uexact, u, degree_rise=0), errornorm(pexact, p, degree_rise=0) | ||
|
|
||
|
|
||
| @pytest.mark.skipsingle # Schur complement fieldsplit does not converge in fp32; tracked in https://github.com/firedrakeproject/firedrake/pull/5033 |
There was a problem hiding this comment.
What does it mean "tracked in 5033", that's this PR
| from firedrake import * | ||
| import pytest | ||
|
|
||
| pytestmark = pytest.mark.skipsingle |
There was a problem hiding this comment.
I think it would be helpful wherever we have this to give a reason for the skip. I don't want confusion between "this is genuinely impossible" (e.g. skipnogpu) and "TODO: this just hasn't been worked out".
| if isinstance(temp, gem.Constant): | ||
| data.append(lp.TemporaryVariable(name, shape=temp.shape, dtype=dtype, initializer=temp.array, address_space=lp.AddressSpace.LOCAL, read_only=True)) | ||
| # loopy raises if initializer.dtype != declared dtype (e.g. float64 GEM constant in fp32 build). | ||
| initializer = temp.array.astype(dtype) if temp.array.dtype != dtype else temp.array |
There was a problem hiding this comment.
I feel like we should really track down where we're inserting float64s and make them the right scalar/real type instead
| in single- and double-precision builds. The threshold is loose enough that | ||
| discretization error dominates round-off for both fp32 and fp64.""" | ||
| err = helmholtz(4)[0] # 16x16 mesh, degree 2 — expected L2 error ~2e-4 | ||
| assert float(err) < 1e-2 |
There was a problem hiding this comment.
Why this new test? What is it testing that the other tests aren't?
| maintainer="the Firedrake team", | ||
| contact="on Slack", |
There was a problem hiding this comment.
I'm not thrilled about taking on the maintenance burden of these additional configurations given that we have never tested them. We're happy to maintain previous Ubuntus because we used to maintain them. The idea behind COMMUNITY_ARCHS is that we could make these other people's responsibilities - like yours or Corintis'.
| return cache[tolerance] | ||
| except KeyError: | ||
| IntTypeC = as_cstr(IntType) | ||
| RealTypeC = RealType_c |
There was a problem hiding this comment.
This seems pretty redundant
| self.tolerance = tolerance | ||
| xs = np.asarray(xs, dtype=utils.ScalarType) | ||
| # Physical coordinates: always float64 (libspatialindex requires double). | ||
| xs = np.asarray(xs, dtype=np.float64) |
There was a problem hiding this comment.
This is going to confuse people in future. Can you put comments in some of the places where we end up seeing double when you would otherwise expect to see RealType. E.g. above
{IntTypeC} locator(struct Function *f, double *x, {RealTypeC} *X, {RealTypeC} *ref_cell_dists_l1, {IntTypeC} *cells, {IntTypeC} npoints, size_t ncells_ignore, {IntTypeC}* cells_ignore)
fixes #3040
Description
Adds single-precision (fp32) build support. Firedrake can now run on a PETSc installation compiled with
--with-precision=single. The approach mirrors complex mode: precision is detected at import time from PETSc's build variables and flows through from there.Prerequisite
Requires https://gitlab.com/petsc/petsc/-/merge_requests/9272 (
petsc4py: handle PETSC_DOUBLE in DMSwarm.getField). Without it,DMSwarm.getField()raisesAssertionErroron fp32 builds becausePETSC_DOUBLEis not mapped to a numpy dtype in the single-precision case wherePETSC_REAL != PETSC_DOUBLE.Changes
scripts/firedrake-configure--arch single/ScalarType.SINGLE; passes--with-precision=singleto PETSc configure; excludesfftwandsuitesparse(no fp32 support in those libraries)tsfc/parameters.py,tsfc/loopy.py,tsfc/kernel_interface/common.py,tsfc/ufl_utils.pyscalar_type/scalar_type_cderived from PETSc precision at import time; constant initializers cast to the kernel scalar dtypeCore (
evaluate.h,locate.c,pointquery_utils.py,pointeval_utils.py,mg/kernels.py)double/intwithPetscReal/PetscIntin generated C code1e-6in fp32 mode vs1e-12in fp64, to stay within single-precision rangefiredrake/mesh.py,firedrake/utility_meshes.pyPETSc.RealType; physical coordinate arrays for rtree andDMSwarmPIC_coorremainfloat64(required by the rtree C API and PETSc swarm internals)firedrake/function.pyfloat64regardless ofScalarType, for geometric robustness in cell locationfiredrake/assemble.py,firedrake/functionspaceimpl.py,pyop2/codegen/builder.pydtype=intwithdtype=IntTypeinnumpy.prodand array allocation callsfiredrake/utils.pysingle_modeboolean flag (mirrorscomplex_mode)Tests
@pytest.mark.skipsinglemarker for tests incompatible with fp32tests/firedrake/conftest.py: registers the marker and wires it uptest_locate_cell,test_interpolate_cross_mesh[extrudedcube],test_parallel_high_order_location).github/workflows/core.ymlsingleto the CI matrix alongsidedefaultandcomplexKnown limitations
test_parallel_high_order_locationis skipped in fp32: high-order cell location in a warped mesh requires double-precision accuracy that fp32 cannot provide attolerance=0.0001.