Repository navigation
Leo/merge release - #5443
Merged
Merged
Leo/merge release#5443
Conversation
* Add xfail tests for known mesh plotting failures triplot rejects extruded meshes outright, silently mangles the quadrilateral facets of hexahedral meshes into triangles, and pops entries out of the caller's boundary_kw. FunctionPlotter assumes the vertical node offset of an extruded function space is 1, so it samples the wrong data for anything but CG1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Draw mesh facets from the reference cell topology in triplot triplot assumed that a facet has as many vertices as the mesh has topological dimensions, which is only true for simplices. The quadrilateral facets of a hexahedron were reshaped into triangles spanning several facets at once, so the boundary of a hexahedral mesh was drawn as a mess of overlapping slivers. Take the nodes of each facet from the topology of the reference cell instead, which gives the right answer for any cell. This also replaces the Python loop over facets with an indexing operation and hoists it out of the loop over markers, where it was being recomputed for every marker. While here, copy boundary_kw before popping the colours out of it, so that a dict of keyword arguments can be reused across calls. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Respect the vertical node offset when sampling on extruded meshes FunctionPlotter walked up a column of an extruded mesh by adding the layer number to the node numbers of the bottom cell. That is only right when consecutive layers are one node apart, which holds for CG1 but not for CG2 (offset 2), DQ1 (offset 4) or the coordinates of a periodic extrusion (offset 2). Every field that tricontourf, tricontour, tripcolor and trisurf drew on such a space was sampled from the wrong degrees of freedom. Use the offset that the cell node map already carries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Support extruded meshes in triplot An extruded mesh has two kinds of facet. The vertical facets are the extrusions of the facets of the base mesh and carry its markers, so they are drawn from the extruded facet node maps. The horizontal facets are not facets of the base mesh at all; they bound the bottom and top of each column of cells, and are drawn from the cell node map of the lowest and highest layer under the markers "bottom" and "top", matching the subdomain ids of the ds_b and ds_t measures. A periodic extrusion identifies the bottom with the top, so neither is a boundary and neither is drawn. The test that the coordinates are piecewise linear now compares the dimension of the coordinate space against the number of vertices of the cell. Comparing the degree against 1 fails on a tensor product element, whose degree is a tuple. Variable layer extrusion is still unsupported, but now says so instead of failing obscurely. It is deprecated for removal in 2026.10.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Look up the boundary colormap without a deprecated call triplot reached for matplotlib.cm.get_cmap first and only fell back to matplotlib.colormaps if it was missing, so every call raised a MatplotlibDeprecationWarning. The fallback is the newer of the two: matplotlib.colormaps has been available since 3.5, well before matplotlib.cm.get_cmap was deprecated in 3.7, so nothing needs the deprecated call and it can go. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Inline the single-use plotting helpers _entity_dimensions had one caller and _cycle had one caller, so neither earned a name of its own. Fold them into the call site and into _entity_node_list respectively, keeping what they documented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make plotting circle manifold mesh work * Stop closing the polygons that bound each cell in triplot Repeating the first vertex of a cell was necessary when the interior was drawn with a LineCollection, which leaves its polylines open: without it every triangle came out missing an edge. The interior became a PolyCollection ten days later in 808ba85, and both it and Poly3DCollection close their polygons themselves, so the repeated vertex has been dead since. Dropping it leaves the 2D output pixel for pixel identical. In 3D it changes the depth sorting, which averages the z coordinates of a polygon's vertices: the repeated vertex weighted that average towards vertex 0, so the polygons are now sorted on their true centroid instead. The renderings are indistinguishable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Draw the boundary of a 1D mesh with markers The facets of a 1D mesh are points, which no line collection can show, so an interval embedded into 2D or 3D lost its two endpoints. Draw them with a PathCollection of markers instead, promoted to a Path3DCollection when the mesh lives in space. The markers are sized in points like those of a scatter plot, so only their offsets are in data coordinates and they need the axes to build. Choosing the collection from the geometric and topological dimensions cannot express that, since the types were looked up before the axes existed. Choose it from the shape of the vertex array instead: entities with one vertex are markers, those with two are segments, and those with more are polygons, with the trailing axis giving the dimension of the space they are drawn in. Two entities that agree on both are always drawn the same way, so the shape is all the dispatch needs, and it happens at the point of use where the axes are in hand. The rendering of every mesh that already plotted is unchanged, pixel for pixel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove triplot warnings, can't handle interior markers Every call to triplot was producing warnings about empty subsets because of how the querying for marked facets worked. This patch changes triplot to only color marked exterior facets, which gets rid of the warning and simplifies the code. But it won't plot any marked interior facets. Fixing that requires some changes to `mesh.py`. * Remove silly test * Switch to numpydoc style docstrings * Remove error on variable-layer meshes Co-authored-by: Connor Ward <c.ward20@imperial.ac.uk> * Shorten docstring * Lint --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Connor Ward <c.ward20@imperial.ac.uk>
Allows us to drop the PIP_CONSTRAINT hack.
* RieszMap: allow passing options_prefix for solver
* Pin the ADS cycle in the fieldsplit test
test_hypre_ads_fieldsplit left the ADS cycle to hypre, and asserted that the
resulting condition number stays under 2. The cycle hypre picks varies
between builds, and the one selected here gives 1.775 -- so little margin that
rounding decides the outcome. Refining the same problem walks straight
through the bound:
n=4 1.7583 n=7 2.0681
n=5 1.7855 n=8 2.0859
n=6 1.7750 n=9 2.0633
Cycle type 1 holds between 1.14 and 1.26 across that range, in 5 to 6
iterations rather than 8 to 9, so pin it and let the test measure ADS rather
than the default it happened to be built against.
The comment was wrong in both halves. The condition number was 1.775, not
1.3, and without a preconditioner it is 4.8e3, not 66. Quote what the
configuration above actually gives.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NoTXWyj2fVdJTHnMDFB4k
* Assert on the condition number in the hypre field tests
Both homogeneous-field tests checked only that the answer came out right,
which a bad preconditioner still manages given enough iterations. What they
are there to demonstrate is that ADS and AMS conditions the operator, so
measure that and assert on it.
They also left the cycle to hypre, whose default varies between builds. Pin
it to 1 so the tests measure the preconditioner rather than the default they
happen to be built against, and tighten the iteration cap from 30 to 20:
cycle 1 hypre default unpreconditioned
ADS simplex 1.6709 / 13 2.9143 / 21 1.2e4 / 955
ADS hexahedron 1.1084 / 9 1.4779 / 14 4.1e3 / 257
AMS simplex 1.2572 / 12 1.1236 / 10 1.2e2 / 97
AMS hexahedron 1.0641 / 8 1.0095 / 6 7.1 / 7
The cap sits above the 13 iterations the worst case needs and below the 21
the unpinned default takes, so it also catches the pin silently failing --
the option prefix is easy to get wrong, and a wrong one is invisible.
Reaching the KSP means building the solver rather than calling solve(), which
is why the bodies change shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NoTXWyj2fVdJTHnMDFB4k
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
bibtexparser 2.0.0 drops the bibtexparser.bwriter and bibtexparser.bparser
modules, so the docs build fails at import with
ModuleNotFoundError: No module named 'bibtexparser.bwriter'
taking down buildhtml, validate-bibtex and linkcheck together.
Use the v2 entry points instead, and require bibtexparser>=2. Two v1
behaviours have to be asked for explicitly under v2:
* v1's writer sorted entry fields alphabetically, so keep
SortFieldsAlphabeticallyMiddleware in the unparse stack;
* v1's parser lowercased field keys, which the "url"/"doi"/"archiveprefix"
lookups rely on, so keep NormalizeFieldKeys in the parse stack.
Entry.get() now returns a Field rather than the field value, so the
archiveprefix test is written against Entry.__getitem__ instead; left as
it was it would compare a Field to a string, silently never synthesising
an arXiv URL.
v2 separates blocks with two blank lines instead of one, so set
block_separator to match the committed files. The only remaining
formatting change is the @comment header, which v2 writes without the
newlines just inside the braces; the three validated .bib files are
reformatted accordingly.
Claude-Session: https://claude.ai/code/session_014Tf8jeKeRnvJyoJ36BzmZ9
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
I think that the docbuild fail could go away with #5440 |
Contributor
Nope. It will go away once this is merged. We have to rebuild the containers. |
connorjward
requested changes
Sep 10, 2026
Co-authored-by: Connor Ward <c.ward20@imperial.ac.uk>
Contributor
|
I merged a few PRs into FIAT this morning that required #5289. The FIAT-related failures should go away if you merge latest firedrake main. |
Contributor
|
I think we can merge this as is. There's no point on waiting for CI again, if we know that the docbuild will still fail until containers are rebuilt. |
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.
No description provided.