You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Code-quality audit: fix three bugs, tighten types, deduplicate
Three real bugs, each a case of drifted duplication:
- spectral1d chebfun's x < -1 branch was a stale copy of evaluation's, with a
length-n sign vector where n+1 was needed, so left extrapolation through
interpolate() threw DimensionMismatch while right extrapolation worked.
- The spectral1d plot method splatted its keyword arguments positionally
(missing semicolon), so plot(geom, z; color="red") sent Pair objects to
matplotlib as positional arguments. Its spectral2d sibling was correct.
- A stale "export plot" survived in AlgebraicMultiGridBarrier.jl after the
plotting extension moved plot onto PyPlot.plot; it left plot in
names(MultiGridBarrier) as an UndefVarError trap without PyPlot.
Error handling. The broad catch in the feasibility probe and the line searches
is deliberate and stays: a barrier has no fixed protocol for signalling domain
escape (DomainError from log, non-finite values, InexactError, complex results,
division by zero). Both now rethrow InterruptException so Ctrl-C is not
swallowed and retried through a thousand step halvings, and the comments record
the rationale. Correctness-bearing @asserts, which may be compiled out at higher
optimization levels, become equally terse `cond || error(...)` one-liners.
throw(e) becomes rethrow() so backtraces keep their origin. Dead defensive
branches are removed: a tautological L_x <= L_max guard, two BlockMatrices
guards that would have silently truncated a scan (replaced by one upfront
DimensionMismatch check), and an unreachable CUDA early return. Duplicate
validations spanning two layers of one call chain now have a single owner.
Types. Barrier gains type parameters instead of three abstract ::Function
fields. barrier(Q)'s gradient and Hessian seeded their accumulators with a
wrong-typed sentinel (an Int, and a dead store) making them Union-typed; the
first term is hoisted instead. mgb_step's Newton closures captured a variable
that eta reassigns, boxing it and untyping every objective, gradient and
Hessian call in the innermost loop; they now capture a snapshot. MGBSOL,
MGBConvergenceFailure, the Zoo D-table containers and the progress-bar locals
are typed. The CUDA AssemblyPlan and CuDSSCacheEntry are parameterized on
concrete array types (CuArray{T,N} is a UnionAll under CUDA.jl 5), with a
function barrier where the plan leaves its cache. In the JuMP extension settag
becomes a closed union and geometry is typed; objexpr must stay Any because
MGBExpr and MGBModel are mutually recursive.
Defaults. newton's stopping_criterion and line_search sentinels become real
keyword defaults (T is in scope, as it is everywhere else in the package). The
Clenshaw-Curtis weights turned out to be a plain trigonometric sum rather than
FFT acrobatics, so _tf_nodes and _tf_weights are now T-generic via cospi
instead of computing in Float64 and converting; the Gmsh extension's _cheb is
kept bit-matched.
Waste and duplication. _tf_hierarchy computed a sparse triple product and threw
it away whenever amg_input was supplied, which both call sites always do.
convex_linear sampled A (a device-to-host copy on GPU) even when A_grid was
given. The Euclidian-power cobarrier gradient inlined twenty lines of scatter
logic that already existed as a helper, and that its Hessian sibling reused.
The three PiecewiseCobarrier structs were byte-clones of the barrier trio,
which the callables never distinguish. The Zoo problems shared forty-five lines
of state setup across two pairs of files, now _vector_state_setup and
_scalar_fg. fem2d_P2's continuous() reimplemented three helpers with a drifted
tolerance and mutated its input; it delegates now, verified bit-identical
against the old implementation. The AMG ladder scaffold, pasted in three FEM
families, is _assemble_amg_ladder. blockdiag-of-n-copies splats become kron;
one such splat was an obfuscated identity multiply. plot3d's three plotter
epilogues and the 2D/3D animation timelines share helpers. CUDA's
Vector{T}(Array(x)) double copies drop the outer constructor where the eltypes
already match (the Int32-to-Int sites are genuine conversions and stay).
Slop. The GPU AssemblyPlan uploaded col_indices and c_counts to the device and
never read them, holding Int32 matrices in VRAM for the whole solve. Dead
identifiers go: a "hi" string placeholder for a Matrix, an unused VTK_LINE, an
unused KindedVariable field that was the struct's only reason to be parametric,
unused hierarchy parameters, unused imports, a stale commented-out formula.
Comments that narrated a past diff for a reviewer are rewritten to describe the
code as it stands.
Tests. New regression tests cover spectral left extrapolation and the plot
keyword forwarding. Full Pkg.test passes, including the Zoo reference values,
the JuMP and Gmsh suites, and the PyVista plotting tests.
Held back pending discussion: the mesh constructors' unused rest... kwarg
slurp, the objectid-keyed plan caches, the RR2 component-selection matrices,
and a purely stylistic @eval rewrite of the CUDA map_rows arities.
0 commit comments