Skip to content

Latent C++ correctness follow-ups from the fable review #6

Description

@balbasty

Confirmed defects from the fable review that are unreachable via the currently-exposed API (so they change no current behaviour) but should be fixed before the relevant paths are wired up. The live/critical findings were already fixed in the correctness pass; these are the remainder, tracked rather than rushed because verifying them requires exercising code paths the tests don't yet reach.

  • reg `` hard-codingfastfields-cpu-impl/reg_flow.h and `reg_field.h`: every matvec/kernel/diag site instantiates `Impl::template …` although the functions take a `char op` template param; jitfields (and the CUDA impl) dispatch `opfunc = Op::f`. `cpu-lib` only ever passes `'='`, so results are correct today, but a future `'+'`/`'-'` would silently overwrite instead of accumulate. Port the CUDA impl's `opfunc` pattern to the CPU impl (~15 sites).
  • Dynamic spline/bound bypassfastfields-kernels/pushpull/1d.h (Any-1D push, ~line 1265) computes runtime b/s then calls utils::index(loc,size,ix,wx,fx) without forwarding them; pushpull/utils.h cubic hindex (~1220) builds the *_fn dynamic dispatchers then bypasses them. Harmless today (cpu-lib dispatches spline/bound statically, orders 0–7) but breaks the Dynamic path the Config machinery exists to support. Forward b/s (watch the s shadowing vs the stride loop variable).
  • DLTensor.strides == NULL — legal DLPack for compact row-major tensors, but fastfields-cpu-lib/distance.cppCANUSE32BITS/copy_if_needed (and the other modules' dispatch) dereference strides unconditionally → nullptr deref. nanobind always supplies strides so the current bindings never hit it, but external DLPack producers can. Add a null-strides = contiguous fallback in the shared dispatch/autocast path.
  • tetrahedron.h rasterization mathfastfields-kernels/tetrahedron.h:151-160: the scanline loop runs y = ceil(p1y) .. floor(p1y) (≤1 row; should span to the top vertex) and the edge-intersection divides by (p2y-p1y)*y (the *y is spurious). Unported prototype (not in any dispatch layer or test), inherited from a jitfields draft — fix the math when porting it.

Workstream: claude-jitfields-to-fastfields

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions