From 78341e309252a5b5d9db90fe8e205a39373834bb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 12:20:44 +0000 Subject: [PATCH 1/5] docs: propose what core/ means, and a naming scheme, with measured costs Answers the two questions the owner asked together -- what moves from impl/kernels/ to core/, and how namespaces, kernels and internal helpers should be named -- plus the thread-count API question that follows from them. The recommended definition, which decides every borderline file: core/ holds every header that more than one layer depends on and that is not itself the computation of a named fastfields operation -- device- specialised or not. impl/kernels/ keeps only the per-element maths of a named operation. That admits atomic.h (device-specialised, but not an operation) and the parallel/threadpool group, and it excludes splinc.h (which IS the operation exported as spline_coeff). Ten files move, 4,262 lines; the closure is clean, so core/ never acquires a dependency on impl/kernels/. Two premises in the brief turned out not to hold, and both are recorded with the evidence: * core/ was said to be barred from opening the device namespace. core/autocast.h already opens FF_NAMESPACE_BEGIN(FF_DEVICE), and core/cuda_switch.h exists to branch on __CUDACC__. The real constraint is narrower: a core/ header must compile under both compilers and mean the right thing in each. No fourth directory is needed, and no core/cpu + core/cuda split. * core/dispatch.h says canUse32BitIndexMath comes from core/autocast.h. It is defined in impl/kernels/utils.h, which is why 17 files in src/ include the kernels layer directly. The dependency inversion this proposal repairs already exists, and is mis-documented. Findings that are independent of any decision here, worth acting on separately: impl/cpu/tetrahedron.h does not compile and is referenced by nothing; impl/kernels/vector/ (2,389 lines) is used only by a scratch target and duplicates the point hierarchy in distance/mesh_utils.h; and the CUDA backend launches all 39 kernels at the architectural maximum of 1024 threads per block while checking for no launch error anywhere, so a cudaErrorLaunchOutOfResources would be discarded silently. tools/move-core-headers.py carries the move with a self-verifying --check, following the pattern of rename-macros.py and dedup-dispatch-helpers.py. It does not hard-code an include delimiter -- it measures the tree's dominant one -- so it survives a rebase by being re-run rather than hand-merged. That was demonstrated rather than asserted: #146 and #143 landed mid-review, and the same script produced the angle-bracket spelling on the new base with no edit. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z --- docs/proposals/core-and-naming.md | 820 ++++++++++++++++++++++++++++++ tools/move-core-headers.py | 231 +++++++++ 2 files changed, 1051 insertions(+) create mode 100644 docs/proposals/core-and-naming.md create mode 100755 tools/move-core-headers.py diff --git a/docs/proposals/core-and-naming.md b/docs/proposals/core-and-naming.md new file mode 100644 index 0000000..1f9164b --- /dev/null +++ b/docs/proposals/core-and-naming.md @@ -0,0 +1,820 @@ +# Proposal: what `core/` means, and how things are named + +**Status: proposal. Nothing here is a decision.** The prototype on this branch +exists to turn cost estimates into measurements; it is not the migration. + +Two questions were asked together because they are the same question twice: +*what is this layer for, and does its name say so?* + +Every number below is measured on `85fdac7`, not estimated. The commands are +in [Appendix A](#appendix-a--how-the-numbers-were-measured). + +--- + +## 0. Summary + +| | Recommendation | +| --- | --- | +| **`core/` means** | everything more than one layer depends on that is not itself the maths of a named operation — device-specialised or not | +| **Move** | the owner's eight, plus `spline.h`; `bounds.h` and `spline.h` move whole | +| **Do not move** | `vector/` (superseded, unused), `tetrahedron.h` (dead), `splinc.h` (an operation) | +| **Namespaces** | add `vox` *inside* each module: `ff::::::vox` | +| **`atomic.h`** | real bug, not just inconsistency — fix during the move | +| **`_impl` functions** | adopt #147's `flow_slice::` precedent; keep `_x` for locals | +| **Thread API** | it already exists — export it; do **not** mirror it on CUDA | + +The single most important finding is not in either question as asked: +**`include/fastfields/impl/cpu/tetrahedron.h` does not compile**, is referenced +by nothing, and is built by nothing. See §1.4. + +--- + +## 1. Question 1 — what moves to `core/` + +### 1.1 The definition + +The owner has ruled that `core/` means *shared infrastructure, possibly +device-specialised* — the disqualifier is being a kernel implementation, not +being device-aware. That ruling is right, and the tree already assumed it: + +- **`core/cuda_switch.h`** exists to branch on `__CUDACC__`, and pulls in + `` under nvcc. `core/` has never been free of device code. +- **`core/autocast.h` already opens `FF_NAMESPACE_BEGIN(FF_DEVICE)`.** So the + premise that `core/` must not open the device namespace was never true; one + of the five files there does it today. + +So the constraint is narrower than "backend-agnostic". It is: + +> A `core/` header must compile under the host compiler *and* nvcc, and must +> mean the right thing in each. + +`ff::::` satisfies that — each translation unit resolves `FF_DEVICE` +to the one backend it is being compiled for, which is exactly why `autocast.h` +works. The proposed definition, which decides every borderline file: + +> **`core/` holds every header that more than one layer depends on and that is +> not itself the computation of a named fastfields operation — device-specialised +> or not. `impl/kernels/` keeps only the per-element maths of a named operation.** + +"Named operation" means the eight families the library exposes: distance, +posdef, resample, restriction, spline_coeff, pushpull, reg_field, reg_flow. +Boundary conditions and spline weights are not operations; they are the +vocabulary every operation is written in. + +### 1.2 The file list + +Ten files. The dependency closure is **clean**: every one of them includes only +`core/` headers and other files in this list, so `core/` never acquires a +dependency on `impl/kernels/`. That is checked mechanically, not asserted — +`tools/move-core-headers.py --check` fails if it stops being true. + +| File | Lines | Namespace today | Why it moves | +| --- | ---: | --- | --- | +| `utils.h` | 701 | `ff::` | Numeric/type helpers with zero domain knowledge. **52 include sites, 17 of them in `src/`** — the dispatch layer already reaches into the kernels layer for it. | +| `bounds.h` | 800 | `ff::bound` **and** `ff::::bound` | `bound::type` is used **209 times across 18 files in `src/`**. Also carries the `FF_STATIC_BOUND_*` / `FF_BOUND_SEL` build-policy macros — the definition of "shared by both dispatch layers". | +| `spline.h` | 1447 | `ff::spline` **and** `ff::::spline` | Same shape as `bounds.h`: `spline::type` used 23× in `src/`. Not on the owner's list as stated, but it should be — see §1.3. | +| `batch.h` | 273 | `ff::` | Linear-index ↔ sub-index conversion. Used by **12 files in `impl/cpu` and 11 in `impl/cuda`** — genuinely cross-backend. | +| `atomic.h` | 366 | `ff::` / **global** | The accumulate primitive. Device-specialised by construction, which the new definition admits. Carries a real bug — §2.2. | +| `meta.h` | 43 | `ff::meta` | `Pack` / `Tuple` metaprogramming. Pure infrastructure. | +| `parallel.h` | 55 | `ff::` | `parallel_for` + the grain-size policy. | +| `parallel_impl.h` | 235 | `ff::`, `ff::internal` | Its backend selection (native / OpenMP / none). Travels with `parallel.h`. | +| `threadpool.h` | 260 | `ff::` | The work-stealing pool. | +| `threadpool.inl` | 79 | `ff::`, `ff::internal` | Its inline definitions. Travels with `threadpool.h`. | + +**4,262 lines.** `core/` goes 5 → 15 files; `impl/kernels/` goes 57 → 47 and +then contains only per-operation implementations, plus the two exceptions in +§1.4. + +**The pairs travel as pairs — confirmed, and it is stronger than "convention".** +`parallel.h` includes `parallel_impl.h`, `parallel_impl.h` includes +`threadpool.h`, and `threadpool.h` includes `threadpool.inl`. It is a single +include chain; there is no cut point. `parallel_impl.h`, `threadpool.h` and +`threadpool.inl` have **one include site each** — they are reached only through +`parallel.h`. + +### 1.3 `spline.h` versus `splinc.h` — the owner's instinct is right + +The owner's list moves `spline` and not `splinc`, and that is correct, but the +reason is worth stating because the names are one character apart and the +distinction is not visible from them: + +- **`spline.h` (1447 lines)** is the B-spline *basis*: `weight0..7`, + `grad0..7`, `hess0..7`, and the `spline::type` enum. It is a shared + primitive that pushpull, resize and restrict all call. It is not an + operation. → **moves.** +- **`splinc.h` (339 lines)** is the prefilter *implementation*. It is the + operation exported as `spline_coeff`. → **stays.** + +The one-character gap between the names is the trap. Since a rename is on the +table anyway, the cheapest fix is to make the moved file's new home carry the +distinction: `core/spline.h` (basis) versus `impl/kernels/splinc.h` (operation) +already reads better than the two sitting side by side. If more is wanted, +`splinc.h` → `spline_coeff.h` aligns it with its exported name (`resize.h` and +`restrict.h` have the same mismatch — see §2.4). + +**On moving `bounds.h` and `spline.h` whole.** Both files are already two files +glued together: a device-free vocabulary half (`ff::bound`, `ff::spline` — the +`enum class type`, `BoundVecN`, `SplineVecN`) and a device-scoped maths half +(`ff::::bound`, `ff::::spline`). The device half already re-exports +the vocabulary: + +```cpp +FF_NAMESPACE_BEGIN(FF_DEVICE) +FF_NAMESPACE_BEGIN(bound) +using FF_NS::bound::type; // bounds.h:203 +using FF_NS::bound::transpose; +using FF_NS::bound::BoundVec; +``` + +So a purist split — vocabulary to `core/`, maths stays — is *available* and the +bridge for it already exists. I recommend **against doing it now**: it turns +two renames git can follow into two deletions and four creations, loses blame +continuity on 2,247 lines, and buys nothing the whole-file move does not. It is +a clean follow-up if the owner ever wants `core/` to be small. Flagging it +because the seam is real and someone will otherwise rediscover it. + +### 1.4 What I would **not** move — and two things that should not be there at all + +**`vector/` — 11 files, 2,389 lines. Do not move. It is superseded, not +infrastructure.** + +The brief asks whether it is infrastructure or implementation. Measured, it is +neither, because **nothing uses it**. Its only includer in the entire tree is +`tests/kernels/vector/test.cpp`, which is not in the gate — it is a hand-run +scratch target that compiles but never runs (`Makefile:50`, "A hand-run scratch +program … compiled (not run)"). + +It is also not merely unused, it is *duplicated*. +`impl/kernels/distance/mesh_utils.h` (1,158 lines, and in the gate) contains an +independent parallel hierarchy of the same abstraction — `StridedPoint`, +`RefPoint`, `SizedStridedPointer`, `Sized`, `ConstStridedPoint` — against +`vector/`'s `AbstractVector`, `WeakRef`, `AbstractSizedPointer`, `Sized`. Two +implementations of one idea; the used one is the other one. + +Moving 2,389 unreferenced lines into the directory whose every edit triggers the +full CI matrix, CUDA included, is the wrong direction. Recommend deciding its +fate separately: either adopt it in `mesh_utils.h` (and *then* it is core +infrastructure) or delete it. 9 of its 11 files also hardcode `namespace ff {` +instead of the `FF_NS` macro, which is its own small argument that it has not +been maintained alongside the rest. + +**`tetrahedron.h` — dead, in two divergent copies, one of which does not +compile.** + +The brief asks where it sits. The answer is that this is not a placement +question: + +- `impl/kernels/tetrahedron.h` (243 lines, `ff::::tetra`) — **zero** + references anywhere in `include/`, `src/`, `tests/` or any Makefile. +- `impl/cpu/tetrahedron.h` (3.4 KB, `ff::tetra` — hardcoded, and missing the + `cpu` level every other file in that directory has) — also zero references, + **has no `#include` directives at all**, and does not compile: + +``` +include/fastfields/impl/cpu/tetrahedron.h:42:28: + error: use of undeclared identifier 'GRAIN_SIZE' + 42 | parallel_for(0, numel, GRAIN_SIZE, [&](long start, long end) { +``` + +Neither is compiled by anything, so neither is syntax-checked by CI. They are +the stub for `fastfields-kernels#30` ("Implement the tetrahedron rasterization +(invfield) kernel"). Whoever picks that issue up will start from a file that +does not build and will assume they broke it. + +Recommend: leave `impl/kernels/tetrahedron.h` where it is (it *is* +per-element geometry for a future operation, so it is correctly placed), and +either fix or delete `impl/cpu/tetrahedron.h`. Either way, add both to a +compile-only target so the tree stops carrying headers nobody has ever built. +This is independent of the move and can land first. + +**`splinc.h`, `resize.h`, `restrict.h`, `distance/`, `posdef/`, `pushpull/`, +`regularisers/`** — all operations. Stay. + +### 1.5 Does this need a fourth directory, or `core/cpu` + `core/cuda`? + +**No.** The brief asks, and the answer falls out of §1.1: `FF_DEVICE` already +partitions the namespace per compiler, so a device-specialised header in a +single `core/` directory produces the right symbols in each backend without any +directory split. `core/autocast.h` has been doing this the whole time. + +A `core/cpu` + `core/cuda` split would also be actively wrong for +`atomic.h`, whose whole job is to present *one* interface +(`ff::anyAtomicAdd`) over two implementations. Splitting the directory would +split the file that exists to not be split. + +The one honest wrinkle: `parallel.h` and `threadpool.h` are CPU-only in fact — +their 13 include sites are **all** in `impl/cpu/` (plus one test). Pure +dependency logic puts them in `impl/cpu/`, not `core/`. See §3.3, where the +owner's discoverability argument and Question 4 settle it. + +--- + +## 2. Question 2 — naming + +### 2.1 Namespaces: the problem is not "marking voxelwise-ness" + +The owner's `vox` idea is right, but for a stronger reason than the one given. +The real defect is that **the kernels layer and the impl layer share a +namespace**. All seven module namespaces are opened by both: + +| Namespace | opened in `impl/kernels/` | opened in `impl/{cpu,cuda}/` | +| --- | ---: | ---: | +| `ff::::pushpull` | 5 files | 2 | +| `ff::::reg_field` | 4 | 2 | +| `ff::::reg_flow` | 4 | 2 | +| `ff::::posdef` | 3 | 2 | +| `ff::::resize` | 1 | 2 | +| `ff::::restrict` | 1 | 2 | +| `ff::::splinc` | 1 | 2 | + +**Today this does not collide** — and I want to be precise, because the obvious +grep overstates it. At namespace scope the overlap is **zero**. It is avoided +because the kernels do not expose free functions at all: every kernel entry +point is a static member of a class template — `Kernels` (pushpull, reg_field), +`RegFlow`, `Multiscale` (resize, restrict), `Child` (posdef) — while the impl +drivers are free functions in the same namespace. Members and free functions +cannot collide. + +That class wrapper is the workaround. Two pieces of evidence that it is +load-bearing rather than incidental: + +1. **`impl/cpu/pushpull.h:37-82`.** Inside the driver `void pull(...)`, there + is a lambda *also called* `pull` that shadows it, whose body calls the + kernel `PushPull<...>::pull`: + + ```cpp + void pull( /* ...the driver... */ ) { + auto pull = [&](const reduce_t * loc, offset_t o, offset_t i) { + return PushPull::pull(...); // the kernel + }; + ... + pull(loc, out_offset, inp_offset); // the lambda + } + ``` + + Three different things called `pull` in one scope. Remove the shadowing + lambda and line 82 recurses into the driver. + +2. **The `teeny` branch already hit the collision and already fixed it this + way.** Commit `b7dbd08`, *"pushpull: nest voxel kernels in + `ff::cpu::pushpull::vox`"*: **"Avoids a name collision with the impl layer's + loop drivers, which are also `ff::cpu::pushpull::pull/push/count/grad`."** + On `teeny` the kernels had been refactored into free functions, at which + point the shared namespace stopped being survivable. + +So: `vox` is not cosmetics. It is what lets a kernel be a plain function. + +### 2.2 Recommended spelling: `ff::::::vox` + +`vox` goes **inside** the module, not above it. + +| Option | Verdict | +| --- | --- | +| `ff::::::vox` | **Recommended.** Smallest diff; the level where the collision actually is; keeps module cohesion; matches what `teeny` already did, so `main` and `teeny` converge rather than diverge further. | +| `ff::::vox::` | Groups all kernels under one node and allows one `using`, but separates a kernel from its module and is a larger conceptual change for no extra safety. | +| `ff::vox::` | **Wrong.** Kernels genuinely differ by device (`FF_CUDEV`, the `half` specialisation in `utils.h`), so they must stay device-scoped. | +| Neither | Leaves the class-template workaround permanently load-bearing and leaves `main` diverging from `teeny`. | + +**`vox` versus `wise`.** `wise` is an adjective fragment; +`ff::cpu::pushpull::wise::pull` does not read as anything. `vox` reads, is +short at every call site, and has precedent in this codebase. + +**The honest caveat, since the owner should get the trade and not a false +verdict:** `vox` is slightly inaccurate. Not everything in this layer is a +voxel — `posdef` operates on one small matrix, `distance/mesh` on one point +against a mesh, `tetrahedron` on a tetrahedron. The project's own prose says so: +`CLAUDE.md` describes the layer as *"single-element math (one voxel, one point, +one small matrix)"* and the impl layer as owning *"the loops over elements"*. +By the codebase's own vocabulary the accurate name is **`elem`**. + +I recommend `vox` anyway, on two grounds: the `teeny` precedent exists and +choosing differently means renaming twice, and `vox` matches the project's +name and domain. But if the owner would rather be accurate than convergent, +**this is the only cheap moment to pick `elem`** — after the sweep it is +another 28-site rename. The `teeny` precedent is genuinely thin (one commit, +seven lines, one file), so it should not by itself decide this. + +**Cost, measured.** 26 `FF_NAMESPACE_BEGIN()` sites across 25 files in +`impl/kernels/` — 52 line edits with the matching `END`s. Broken down: +`pushpull` 5, `reg_flow` 4, `reg_field` 4, `posdef` 3, `distance_mesh` 3, and +one each for `resize`, `restrict`, `splinc`, `tetra`, `distance_e`, +`distance_l1`, `distance_spline`. Call sites in the impl layers mostly need +*no* change, +because they reach kernels through the class templates (`Kernels<...>::`, +`Multiscale<...>::`), which are found by ordinary lookup from the enclosing +namespace either way. `posdef` is the exception at 168 qualified uses. This is +a small change that unlocks a later, larger one (turning kernels into free +functions); it does **not** have to be done at the same time as the file move, +and should not be. + +### 2.3 `atomic.h` — the concrete test case, and a real bug + +The coordinator asked me to confirm the scoping before asserting it. Confirmed, +and it is worse than described. + +**CPU half** (`#ifndef __CUDACC__`, lines 13–89): opens `FF_NAMESPACE_BEGIN(FF_NS)` +only → `ff::`. No device level, unlike the rest of the layer. Exposes +`ff::anyAtomicAdd`, `ff::anyAtomicAddNoReturn`, and also `ff::AtomicAdd`, +`ff::has_atomic_add`, `ff::has_fetch_add`. + +**CUDA half** (lines 94–364): contains **no namespace macros at all**. Verified +by brace-depth tracking, these sit at **global scope**: + +``` + 97: struct AtomicFPOp; + 100: struct AtomicFPOp + 119: struct Atomic##NAME##IntegerImpl (×4, via FF_ATOMIC_INTEGER_IMPL) + 263: static inline FF_CUDEV double atomicAdd(double* address, double val) + 279: static inline FF_CUDEV double gpuAtomicAdd(double*, double) + 283: static inline FF_CUDEV float gpuAtomicAdd(float*, float) + 341: static inline FF_CUDEV void gpuAtomicAddNoReturn(double*, double) + 345/347: gpuAtomicAddNoReturn(float*, float) +``` + +Only the two `anyAtomicAdd` wrappers are namespaced, and they use a hardcoded +`namespace ff {` rather than `FF_NAMESPACE_BEGIN(FF_NS)` — the one place in +`include/` outside `vector/` that does. + +Three distinct problems: + +1. **Line 263 declares an overload of CUDA's own built-in `atomicAdd` at global + scope.** It is guarded by `__CUDA_ARCH__ < 600`, and the file's own comment + admits the hazard: *"defining it for sm_60+ collides with the built-in"*. So + the collision is understood to be real and is avoided by an arch guard + rather than by scoping. +2. **`AtomicFPOp` and `gpuAtomicAdd` are ATen's names at ATen's scope**, in an + *installed public header*. The file says so in its first line ("CUDA portion + copied from PyTorch/ATen"). `fastfields-torch` is a planned downstream + binding; a translation unit that includes both this header and PyTorch's + CUDA atomics is the expected case, not an exotic one. +3. **`FF_GPU_ATOMIC_INTEGER` (line 200) is defined and never used anywhere**, + and neither it nor `FF_ATOMIC_INTEGER_IMPL` is `#undef`'d, so both leak into + every CUDA TU downstream. They are `FF_`-prefixed, so they satisfy + `CLAUDE.md`'s rule — but a dead macro on the installed surface is still dead. + +**Recommended fix, and it is the case that tests the scheme.** Under §2.2's +scheme, `atomic.h` becomes `core/atomic.h` with: + +```cpp +FF_NAMESPACE_BEGIN(FF_NS) +FF_NAMESPACE_BEGIN(FF_DEVICE) // ff::cpu:: or ff::cuda:: +FF_NAMESPACE_BEGIN(atomic) + // both halves: the implementation details, whichever branch is live +FF_NAMESPACE_END(atomic) + // ff::::anyAtomicAdd -- one name, two implementations +FF_NAMESPACE_END(FF_DEVICE) +FF_NAMESPACE_END(FF_NS) +``` + +Note what this does and does not change. `anyAtomicAdd` **moves from `ff::` to +`ff::::`**. That is a behavioural improvement (the CPU and CUDA +implementations stop sharing a name at the same scope) but it is a real API +change for its callers — currently `core/bounds.h` and +`tests/kernels/atomic/test.cpp`, i.e. two files. Cheap now; not free. + +`#undef FF_ATOMIC_INTEGER_IMPL` after its single use at line 209, and delete +`FF_GPU_ATOMIC_INTEGER`, are both unconditional wins and can land immediately, +independent of everything else here. + +### 2.4 Kernel naming — what is actually there + +Surveyed before proposing, as asked. The inconsistencies are real but they are +**not random drift**, and that changes what to do about them. + +**(a) Case style splits exactly along provenance.** Everything project-original +is `snake_case`. Every `camelCase` identifier is in a vendored or adapted file: + +| Identifier | File | Upstream | +| --- | --- | --- | +| `anyAtomicAdd`, `gpuAtomicAdd`, `atomicAddNoReturn` | `atomic.h` | PyTorch/ATen | +| `pushWork`, `requestSteal`, `stealWork`, `threadFunc`, `threadId` | `threadpool.h` | wstpool (MIT) | +| `canUse32BitIndexMath` | `utils.h` | ATen | + +Recommend **leaving these spellings alone**. They are the traceability to +upstream, and renaming them makes future diffs against upstream harder for no +gain. Document the rule instead: *snake_case for our code; a vendored file +keeps its upstream spelling*. That converts an apparent inconsistency into a +stated convention, which is cheaper and more honest than a rename. + +The one exception worth considering is `canUse32BitIndexMath`, because it is no +longer really theirs: `core/dispatch.h`'s `FF_CANUSE32BITS` macro calls it, +**250 times across 21 files in `src/`**. See §2.5 for a documentation bug attached to it. + +**(b) "In-place" is spelled three ways.** This is genuine drift: + +| Spelling | Where | Example | +| --- | --- | --- | +| trailing `_` | distance, posdef, public ABI | `add_`, `solve_`, `sym_invert_` | +| `i` prefix | `posdef/utils.h` | `iadd`, `isub`, `imul`, `idiv`, `iaddcmul`, `idivcadd` | +| `to_` infix | `distance/mesh_utils.h` | `addto_`, `multo_`, `crossto_`, `divto_` | + +The trailing `_` is dominant and already the public ABI's convention +(`field_precond_`, `sym_solve_`). Recommend it as the rule. **But** `add_` and +`addto_` are not synonyms — `add_(other)` is `this += other`, `addto_(lhs, rhs)` +is `this = lhs + rhs`. That is an under-documented distinction, not a +redundancy, so a blind merge would be a behaviour change. Recommend renaming +only the `i`-prefix family (7 names, one file) and *documenting* the +`add_`/`addto_` pair. + +**(c) Internal namespaces are spelled three ways**: `_sign`, `_splinc`, +`_spline` (leading underscore) versus `internal` (`parallel_impl.h`, +`threadpool.inl`) versus `posdef::internal`. Recommend `internal` everywhere — +it is already the majority, it needs no sigil rule, and it matches #147's +choice (§2.5). Five namespace renames. + +**(d) Module namespace does not match directory**, in a way that will confuse: + +| Directory | Namespace | Public name | +| --- | --- | --- | +| `distance/euclidean.h` | `distance_e` | `dt_euclidean` | +| `distance/l1.h` | `distance_l1` | `dt_l1` | +| `regularisers/field/` | `reg_field` | `field_*` | +| `resize.h` | `resize` | `resample` | +| `restrict.h` | `restrict` | `restriction` | +| `splinc.h` | `splinc` | `spline_coeff` | +| `tetrahedron.h` | `tetra` | — | + +Three vocabularies for the same module. Note the constraint from `MIGRATION.md`: +the public names had to differ because *a namespace cannot share a name with a +function inside `ff::cpu`*, which is why `resize`→`resample` happened. That +constraint applies to the **flat exported surface**, not to the kernels layer — +which is precisely why the kernels layer solved the same problem by nesting +instead. Both solutions are defensible; having both, undocumented, is what +costs. + +Recommend the low-risk half only: rename the file to match the exported name +where they differ (`resize.h`→`resample.h`, `restrict.h`→`restriction.h`, +`splinc.h`→`spline_coeff.h`, `distance_e`→`distance_euclidean`). Leave the +namespaces alone — renaming those is where the collision risk is, and it buys +less than the file rename does. **This should be a separate PR from the move**; +bundling a rename into a relocation makes both unreviewable. + +### 2.5 "impl" functions — the sigil is overloaded, and #147 already fixed it + +Measured across the 21 dispatch translation units (`src/lib-cpu`, +`src/lib-cuda`): **116 distinct `_`-prefixed identifiers, 2,943 mentions.** + +The important finding is that the sigil means **two different things**: + +| Role | Examples | Count | +| --- | --- | --- | +| Narrowed local — the typed/32-bit copy of an ABI argument | `_stride_out` (195), `_stride_inp` (112), `_out` (74), `_stride_wgt` (84) | the large majority | +| Internal dispatch template | `_flow_matvec` (16), `_field_diag` (28), `_flow_kernel` (28), `_sym_matvec` | a few dozen | + +The brief notes correctly that this is *legal* — `_x` is reserved only at global +scope, and these are inside `ff::…`. The problem is not legality, it is that +one sigil marks two unrelated things in the same file. + +**Recommendation: split the roles, and do not invent a convention — adopt the +one #147 is already landing.** + +- **Keep `_x` for the narrowed local.** It is consistent, it is meaningful + (`_out` is the typed `out`), and it is ~2,800 sites. Renaming it is pure + churn. +- **Drop it for functions.** Put them in a named nested namespace. + +That is exactly what PR #147 does. Its new seam declares +`ff::cuda::flow_slice::matvec_3d`, `diag_3d`, `relax_1d` — a named nested +namespace, hidden visibility, **no underscore**. So the convention is already +being chosen by in-flight work; the recommendation is to generalise it rather +than compete with it. `detail::` or `internal::` for the general case +(`internal` is already in use, per §2.4c). + +Note **anonymous namespaces will not do the job here**, though all 21 files +already use them: an anonymous namespace nested inside `ff::cuda` cannot hold a +`flow_matvec` alongside the exported `ff::cuda::flow_matvec` without making +unqualified calls ambiguous. A *named* nested namespace is the mechanism that +works, which is presumably why #147 reached for one. + +One small thing to flag to whoever reviews #147: its seam opens `namespace ff {` +/ `namespace cuda {` literally rather than via `FF_NAMESPACE_BEGIN(FF_NS)`. +Defensible in `src/lib-cuda` (where `cuda` really is fixed), but `FF_NS` exists +so the root namespace is spelled in one place. + +**A documentation bug found on the way.** `core/dispatch.h:28` and `:48` state +that `canUse32BitIndexMath` comes from `core/autocast.h`. It does not — it is +defined at `impl/kernels/utils.h:668`, and `autocast.h` only mentions it in a +comment. That is why the 17 `src/*.cpp` files that need it also include +`impl/kernels/utils.h` directly. So **`core/` already depends on a symbol that +lives in `impl/kernels/`** — the inversion this proposal repairs is not +hypothetical, it is load-bearing today and mis-documented. Worth fixing the +comment even if nothing moves. + +--- + +## 3. Question 4 (new) — a thread-count API + +### 3.1 The CPU API already exists. It is not exported. + +This is the headline. `include/fastfields/impl/kernels/threadpool.inl:58-69` +already defines, fully implemented: + +```cpp +inline size_t set_num_threads(size_t nthreads); // ff:: +inline size_t get_num_threads(); // ff:: +``` + +and `parallel_impl.h:61-79` already wraps them in a **backend-abstracted** pair +that also covers the OpenMP and single-threaded builds: + +```cpp +ff::get_parallel_threads(); // native pool | omp_get_max_threads() | 1 +ff::set_parallel_threads(int); +ff::get_parallel_backend(); // "native" | "omp" | "none" +``` + +So the task is **export**, not design. The public API should forward to +`set_parallel_threads`, not `set_num_threads`, so that an OpenMP build answers +correctly. + +**A name collision to resolve first.** The obvious public name `ff::set_num_threads` +is *already taken* by the internal header-only helper above. Declaring an +exported `ff::set_num_threads` in `api/` while the inline one is in scope is +asking for an ambiguity or an ODR problem. Recommend three distinct names for +three distinct roles: + +| Role | Name | Where | +| --- | --- | --- | +| Exported public API | `ff::set_num_threads(int)` / `ff::get_num_threads()` | `api/threads.h` + `src/lib/threads.cpp` | +| Backend-abstracted | `ff::set_parallel_threads` / `get_parallel_threads` | `core/parallel_impl.h` (unchanged) | +| Native-pool internal | `ff::internal::set_pool_size` / `pool_size` | `core/threadpool.inl` (**renamed**) | + +**Where it lives:** the hub (`api/` + `src/lib/`), because it is user-facing and +must be an exported symbol in `libfastfields.so`. It is *not* a device dispatch +— thread count is CPU-only — so `src/lib/threads.cpp` forwards straight to +`FF_CPU::` with no `device_type` switch. That is a deliberate asymmetry with +every other file in `src/lib/` and should be commented as such. + +### 3.2 Three defects a public setter would expose + +**(a) Negative input spawns unbounded threads.** `set_parallel_threads` takes +`int`, `set_num_threads` takes `size_t`, and nothing validates: + +```cpp +set_parallel_threads(-1) + -> set_num_threads(size_t(-1)) // 18446744073709551615 + -> if (nthreads == 0) ... // false, guard misses + -> num_threads() = static_cast(...) // -1 + -> new ThreadPool(num_threads()) // int -1 -> size_t count + -> for (size_t i = 0; i < count; i++) mWorkers.emplace_back(...) +``` + +Unbounded thread creation until the process dies. Harmless while the function +is internal and every caller passes a sane constant; not harmless the moment it +is public. **A public setter must validate**: reject `n <= 0`, and clamp to some +sane ceiling. + +**(b) The setter is unsynchronised.** `internal::num_threads()` (an `int&` to a +function-local static) and `internal::global_pool()` (a `shared_ptr&`) are +mutated with no lock. Concurrent `set_num_threads` and `get_global_pool()` is a +data race on the control block. **#97 just fixed a data race and a lost-wakeup +deadlock in this pool that were reachable only at higher thread counts** — a +setter is precisely the thing that makes those counts reachable on purpose. +Recommend a mutex around the (count, pool) pair. + +The *resize* semantics are already safe and worth keeping: `get_global_pool()` +returns `shared_ptr` **by value**, so an in-flight `parallel_for` holds a +reference and the old pool outlives the `reset()`. Document that as the +contract: *"takes effect for parallel regions that begin after it returns; +in-flight regions keep their pool."* That is the same guarantee +`omp_set_num_threads` and `torch.set_num_threads` give, so it needs no +explaining to users. + +**(c) The default is `hardware_concurrency() / 2` — deliberate, but a +heuristic.** `threadpool.inl:30-36`: + +```cpp +auto num_threads = std::thread::hardware_concurrency(); +#if defined(_M_X64) || defined(__x86_64__) + num_threads /= 2; +#endif +``` + +The halving is guarded on **x86_64 only**, which makes its intent unambiguous: +it is an SMT/hyper-threading correction (assume 2-way SMT, estimate physical +cores), not a "use half the machine" policy. It is inherited from ATen, and it +is correct in intent — physical cores usually beat logical ones for this kind of +work. It is still a *guess*: it halves wrongly on an x86_64 machine with SMT +disabled (common on cloud VMs pinned one thread per core), and it does nothing +on ARM, which has no SMT and needs no correction. + +**Verdict: deliberate, keep it, but document it and let the setter override it.** +Worth noting the CI consequence: `ubuntu-latest` has 4 vCPUs, so the pool +defaults to **2 threads**. That is a thin margin for the `tsan` leg to find +concurrency bugs in — which is consistent with #97's bugs having needed higher +counts to surface. + +**Test coverage.** Whatever lands should be exercised by the new +`test-cpu (tsan, grain=1)` leg at **more than one thread count** — the setter's +whole purpose is to reach counts CI otherwise never sees. `tests/kernels/atomic/` +and the `test-atomics` target are the natural place; note the gate is fixed at +59,886 checks / 13 suites, so this must go in a target outside `make test`, +exactly as `test-atomics` already does. + +### 3.3 This is also what settles `threadpool` → `core/` + +The owner leans `core/` for discoverability and worries the pool would be "lost +in the sea of actual implementations" in `impl/cpu/`. **Pure dependency logic +disagrees**: all 13 `parallel.h` include sites are in `impl/cpu/` (plus one +test), and `threadpool.h` has exactly one includer, `parallel_impl.h`. By +dependency alone, the whole group belongs in `impl/cpu/`. + +But dependency logic is answering the wrong question once §3.1 lands. A file +that backs an **exported public API** is not an implementation of an operation +— it is infrastructure, by any reading of §1.1's definition. That converts the +owner's discoverability instinct into a structural argument, which is stronger +than the one they made. + +**Recommendation: `core/`, and I agree with the owner — but the reasoning is +conditional.** If the thread-count API is exported, `core/` is correct on the +merits. If Question 4 is declined, `impl/cpu/` is the defensible home and the +argument is only discoverability. Since Question 4 looks likely to land, and +since moving it twice is worse than moving it once, `core/` now is the right +call either way. + +### 3.4 CUDA: do not mirror it. But there is a real bug here. + +**Allowed?** Yes — threads-per-block is purely a launch parameter, and +`impl/cuda/utils.h:20` already takes it as an argument +(`GET_BLOCKS(N, max_threads_per_block = CUDA_NUM_THREADS)`). + +**Recommended as a user-facing knob?** No, and the coordinator's reasoning is +right: CPU thread count is a **resource-sharing** control (how much of a shared +machine you may consume); CUDA threads-per-block is a **work-shaping** control +(occupancy and efficiency, not consumption). Exposing them symmetrically +implies a parallel that does not exist and invites users to "limit GPU usage" +with a knob that cannot do that. Limiting GPU consumption is MPS/MIG/stream +territory. + +**But the `CUDA_NUM_THREADS = 1024` question is more serious than the API +question, and it is a latent correctness bug.** Measured: + +- `impl/cuda/utils.h:16` — `static constexpr int CUDA_NUM_THREADS = 1024`. + 1024 is the *architectural maximum* any current NVIDIA arch permits. +- **All 39 `<<<...>>>` launch sites in `impl/cuda/` use it.** The + `max_threads_per_block` parameter of `GET_BLOCKS` is never once overridden — + it is dead. +- **There is no launch error checking anywhere.** `cudaGetLastError` and + `cudaPeekAtLastError` appear **zero** times in `include/fastfields/impl/cuda/` + or `src/lib-cuda/`. Only `cudaMalloc`/`cudaMemcpy` return codes are checked. + +Put together: a kernel whose register pressure puts +`cudaFuncGetAttributes().maxThreadsPerBlock` below 1024 fails at launch with +`cudaErrorLaunchOutOfResources`, **and the failure is silently discarded** — the +kernel does not run, the output tensor keeps whatever it held, and the caller +gets no error and no exception. With no GPU in CI, nothing would ever report it. + +The regularisers are the obvious candidates: `reg_flow` and `reg_field` are the +12.98 GB and 8.11 GB nvcc compiles precisely because their 3-D bending kernels +are enormous, and register count tracks that. + +**Recommended, in priority order — none of this is a user-facing API:** + +1. **Add launch error checking.** Cheap, mechanical, and it converts a silent + wrong answer into a thrown exception. This is worth doing on its own merits + regardless of everything else in this document, and it is the only item here + I would call urgent. +2. **Replace the constant with a per-kernel choice** — `cudaOccupancyMaxPotentialBlockSize`, + or at minimum clamp to that kernel's `cudaFuncGetAttributes().maxThreadsPerBlock`. + Both faster and immune to the failure by construction. +3. **If a knob is still wanted**, make it an optional *cap* — `ff::cuda::set_max_block_size(int)`, + defaulting to auto and validated against the per-kernel attribute — never a + mirror of the CPU thread count. + +I have **no GPU to verify any of this on**, so items 1–2 rest on reading the +CUDA contract rather than on a reproduction. Flagging that explicitly: the +"1024 might be too many" claim is a well-founded suspicion, not a measured bug. +The "a failed launch is silently ignored" half **is** verified — it is a grep +result, not a judgement. + +--- + +## 4. Sequencing + +``` + #145 header guards ──┐ + #146 ├──► THE MOVE (this proposal, §1) ──► kernels-as-free-functions + #143 FF_INDEX32 ───┘ │ + #147 TU split ─── independent ─┘ +``` + +**Must land after:** #145 and #146. Both rewrite includes tree-wide; #146 alone +touches 149 files, 14 of them in `impl/kernels/`. Rebasing a file move under +them by hand is the failure mode the rewrite-script pattern exists to prevent. + +**`tools/move-core-headers.py` is written to survive that rebase**, following +the pattern set by `rename-macros.py` and `dedup-dispatch-helpers.py`: it does +not hard-code an include delimiter, it *measures* the tree's dominant one and +matches it. Run before #146 it emits `"fastfields/…"`; run after, ``. +Either way `--check` passes on its own output. **Rebase by re-running, never by +hand.** + +> **This was demonstrated, not assumed.** #146 and #143 landed on `main` while +> this proposal was being written. The rebase was performed exactly as the +> script's docstring prescribes — revert the prototype, merge `main`, re-run the +> script unedited — and it produced `` on the new base with no +> change to the script and no hand-editing: +> +> ``` +> $ python3 tools/move-core-headers.py +> moved 10 header(s) to include/fastfields/core; +> rewrote includes in 81 file(s) using +> +> $ python3 tools/move-core-headers.py --check +> clean; 10 header(s) in include/fastfields/core, +> include delimiter <...>, no dependency leak +> ``` +> +> The merge with #143 was clean: this change touches 17 files under `src/` with +> 23 line changes, all of them include lines, against #143's 19 — the same +> files, different lines, as predicted below. + +**Independent of the move (verified, not assumed):** + +- **#147** touches `src/lib-cuda/` only — 16 files, none under `include/`. Zero + overlap. Its `flow_slice::` seam is an *input* to §2.5, not a conflict. +- **#143 (`FF_INDEX32`)** touches `make/`, the Makefiles and the dispatch layer. + Its only intersection is the ~16 `src/*.cpp` whose `impl/kernels/utils.h` + include line changes; different lines in the same files. + +**CI is neutral.** `.github/workflows/ci.yml:102` already puts +`^include/fastfields/impl/kernels/` and `^include/fastfields/core/` in the *same* +"trigger EVERYTHING" clause, so nothing about the path filters changes. + +**Can land independently, in any order, right now:** + +| Item | § | Size | +| --- | --- | --- | +| Fix or delete `impl/cpu/tetrahedron.h`; add a compile-only target for orphan headers | 1.4 | small | +| `#undef FF_ATOMIC_INTEGER_IMPL`; delete unused `FF_GPU_ATOMIC_INTEGER` | 2.3 | trivial | +| Fix `core/dispatch.h`'s wrong comment about `canUse32BitIndexMath` | 2.5 | trivial | +| CUDA launch error checking | 3.4 | small, **highest value here** | + +**Must land together:** the ten-file move, its include rewrite, the one-line +`tools/test-baseline.sh` probe update (§ below), and the five live prose +references. One commit, one script, one `--check`. + +**Should wait:** everything in §2. The `vox` rename, the module/file renames and +the `_impl` convention are each a separate PR *after* the move settles. +Bundling a rename into a relocation makes both unreviewable, and §2.2's rename +is only worth its cost once the free-function refactor it enables is actually +scheduled. + +**One thing the move breaks that is easy to miss:** `tools/test-baseline.sh:238` +probes for `include/fastfields/impl/kernels/bounds.h` to verify tree layout, and +dies if it is absent. The gate tool fails *before running any test*, with a +message that looks like a broken checkout. One line. Note that +`tools/consolidate.sh` and `test-baseline.sh:249` reference the same paths and +must **not** be updated — they describe the frozen pre-consolidation layout. +Similarly `MIGRATION.md`'s five references are historical and should be left as +history. + +--- + +## 5. Where I am uncertain + +1. **`bounds.h` / `spline.h`: whole-file move versus splitting at the existing + seam (§1.3).** I recommend whole-file, but this is a judgement about how + small `core/` should be, not a technical finding. The seam is real and the + `using` bridge for it already exists, so the split stays cheap later. The + owner's taste should decide. +2. **`vox` versus `elem` (§2.2).** `vox` is convergent with `teeny`; `elem` + matches what the codebase's own prose says the layer does. I lean `vox` and + the margin is thin. This is the trade, not a verdict. +3. **`threadpool` → `core/` (§3.3)** is conditional on the thread API being + exported. If Question 4 is declined, dependency logic says `impl/cpu/`. +4. **The CUDA 1024 hazard (§3.4)** is reasoned from the CUDA contract, not + reproduced — there is no GPU here. The *silent* part is verified; the + *triggering* part is not. +5. **`vector/`'s fate (§1.4)** — I can show it is unused and duplicated, but not + whether it was meant to replace `mesh_utils.h`'s hierarchy or was abandoned. + That is history only the owner has. + +--- + +## Appendix A — how the numbers were measured + +Everything is reproducible on `85fdac7`. + +| Claim | Command | +| --- | --- | +| include sites per file | resolve every `#include` (absolute *and* relative) to a repo path, count edges | +| `bound::type` 209× in `src/` | `grep -ro 'bound::type' src \| wc -l` | +| 39 CUDA launch sites | `grep -rho '<<<' include/fastfields/impl/cuda/*.h \| wc -l` | +| 0 launch error checks | `grep -rn 'cudaGetLastError\|cudaPeekAtLastError' include/fastfields/impl/cuda src/lib-cuda` | +| `vector/` used once | `grep -rn 'kernels/vector' include src tests` | +| `tetrahedron` unreferenced | `grep -rn 'tetrahedron\|tetra' include src tests make Makefile` | +| `impl/cpu/tetrahedron.h` does not compile | `clang++ -std=c++11 -fsyntax-only -I include` on a TU that includes it | +| atomic.h global scope | brace-depth scan of the `#else` branch | +| 116 `_` identifiers / 2,943 mentions | `grep -rhoE '\b_[a-z][a-z0-9_]*\b' src/lib-c* \| sort -u \| wc -l` | +| dependency closure clean | `tools/move-core-headers.py --check` | + +**Prototype result.** `tools/move-core-headers.py` applied twice, unedited: + +| Base | Delimiter emitted | Files rewritten | +| --- | --- | ---: | +| `85fdac7` (before #146) | `"fastfields/…"` | 81 | +| `de288a9` (after #146 + #143) | `` | 81 | + +`--check` clean, idempotent and free of dependency leaks on both. The gate +result is recorded in the pull request that carries this document. diff --git a/tools/move-core-headers.py b/tools/move-core-headers.py new file mode 100755 index 0000000..18c9d41 --- /dev/null +++ b/tools/move-core-headers.py @@ -0,0 +1,231 @@ +#!/usr/bin/env python3 +""" +move-core-headers.py -- relocate the shared-infrastructure headers out of +`include/fastfields/impl/kernels/` and into `include/fastfields/core/`. + +WHAT THIS APPLIES +-------------------------------------------------------------------------- +`impl/kernels/` is meant to hold single-element math and nothing else, but it +also carries the helpers every layer above it uses: atomics, indexing, the +thread pool, the numeric utilities, the boundary/spline vocabulary. This +script moves those helpers to `core/` -- the directory that already holds +what `src/lib`, `src/lib-cpu` (host compiler) and `src/lib-cuda` (nvcc) all +share -- and rewrites every `#include` that named them. + +After it runs, `impl/kernels/` contains only per-operation implementations. + +The move is closed under dependency: every file in MOVES includes only +`core/` headers and other files in MOVES, so `core/` never acquires a +dependency on `impl/kernels/`. `--check` re-verifies that property rather +than trusting this comment. + +INCLUDE REWRITING +-------------------------------------------------------------------------- +Includes inside `impl/kernels/` are spelled relative ("utils.h", +"../bounds.h", "../../utils.h"); includes from `src/`, `tests/` and the other +`include/` subtrees are spelled absolutely ("fastfields/impl/kernels/utils.h"). +Both forms are handled by resolving every include to a repository path, +applying the move map, and re-emitting: + + * same-directory after the move -> quoted bare name ("utils.h") + * anything else -> the absolute form, in whichever + delimiter the tree already uses for + `fastfields/...` includes + +That last point is what makes this script survive the in-flight +include-delimiter sweep (PR #146, `"fastfields/..."` -> ``): +it does not hard-code a delimiter, it measures the tree's dominant one and +matches it. Run it before that sweep and it emits quotes; run it after and it +emits angle brackets. Either way `--check` passes on its own output. + +HEADER GUARDS ARE NOT TOUCHED +-------------------------------------------------------------------------- +The guard-normalisation pass (PR #145) derives a guard from the file's path +only for guards it *adds*; existing guards keep their names, and its `--check` +enforces the guard's shape (present, whole-file, `FF_`-prefixed, unique), not +its spelling. So `FF_UTILS` stays `FF_UTILS` after moving to `core/`, and the +two passes do not fight. Nothing here renames a macro. + +USAGE + python3 tools/move-core-headers.py # apply + python3 tools/move-core-headers.py --check # verify, change nothing + +Idempotent: re-running over an already-moved tree rewrites nothing. If this +lands on a base that has moved, do NOT resolve conflicts by hand -- reset, +re-run the script on the new base, and commit that. +""" + +import os +import re +import subprocess +import sys + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +KERNELS = "include/fastfields/impl/kernels" +CORE = "include/fastfields/core" + +# The shared infrastructure. Each entry is a bare filename under KERNELS that +# moves to the same filename under CORE. See the proposal for the per-file +# rationale; the short version is "used by layers above the kernels, and not +# the implementation of any one operation". +MOVES = [ + "atomic.h", # ff::anyAtomicAdd -- the CPU/CUDA accumulate primitive + "batch.h", # linear index <-> sub-index / strided-index conversion + "bounds.h", # bound::type + BoundVec vocabulary, and the bound math + "meta.h", # Pack / Tuple template metaprogramming + "parallel.h", # parallel_for + the grain-size policy + "parallel_impl.h", # its backend selection (native / OpenMP / none) + "spline.h", # spline::type + SplineVec vocabulary, and weight math + "threadpool.h", # the work-stealing pool + "threadpool.inl", # its inline definitions + the thread-count accessors + "utils.h", # numeric/type helpers, canUse32BitIndexMath +] + +SOURCE_DIRS = ("include", "src", "tests") +EXTS = (".h", ".hpp", ".inl", ".cuh", ".cpp", ".cu") + +INCLUDE_RE = re.compile(r'(#\s*include\s*)([<"])([^">]+)([">])') + + +def move_map(): + """old repo-relative path -> new repo-relative path.""" + return {f"{KERNELS}/{n}": f"{CORE}/{n}" for n in MOVES} + + +def sources(): + for d in SOURCE_DIRS: + for dirpath, _, filenames in os.walk(os.path.join(ROOT, d)): + for name in sorted(filenames): + if name.endswith(EXTS): + yield os.path.relpath(os.path.join(dirpath, name), ROOT) + + +def dominant_delimiter(): + """Whichever bracket the tree already uses for `fastfields/...` includes. + + Ties and empty trees fall back to quotes, which is what `main` uses today. + """ + angle = quote = 0 + for rel in sources(): + with open(os.path.join(ROOT, rel), encoding="utf8", errors="replace") as fh: + for m in INCLUDE_RE.finditer(fh.read()): + if m.group(3).startswith("fastfields/"): + if m.group(2) == "<": + angle += 1 + else: + quote += 1 + return ("<", ">") if angle > quote else ('"', '"') + + +def resolve(including_file, spelling): + """Repo-relative path an include names, or None if it is not ours.""" + if spelling.startswith("fastfields/"): + return "include/" + spelling + if spelling.startswith(("<", "/")) or "." not in spelling: + return None + cand = os.path.normpath(os.path.join(os.path.dirname(including_file), spelling)) + return cand if os.path.exists(os.path.join(ROOT, cand)) else None + + +def rewrite(rel, mapping, delim): + """Return the file's new text, or None if unchanged.""" + path = os.path.join(ROOT, rel) + with open(path, encoding="utf8", errors="replace") as fh: + text = fh.read() + + # Where this file itself ends up, so same-directory includes stay bare. + new_self = mapping.get(rel, rel) + + def sub(m): + pre, open_d, spelling, close_d = m.groups() + target = resolve(rel, spelling) + if target is None: + return m.group(0) + new_target = mapping.get(target, target) + if new_target == target and new_self == rel: + return m.group(0) # nothing about this edge moved + if os.path.dirname(new_target) == os.path.dirname(new_self): + return f'{pre}"{os.path.basename(new_target)}"' + if new_target.startswith("include/fastfields/"): + pub = new_target[len("include/"):] + return f"{pre}{delim[0]}{pub}{delim[1]}" + # Not under the public root: keep it relative to the new location. + newrel = os.path.relpath(new_target, os.path.dirname(new_self)) + return f'{pre}"{newrel}"' + + out = INCLUDE_RE.sub(sub, text) + return out if out != text else None + + +def check_closure(mapping): + """core/ must not end up depending on impl/kernels/.""" + problems = [] + for old, new in mapping.items(): + with open(os.path.join(ROOT, old if os.path.exists(os.path.join(ROOT, old)) else new), + encoding="utf8", errors="replace") as fh: + text = fh.read() + src = old if os.path.exists(os.path.join(ROOT, old)) else new + for m in INCLUDE_RE.finditer(text): + target = resolve(src, m.group(3)) + if target is None: + continue + target = mapping.get(target, target) + if target.startswith(KERNELS): + problems.append(f"{new} still depends on {target}") + return problems + + +def main(): + check = "--check" in sys.argv[1:] + mapping = move_map() + delim = dominant_delimiter() + + missing = [o for o in mapping if not os.path.exists(os.path.join(ROOT, o))] + done = all(os.path.exists(os.path.join(ROOT, n)) for n in mapping.values()) + + problems = check_closure(mapping) + + changed = [] + for rel in sources(): + new = rewrite(rel, mapping, delim) + if new is None: + continue + changed.append(rel) + if not check: + with open(os.path.join(ROOT, rel), "w", encoding="utf8") as fh: + fh.write(new) + + if not check: + for old, new in mapping.items(): + if os.path.exists(os.path.join(ROOT, old)): + os.makedirs(os.path.join(ROOT, os.path.dirname(new)), exist_ok=True) + subprocess.check_call(["git", "mv", old, new], cwd=ROOT) + + if check: + if problems: + for p in problems: + print("DEPENDENCY LEAK:", p) + return 1 + if changed: + print(f"{len(changed)} file(s) would change:") + for c in changed[:40]: + print(" ", c) + if len(changed) > 40: + print(f" ... and {len(changed) - 40} more") + return 1 + if missing and not done: + print("move map names files that do not exist:", missing) + return 1 + print(f"clean; {len(MOVES)} header(s) in {CORE}, " + f"include delimiter {delim[0]}...{delim[1]}, no dependency leak") + return 0 + + print(f"moved {len(MOVES)} header(s) to {CORE}; " + f"rewrote includes in {len(changed)} file(s) " + f"using {delim[0]}fastfields/...{delim[1]}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 492b4bbb869da93ef657a96649b0f7dea04466f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 12:21:02 +0000 Subject: [PATCH 2/5] PROTOTYPE (evidence, not a request to merge): apply the core/ move This commit exists so the proposal's cost figures are measurements rather than estimates. It is the unedited output of tools/move-core-headers.py run on de288a9, nothing else. Drop it and the design in the preceding commit still stands. 10 headers moved, 4,262 lines 81 files rewritten, emitting on this base --check clean, idempotent, no dependency leak core/ goes 5 -> 15 files; impl/kernels/ goes 57 -> 47 and then holds only per-operation implementations. The one-line tools/test-baseline.sh probe update rides along here because the script does not touch that file: the gate tool probes for impl/kernels/bounds.h to validate tree layout and dies before running a single test if it is absent -- with a message that reads like a broken checkout. tools/consolidate.sh and the six-repo probe in the same script name the same paths and are deliberately left alone; they describe the frozen pre-consolidation layout. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z --- include/fastfields/api/cpu/pushpull_dispatch.h | 2 +- include/fastfields/api/cuda/pushpull_dispatch.h | 2 +- include/fastfields/{impl/kernels => core}/atomic.h | 2 +- include/fastfields/{impl/kernels => core}/batch.h | 2 +- include/fastfields/{impl/kernels => core}/bounds.h | 2 +- include/fastfields/{impl/kernels => core}/meta.h | 2 +- include/fastfields/{impl/kernels => core}/parallel.h | 2 +- include/fastfields/{impl/kernels => core}/parallel_impl.h | 2 +- include/fastfields/{impl/kernels => core}/spline.h | 2 +- include/fastfields/{impl/kernels => core}/threadpool.h | 2 +- include/fastfields/{impl/kernels => core}/threadpool.inl | 2 +- include/fastfields/{impl/kernels => core}/utils.h | 2 +- include/fastfields/impl/cpu/distance_euclidean.h | 6 +++--- include/fastfields/impl/cpu/distance_l1.h | 6 +++--- include/fastfields/impl/cpu/distance_mesh.h | 4 ++-- include/fastfields/impl/cpu/distance_spline.h | 4 ++-- include/fastfields/impl/cpu/posdef.h | 4 ++-- include/fastfields/impl/cpu/pushpull.h | 6 +++--- include/fastfields/impl/cpu/reg_field.h | 8 ++++---- include/fastfields/impl/cpu/reg_flow.h | 8 ++++---- include/fastfields/impl/cpu/resize.h | 4 ++-- include/fastfields/impl/cpu/restrict.h | 4 ++-- include/fastfields/impl/cpu/solve_field.h | 6 +++--- include/fastfields/impl/cpu/splinc.h | 6 +++--- include/fastfields/impl/cuda/distance_euclidean.h | 2 +- include/fastfields/impl/cuda/distance_l1.h | 2 +- include/fastfields/impl/cuda/distance_mesh.h | 4 ++-- include/fastfields/impl/cuda/distance_spline.h | 4 ++-- include/fastfields/impl/cuda/posdef.h | 2 +- include/fastfields/impl/cuda/pushpull.h | 6 +++--- include/fastfields/impl/cuda/reg_field.h | 6 +++--- include/fastfields/impl/cuda/reg_flow.h | 6 +++--- include/fastfields/impl/cuda/resize.h | 6 +++--- include/fastfields/impl/cuda/restrict.h | 6 +++--- include/fastfields/impl/cuda/splinc.h | 4 ++-- include/fastfields/impl/kernels/distance/euclidean.h | 2 +- include/fastfields/impl/kernels/distance/l1.h | 2 +- include/fastfields/impl/kernels/distance/mesh.h | 2 +- include/fastfields/impl/kernels/distance/mesh_utils.h | 2 +- include/fastfields/impl/kernels/distance/spline.h | 6 +++--- include/fastfields/impl/kernels/posdef/cholesky.h | 2 +- include/fastfields/impl/kernels/posdef/posdef.h | 2 +- include/fastfields/impl/kernels/posdef/utils.h | 2 +- include/fastfields/impl/kernels/pushpull/1d.h | 4 ++-- include/fastfields/impl/kernels/pushpull/2d.h | 4 ++-- include/fastfields/impl/kernels/pushpull/3d.h | 4 ++-- include/fastfields/impl/kernels/pushpull/nd.h | 4 ++-- include/fastfields/impl/kernels/pushpull/utils.h | 6 +++--- include/fastfields/impl/kernels/regularisers/field/1d.h | 4 ++-- include/fastfields/impl/kernels/regularisers/field/2d.h | 4 ++-- include/fastfields/impl/kernels/regularisers/field/3d.h | 4 ++-- .../fastfields/impl/kernels/regularisers/field/utils.h | 6 +++--- include/fastfields/impl/kernels/regularisers/flow/1d.h | 4 ++-- include/fastfields/impl/kernels/regularisers/flow/2d.h | 4 ++-- include/fastfields/impl/kernels/regularisers/flow/3d.h | 4 ++-- include/fastfields/impl/kernels/regularisers/flow/utils.h | 4 ++-- include/fastfields/impl/kernels/resize.h | 8 ++++---- include/fastfields/impl/kernels/restrict.h | 6 +++--- include/fastfields/impl/kernels/splinc.h | 6 +++--- include/fastfields/impl/kernels/tetrahedron.h | 2 +- src/lib-cpu/distance.cpp | 2 +- src/lib-cpu/posdef.cpp | 2 +- src/lib-cpu/reg_field.cpp | 4 ++-- src/lib-cpu/reg_flow.cpp | 4 ++-- src/lib-cpu/resize.cpp | 2 +- src/lib-cpu/restrict.cpp | 2 +- src/lib-cpu/solve_field.cpp | 2 +- src/lib-cpu/splinc.cpp | 2 +- src/lib-cuda/distance.cpp | 2 +- src/lib-cuda/posdef.cpp | 2 +- src/lib-cuda/reg_field.cpp | 4 ++-- src/lib-cuda/reg_field_rls.cpp | 4 ++-- src/lib-cuda/reg_flow.cpp | 4 ++-- src/lib-cuda/reg_flow_rls.cpp | 4 ++-- src/lib-cuda/resize.cpp | 2 +- src/lib-cuda/restrict.cpp | 2 +- src/lib-cuda/splinc.cpp | 2 +- tests/kernels/atomic/test.cpp | 4 ++-- tests/lib-cpu/test_distance_spline.cpp | 4 ++-- tests/lib-cpu/test_reg_op.cpp | 4 ++-- tests/lib-cpu/test_splinc.cpp | 4 ++-- tools/test-baseline.sh | 2 +- 82 files changed, 150 insertions(+), 150 deletions(-) rename include/fastfields/{impl/kernels => core}/atomic.h (99%) rename include/fastfields/{impl/kernels => core}/batch.h (99%) rename include/fastfields/{impl/kernels => core}/bounds.h (99%) rename include/fastfields/{impl/kernels => core}/meta.h (98%) rename include/fastfields/{impl/kernels => core}/parallel.h (98%) rename include/fastfields/{impl/kernels => core}/parallel_impl.h (99%) rename include/fastfields/{impl/kernels => core}/spline.h (99%) rename include/fastfields/{impl/kernels => core}/threadpool.h (99%) rename include/fastfields/{impl/kernels => core}/threadpool.inl (98%) rename include/fastfields/{impl/kernels => core}/utils.h (99%) diff --git a/include/fastfields/api/cpu/pushpull_dispatch.h b/include/fastfields/api/cpu/pushpull_dispatch.h index a05e64c..d189876 100644 --- a/include/fastfields/api/cpu/pushpull_dispatch.h +++ b/include/fastfields/api/cpu/pushpull_dispatch.h @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/api/cuda/pushpull_dispatch.h b/include/fastfields/api/cuda/pushpull_dispatch.h index 23c048c..2c02e4d 100644 --- a/include/fastfields/api/cuda/pushpull_dispatch.h +++ b/include/fastfields/api/cuda/pushpull_dispatch.h @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/atomic.h b/include/fastfields/core/atomic.h similarity index 99% rename from include/fastfields/impl/kernels/atomic.h rename to include/fastfields/core/atomic.h index 4f9df1e..f2e1bdc 100755 --- a/include/fastfields/impl/kernels/atomic.h +++ b/include/fastfields/core/atomic.h @@ -5,7 +5,7 @@ #ifndef FF_ATOMIC #define FF_ATOMIC -#include +#include "cuda_switch.h" /*********************************************************************** * CPU diff --git a/include/fastfields/impl/kernels/batch.h b/include/fastfields/core/batch.h similarity index 99% rename from include/fastfields/impl/kernels/batch.h rename to include/fastfields/core/batch.h index 8cdceea..310b653 100755 --- a/include/fastfields/impl/kernels/batch.h +++ b/include/fastfields/core/batch.h @@ -21,7 +21,7 @@ */ #ifndef FF_BATCH #define FF_BATCH -#include +#include "cuda_switch.h" #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/bounds.h b/include/fastfields/core/bounds.h similarity index 99% rename from include/fastfields/impl/kernels/bounds.h rename to include/fastfields/core/bounds.h index 199f285..c0889ea 100755 --- a/include/fastfields/impl/kernels/bounds.h +++ b/include/fastfields/core/bounds.h @@ -1,6 +1,6 @@ #ifndef FF_BOUNDS #define FF_BOUNDS -#include +#include "cuda_switch.h" #include "atomic.h" #include "utils.h" #include "meta.h" diff --git a/include/fastfields/impl/kernels/meta.h b/include/fastfields/core/meta.h similarity index 98% rename from include/fastfields/impl/kernels/meta.h rename to include/fastfields/core/meta.h index dcb2705..efed1e5 100644 --- a/include/fastfields/impl/kernels/meta.h +++ b/include/fastfields/core/meta.h @@ -1,6 +1,6 @@ #ifndef FF_META #define FF_META -#include +#include "defines.h" FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(meta) diff --git a/include/fastfields/impl/kernels/parallel.h b/include/fastfields/core/parallel.h similarity index 98% rename from include/fastfields/impl/kernels/parallel.h rename to include/fastfields/core/parallel.h index e91a405..7dc311a 100755 --- a/include/fastfields/impl/kernels/parallel.h +++ b/include/fastfields/core/parallel.h @@ -5,7 +5,7 @@ #ifndef FF_PARALLEL_H #define FF_PARALLEL_H #include -#include +#include "defines.h" #include "parallel_impl.h" /* The minimum number of elements a `parallel_for` must cover before it is diff --git a/include/fastfields/impl/kernels/parallel_impl.h b/include/fastfields/core/parallel_impl.h similarity index 99% rename from include/fastfields/impl/kernels/parallel_impl.h rename to include/fastfields/core/parallel_impl.h index 2fcdb78..c22dfbf 100755 --- a/include/fastfields/impl/kernels/parallel_impl.h +++ b/include/fastfields/core/parallel_impl.h @@ -4,7 +4,7 @@ */ #ifndef FF_PARALLEL_IMPL_H #define FF_PARALLEL_IMPL_H -#include +#include "defines.h" #include #include diff --git a/include/fastfields/impl/kernels/spline.h b/include/fastfields/core/spline.h similarity index 99% rename from include/fastfields/impl/kernels/spline.h rename to include/fastfields/core/spline.h index 89d7364..ad888ef 100755 --- a/include/fastfields/impl/kernels/spline.h +++ b/include/fastfields/core/spline.h @@ -25,7 +25,7 @@ #ifndef FF_SPLINE #define FF_SPLINE -#include +#include "cuda_switch.h" #include "meta.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/threadpool.h b/include/fastfields/core/threadpool.h similarity index 99% rename from include/fastfields/impl/kernels/threadpool.h rename to include/fastfields/core/threadpool.h index 4948eeb..590da5d 100755 --- a/include/fastfields/impl/kernels/threadpool.h +++ b/include/fastfields/core/threadpool.h @@ -14,7 +14,7 @@ #include #include #include -#include +#include "defines.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/threadpool.inl b/include/fastfields/core/threadpool.inl similarity index 98% rename from include/fastfields/impl/kernels/threadpool.inl rename to include/fastfields/core/threadpool.inl index 3f3bc4a..f121628 100755 --- a/include/fastfields/impl/kernels/threadpool.inl +++ b/include/fastfields/core/threadpool.inl @@ -3,7 +3,7 @@ #include #include #include -#include +#include "defines.h" FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(internal) diff --git a/include/fastfields/impl/kernels/utils.h b/include/fastfields/core/utils.h similarity index 99% rename from include/fastfields/impl/kernels/utils.h rename to include/fastfields/core/utils.h index ab0cf7c..a1c0f76 100755 --- a/include/fastfields/impl/kernels/utils.h +++ b/include/fastfields/core/utils.h @@ -1,7 +1,7 @@ #ifndef FF_UTILS #define FF_UTILS #include -#include +#include "cuda_switch.h" #ifndef __CUDACC__ # include diff --git a/include/fastfields/impl/cpu/distance_euclidean.h b/include/fastfields/impl/cpu/distance_euclidean.h index c55f9f2..5e3aac5 100755 --- a/include/fastfields/impl/cpu/distance_euclidean.h +++ b/include/fastfields/impl/cpu/distance_euclidean.h @@ -2,9 +2,9 @@ #define FF_CPU_DISTANCE_EUCLIDEAN #include #include -#include -#include -#include +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/distance_l1.h b/include/fastfields/impl/cpu/distance_l1.h index 3a5a5b2..b950dbf 100755 --- a/include/fastfields/impl/cpu/distance_l1.h +++ b/include/fastfields/impl/cpu/distance_l1.h @@ -2,9 +2,9 @@ #define FF_CPU_DISTANCE_L1 #include #include -#include -#include -#include +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/distance_mesh.h b/include/fastfields/impl/cpu/distance_mesh.h index 7484800..e5d4318 100755 --- a/include/fastfields/impl/cpu/distance_mesh.h +++ b/include/fastfields/impl/cpu/distance_mesh.h @@ -3,8 +3,8 @@ #include #include #include -#include -#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/distance_spline.h b/include/fastfields/impl/cpu/distance_spline.h index a04e340..82e98dd 100755 --- a/include/fastfields/impl/cpu/distance_spline.h +++ b/include/fastfields/impl/cpu/distance_spline.h @@ -2,8 +2,8 @@ #define FF_CPU_DISTANCE_SPLINE #include #include -#include -#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/posdef.h b/include/fastfields/impl/cpu/posdef.h index a467164..a10b164 100755 --- a/include/fastfields/impl/cpu/posdef.h +++ b/include/fastfields/impl/cpu/posdef.h @@ -2,8 +2,8 @@ #define FF_POSDEF_CPU #include #include -#include -#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/pushpull.h b/include/fastfields/impl/cpu/pushpull.h index 89bd380..fd8dfb1 100755 --- a/include/fastfields/impl/cpu/pushpull.h +++ b/include/fastfields/impl/cpu/pushpull.h @@ -2,9 +2,9 @@ #define FF_PUSHPULL_CPU #include #include -#include -#include -#include +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/reg_field.h b/include/fastfields/impl/cpu/reg_field.h index 5990f6e..d8dfabf 100755 --- a/include/fastfields/impl/cpu/reg_field.h +++ b/include/fastfields/impl/cpu/reg_field.h @@ -2,10 +2,10 @@ #define FF_REGULARISERS_FIELD_CPU #include #include -#include -#include -#include -#include +#include +#include +#include +#include #include #include diff --git a/include/fastfields/impl/cpu/reg_flow.h b/include/fastfields/impl/cpu/reg_flow.h index f29ec0e..8901678 100755 --- a/include/fastfields/impl/cpu/reg_flow.h +++ b/include/fastfields/impl/cpu/reg_flow.h @@ -2,10 +2,10 @@ #define FF_REGULARISERS_FLOW_CPU #include #include -#include -#include -#include -#include +#include +#include +#include +#include #include #include diff --git a/include/fastfields/impl/cpu/resize.h b/include/fastfields/impl/cpu/resize.h index 4973fc1..fca8d52 100755 --- a/include/fastfields/impl/cpu/resize.h +++ b/include/fastfields/impl/cpu/resize.h @@ -2,8 +2,8 @@ #define FF_RESIZE_LOOP #include #include -#include -#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/restrict.h b/include/fastfields/impl/cpu/restrict.h index a3e9798..d00a62a 100755 --- a/include/fastfields/impl/cpu/restrict.h +++ b/include/fastfields/impl/cpu/restrict.h @@ -2,8 +2,8 @@ #define FF_RESTRICT_LOOP #include #include -#include -#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/solve_field.h b/include/fastfields/impl/cpu/solve_field.h index 2b06aa0..e9a02ce 100644 --- a/include/fastfields/impl/cpu/solve_field.h +++ b/include/fastfields/impl/cpu/solve_field.h @@ -2,9 +2,9 @@ #define FF_SOLVE_FIELD_CPU #include #include -#include -#include -#include +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cpu/splinc.h b/include/fastfields/impl/cpu/splinc.h index 759d525..16330a3 100755 --- a/include/fastfields/impl/cpu/splinc.h +++ b/include/fastfields/impl/cpu/splinc.h @@ -2,9 +2,9 @@ #define FF_SPLINC_LOOP #include #include -#include -#include -#include +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/cuda/distance_euclidean.h b/include/fastfields/impl/cuda/distance_euclidean.h index eefea71..eb79851 100755 --- a/include/fastfields/impl/cuda/distance_euclidean.h +++ b/include/fastfields/impl/cuda/distance_euclidean.h @@ -1,7 +1,7 @@ #pragma once #include #include -#include +#include #include "utils.h" #include #include diff --git a/include/fastfields/impl/cuda/distance_l1.h b/include/fastfields/impl/cuda/distance_l1.h index f2d46c6..7e5e31a 100755 --- a/include/fastfields/impl/cuda/distance_l1.h +++ b/include/fastfields/impl/cuda/distance_l1.h @@ -1,7 +1,7 @@ #pragma once #include #include -#include +#include #include "utils.h" #include #include diff --git a/include/fastfields/impl/cuda/distance_mesh.h b/include/fastfields/impl/cuda/distance_mesh.h index 04075c9..d96ecdb 100755 --- a/include/fastfields/impl/cuda/distance_mesh.h +++ b/include/fastfields/impl/cuda/distance_mesh.h @@ -1,8 +1,8 @@ #pragma once #include #include -#include -#include +#include +#include #include "utils.h" #include #include // std::unique_ptr diff --git a/include/fastfields/impl/cuda/distance_spline.h b/include/fastfields/impl/cuda/distance_spline.h index 472167d..4be1e5d 100755 --- a/include/fastfields/impl/cuda/distance_spline.h +++ b/include/fastfields/impl/cuda/distance_spline.h @@ -2,8 +2,8 @@ #include #include #include -#include -#include +#include +#include using namespace std; FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/cuda/posdef.h b/include/fastfields/impl/cuda/posdef.h index 5ca2d48..51a1e54 100755 --- a/include/fastfields/impl/cuda/posdef.h +++ b/include/fastfields/impl/cuda/posdef.h @@ -1,7 +1,7 @@ #pragma once #include #include -#include +#include #include "utils.h" #include #include diff --git a/include/fastfields/impl/cuda/pushpull.h b/include/fastfields/impl/cuda/pushpull.h index 9267001..6d96bac 100755 --- a/include/fastfields/impl/cuda/pushpull.h +++ b/include/fastfields/impl/cuda/pushpull.h @@ -1,8 +1,8 @@ #pragma once #include -#include -#include -#include +#include +#include +#include #include #include "utils.h" // allocDevice / copyToDevice / freeDevice / GET_BLOCKS #include // std::intptr_t diff --git a/include/fastfields/impl/cuda/reg_field.h b/include/fastfields/impl/cuda/reg_field.h index d59f44e..ef8fbc1 100755 --- a/include/fastfields/impl/cuda/reg_field.h +++ b/include/fastfields/impl/cuda/reg_field.h @@ -1,8 +1,8 @@ #pragma once #include -#include -#include -#include +#include +#include +#include #include #include #include "utils.h" // allocDevice / copyToDevice / freeDevice / GET_BLOCKS diff --git a/include/fastfields/impl/cuda/reg_flow.h b/include/fastfields/impl/cuda/reg_flow.h index ebe71e3..7e2ad8d 100755 --- a/include/fastfields/impl/cuda/reg_flow.h +++ b/include/fastfields/impl/cuda/reg_flow.h @@ -1,8 +1,8 @@ #pragma once #include -#include -#include -#include +#include +#include +#include #include #include #include "utils.h" // allocDevice / copyToDevice / freeDevice / GET_BLOCKS diff --git a/include/fastfields/impl/cuda/resize.h b/include/fastfields/impl/cuda/resize.h index 5be012f..eb5186a 100755 --- a/include/fastfields/impl/cuda/resize.h +++ b/include/fastfields/impl/cuda/resize.h @@ -5,9 +5,9 @@ */ #include -#include -#include -#include +#include +#include +#include #include #include "utils.h" #include diff --git a/include/fastfields/impl/cuda/restrict.h b/include/fastfields/impl/cuda/restrict.h index d43d5d3..a843900 100755 --- a/include/fastfields/impl/cuda/restrict.h +++ b/include/fastfields/impl/cuda/restrict.h @@ -6,9 +6,9 @@ */ #include -#include -#include -#include +#include +#include +#include #include #include "utils.h" #include diff --git a/include/fastfields/impl/cuda/splinc.h b/include/fastfields/impl/cuda/splinc.h index 02a99ed..b928e52 100755 --- a/include/fastfields/impl/cuda/splinc.h +++ b/include/fastfields/impl/cuda/splinc.h @@ -1,8 +1,8 @@ #pragma once #include #include -#include -#include +#include +#include #include "utils.h" #include #include diff --git a/include/fastfields/impl/kernels/distance/euclidean.h b/include/fastfields/impl/kernels/distance/euclidean.h index b5a6127..aba3e52 100755 --- a/include/fastfields/impl/kernels/distance/euclidean.h +++ b/include/fastfields/impl/kernels/distance/euclidean.h @@ -8,7 +8,7 @@ #ifndef FF_DISTANCE_E #define FF_DISTANCE_E #include -#include "../utils.h" +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/distance/l1.h b/include/fastfields/impl/kernels/distance/l1.h index f80459d..bd9dab0 100755 --- a/include/fastfields/impl/kernels/distance/l1.h +++ b/include/fastfields/impl/kernels/distance/l1.h @@ -5,7 +5,7 @@ #ifndef FF_DISTANCE_L1 #define FF_DISTANCE_L1 #include -#include "../utils.h" +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/distance/mesh.h b/include/fastfields/impl/kernels/distance/mesh.h index 0737766..a83ccdd 100755 --- a/include/fastfields/impl/kernels/distance/mesh.h +++ b/include/fastfields/impl/kernels/distance/mesh.h @@ -5,7 +5,7 @@ #ifndef FF_DISTANCE_MESH_H #define FF_DISTANCE_MESH_H #include -#include "../utils.h" +#include #include "mesh_utils.h" #include diff --git a/include/fastfields/impl/kernels/distance/mesh_utils.h b/include/fastfields/impl/kernels/distance/mesh_utils.h index c67aaa3..3d914f6 100755 --- a/include/fastfields/impl/kernels/distance/mesh_utils.h +++ b/include/fastfields/impl/kernels/distance/mesh_utils.h @@ -1,7 +1,7 @@ #ifndef FF_DISTANCE_MESH_UTILS_H #define FF_DISTANCE_MESH_UTILS_H #include -#include "../utils.h" +#include // ============================================================================= // diff --git a/include/fastfields/impl/kernels/distance/spline.h b/include/fastfields/impl/kernels/distance/spline.h index df9b174..df9dd74 100755 --- a/include/fastfields/impl/kernels/distance/spline.h +++ b/include/fastfields/impl/kernels/distance/spline.h @@ -1,10 +1,10 @@ #ifndef FF_DISTANCE_SPLINE_H #define FF_DISTANCE_SPLINE_H #include -#include "../spline.h" -#include "../bounds.h" +#include +#include #include "../pushpull.h" -#include "../utils.h" +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/posdef/cholesky.h b/include/fastfields/impl/kernels/posdef/cholesky.h index f3a7eff..fc50655 100755 --- a/include/fastfields/impl/kernels/posdef/cholesky.h +++ b/include/fastfields/impl/kernels/posdef/cholesky.h @@ -1,7 +1,7 @@ #ifndef FF_POSDEF_CHOLESKY #define FF_POSDEF_CHOLESKY #include -#include "../utils.h" +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/posdef/posdef.h b/include/fastfields/impl/kernels/posdef/posdef.h index 62e0dfa..6626e1f 100755 --- a/include/fastfields/impl/kernels/posdef/posdef.h +++ b/include/fastfields/impl/kernels/posdef/posdef.h @@ -1,7 +1,7 @@ #ifndef FF_POSDEF #define FF_POSDEF #include -#include "../utils.h" +#include #include "utils.h" #include "cholesky.h" #include diff --git a/include/fastfields/impl/kernels/posdef/utils.h b/include/fastfields/impl/kernels/posdef/utils.h index 0788372..5bde94f 100755 --- a/include/fastfields/impl/kernels/posdef/utils.h +++ b/include/fastfields/impl/kernels/posdef/utils.h @@ -1,7 +1,7 @@ #ifndef FF_POSDEF_UTILS #define FF_POSDEF_UTILS #include -#include "../utils.h" +#include #define FF_ONE_PLUS_TINY 1.000001 #define FF_UNUSED __attribute__((unused)) diff --git a/include/fastfields/impl/kernels/pushpull/1d.h b/include/fastfields/impl/kernels/pushpull/1d.h index e4d7e6f..86fb8ba 100755 --- a/include/fastfields/impl/kernels/pushpull/1d.h +++ b/include/fastfields/impl/kernels/pushpull/1d.h @@ -6,8 +6,8 @@ #ifndef FF_PUSHPULL_1D #define FF_PUSHPULL_1D #include -#include "../spline.h" -#include "../bounds.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/pushpull/2d.h b/include/fastfields/impl/kernels/pushpull/2d.h index a9e81a7..dad72de 100755 --- a/include/fastfields/impl/kernels/pushpull/2d.h +++ b/include/fastfields/impl/kernels/pushpull/2d.h @@ -6,8 +6,8 @@ #ifndef FF_PUSHPULL_2D #define FF_PUSHPULL_2D #include -#include "../spline.h" -#include "../bounds.h" +#include +#include #include "utils.h" // TODO: quadratic and cubic specializations diff --git a/include/fastfields/impl/kernels/pushpull/3d.h b/include/fastfields/impl/kernels/pushpull/3d.h index 9b01dfe..45cd8ee 100755 --- a/include/fastfields/impl/kernels/pushpull/3d.h +++ b/include/fastfields/impl/kernels/pushpull/3d.h @@ -6,8 +6,8 @@ #ifndef FF_PUSHPULL_3D #define FF_PUSHPULL_3D #include -#include "../spline.h" -#include "../bounds.h" +#include +#include #include "utils.h" // TODO: quadratic and cubic specializations diff --git a/include/fastfields/impl/kernels/pushpull/nd.h b/include/fastfields/impl/kernels/pushpull/nd.h index 03df18a..3fec590 100755 --- a/include/fastfields/impl/kernels/pushpull/nd.h +++ b/include/fastfields/impl/kernels/pushpull/nd.h @@ -6,8 +6,8 @@ #ifndef FF_PUSHPULL_ND #define FF_PUSHPULL_ND #include -#include "../spline.h" -#include "../bounds.h" +#include +#include #include "utils.h" // TODO + FIXME diff --git a/include/fastfields/impl/kernels/pushpull/utils.h b/include/fastfields/impl/kernels/pushpull/utils.h index 6d3e67c..08dbc4b 100755 --- a/include/fastfields/impl/kernels/pushpull/utils.h +++ b/include/fastfields/impl/kernels/pushpull/utils.h @@ -1,9 +1,9 @@ #ifndef FF_PUSHPULL_UTILS #define FF_PUSHPULL_UTILS #include -#include "../spline.h" -#include "../bounds.h" -#include "../meta.h" +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/regularisers/field/1d.h b/include/fastfields/impl/kernels/regularisers/field/1d.h index 625e419..bce1f1d 100755 --- a/include/fastfields/impl/kernels/regularisers/field/1d.h +++ b/include/fastfields/impl/kernels/regularisers/field/1d.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FIELD_1D #define FF_REGULARISERS_FIELD_1D #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/regularisers/field/2d.h b/include/fastfields/impl/kernels/regularisers/field/2d.h index 1f307ea..53cdddb 100755 --- a/include/fastfields/impl/kernels/regularisers/field/2d.h +++ b/include/fastfields/impl/kernels/regularisers/field/2d.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FIELD_2D #define FF_REGULARISERS_FIELD_2D #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/regularisers/field/3d.h b/include/fastfields/impl/kernels/regularisers/field/3d.h index 083a44d..85136d2 100755 --- a/include/fastfields/impl/kernels/regularisers/field/3d.h +++ b/include/fastfields/impl/kernels/regularisers/field/3d.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FIELD_3D #define FF_REGULARISERS_FIELD_3D #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/regularisers/field/utils.h b/include/fastfields/impl/kernels/regularisers/field/utils.h index 14e7c2a..02c5ee5 100755 --- a/include/fastfields/impl/kernels/regularisers/field/utils.h +++ b/include/fastfields/impl/kernels/regularisers/field/utils.h @@ -1,9 +1,9 @@ #ifndef FF_REGULARISERS_UTILS #define FF_REGULARISERS_UTILS #include -#include "../../bounds.h" -#include "../../utils.h" -#include "../../meta.h" +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/regularisers/flow/1d.h b/include/fastfields/impl/kernels/regularisers/flow/1d.h index 388b988..78742a3 100755 --- a/include/fastfields/impl/kernels/regularisers/flow/1d.h +++ b/include/fastfields/impl/kernels/regularisers/flow/1d.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FLOW_1D #define FF_REGULARISERS_FLOW_1D #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/regularisers/flow/2d.h b/include/fastfields/impl/kernels/regularisers/flow/2d.h index 1af7a05..8710555 100755 --- a/include/fastfields/impl/kernels/regularisers/flow/2d.h +++ b/include/fastfields/impl/kernels/regularisers/flow/2d.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FLOW_2D #define FF_REGULARISERS_FLOW_2D #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/regularisers/flow/3d.h b/include/fastfields/impl/kernels/regularisers/flow/3d.h index 9fc6159..ee7bf3d 100755 --- a/include/fastfields/impl/kernels/regularisers/flow/3d.h +++ b/include/fastfields/impl/kernels/regularisers/flow/3d.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FLOW_3D #define FF_REGULARISERS_FLOW_3D #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/regularisers/flow/utils.h b/include/fastfields/impl/kernels/regularisers/flow/utils.h index 394b858..acff784 100755 --- a/include/fastfields/impl/kernels/regularisers/flow/utils.h +++ b/include/fastfields/impl/kernels/regularisers/flow/utils.h @@ -1,8 +1,8 @@ #ifndef FF_REGULARISERS_FLOW_UTILS #define FF_REGULARISERS_FLOW_UTILS #include -#include "../../bounds.h" -#include "../../utils.h" +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/resize.h b/include/fastfields/impl/kernels/resize.h index 6162df7..2fb5982 100755 --- a/include/fastfields/impl/kernels/resize.h +++ b/include/fastfields/impl/kernels/resize.h @@ -1,10 +1,10 @@ #ifndef FF_RESIZE #define FF_RESIZE #include -#include "spline.h" -#include "bounds.h" -#include "utils.h" // prod -#include "batch.h" // index2sub +#include +#include +#include // prod +#include // index2sub FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/impl/kernels/restrict.h b/include/fastfields/impl/kernels/restrict.h index 10d6c49..a841707 100755 --- a/include/fastfields/impl/kernels/restrict.h +++ b/include/fastfields/impl/kernels/restrict.h @@ -1,9 +1,9 @@ #ifndef FF_RESTRICT #define FF_RESTRICT #include -#include "spline.h" -#include "bounds.h" -#include "batch.h" // index2sub +#include +#include +#include // index2sub FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/splinc.h b/include/fastfields/impl/kernels/splinc.h index b65e901..05d7139 100755 --- a/include/fastfields/impl/kernels/splinc.h +++ b/include/fastfields/impl/kernels/splinc.h @@ -27,9 +27,9 @@ #ifndef FF_SPLINC #define FF_SPLINC #include -#include "spline.h" -#include "bounds.h" -#include "utils.h" +#include +#include +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(FF_DEVICE) diff --git a/include/fastfields/impl/kernels/tetrahedron.h b/include/fastfields/impl/kernels/tetrahedron.h index ffd8f12..3557501 100755 --- a/include/fastfields/impl/kernels/tetrahedron.h +++ b/include/fastfields/impl/kernels/tetrahedron.h @@ -1,7 +1,7 @@ #ifndef FF_TETRAHEDRON #define FF_TETRAHEDRON #include -#include "utils.h" +#include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/distance.cpp b/src/lib-cpu/distance.cpp index 45ca949..ac14f78 100644 --- a/src/lib-cpu/distance.cpp +++ b/src/lib-cpu/distance.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/lib-cpu/posdef.cpp b/src/lib-cpu/posdef.cpp index 8ac7b36..0bf2e24 100644 --- a/src/lib-cpu/posdef.cpp +++ b/src/lib-cpu/posdef.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/reg_field.cpp b/src/lib-cpu/reg_field.cpp index 457041a..527a6e3 100644 --- a/src/lib-cpu/reg_field.cpp +++ b/src/lib-cpu/reg_field.cpp @@ -8,8 +8,8 @@ #include #include #include -#include -#include +#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/reg_flow.cpp b/src/lib-cpu/reg_flow.cpp index 7cb2163..9665d84 100644 --- a/src/lib-cpu/reg_flow.cpp +++ b/src/lib-cpu/reg_flow.cpp @@ -8,8 +8,8 @@ #include #include #include -#include -#include +#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/resize.cpp b/src/lib-cpu/resize.cpp index 9bc6428..120c013 100644 --- a/src/lib-cpu/resize.cpp +++ b/src/lib-cpu/resize.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/restrict.cpp b/src/lib-cpu/restrict.cpp index f9191db..fa344e1 100644 --- a/src/lib-cpu/restrict.cpp +++ b/src/lib-cpu/restrict.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/solve_field.cpp b/src/lib-cpu/solve_field.cpp index ba309c8..f2f6862 100644 --- a/src/lib-cpu/solve_field.cpp +++ b/src/lib-cpu/solve_field.cpp @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cpu/splinc.cpp b/src/lib-cpu/splinc.cpp index acda3d5..13537a4 100644 --- a/src/lib-cpu/splinc.cpp +++ b/src/lib-cpu/splinc.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/distance.cpp b/src/lib-cuda/distance.cpp index 299da51..6b80bca 100644 --- a/src/lib-cuda/distance.cpp +++ b/src/lib-cuda/distance.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/lib-cuda/posdef.cpp b/src/lib-cuda/posdef.cpp index 4596c94..922ce16 100644 --- a/src/lib-cuda/posdef.cpp +++ b/src/lib-cuda/posdef.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/reg_field.cpp b/src/lib-cuda/reg_field.cpp index f7f0bbd..f5ffbc0 100644 --- a/src/lib-cuda/reg_field.cpp +++ b/src/lib-cuda/reg_field.cpp @@ -9,8 +9,8 @@ #include #include #include -#include -#include +#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/reg_field_rls.cpp b/src/lib-cuda/reg_field_rls.cpp index 73c7468..06a10a4 100644 --- a/src/lib-cuda/reg_field_rls.cpp +++ b/src/lib-cuda/reg_field_rls.cpp @@ -9,8 +9,8 @@ #include #include #include -#include -#include +#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/reg_flow.cpp b/src/lib-cuda/reg_flow.cpp index 1f4bdba..6bc15c3 100644 --- a/src/lib-cuda/reg_flow.cpp +++ b/src/lib-cuda/reg_flow.cpp @@ -8,8 +8,8 @@ #include #include #include -#include -#include +#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/reg_flow_rls.cpp b/src/lib-cuda/reg_flow_rls.cpp index a32238b..664f7e3 100644 --- a/src/lib-cuda/reg_flow_rls.cpp +++ b/src/lib-cuda/reg_flow_rls.cpp @@ -8,8 +8,8 @@ #include #include #include -#include -#include +#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/resize.cpp b/src/lib-cuda/resize.cpp index f0073ec..fcc345e 100644 --- a/src/lib-cuda/resize.cpp +++ b/src/lib-cuda/resize.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/restrict.cpp b/src/lib-cuda/restrict.cpp index bedaf49..33f1c82 100644 --- a/src/lib-cuda/restrict.cpp +++ b/src/lib-cuda/restrict.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/src/lib-cuda/splinc.cpp b/src/lib-cuda/splinc.cpp index e96e443..2b1b2ae 100644 --- a/src/lib-cuda/splinc.cpp +++ b/src/lib-cuda/splinc.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/tests/kernels/atomic/test.cpp b/tests/kernels/atomic/test.cpp index 539f498..21c552d 100644 --- a/tests/kernels/atomic/test.cpp +++ b/tests/kernels/atomic/test.cpp @@ -26,8 +26,8 @@ * takes milliseconds; it is built by the tsan CI leg, where the threaded case * below is the interesting one. */ -#include -#include +#include +#include #include #include #include diff --git a/tests/lib-cpu/test_distance_spline.cpp b/tests/lib-cpu/test_distance_spline.cpp index 7e71a6f..a4a33a6 100644 --- a/tests/lib-cpu/test_distance_spline.cpp +++ b/tests/lib-cpu/test_distance_spline.cpp @@ -32,8 +32,8 @@ #include #include #include -#include -#include +#include +#include // Spline weight / boundary helpers reused to build an independent reference for // the higher-order (Cubic) spline-distance path (B5). diff --git a/tests/lib-cpu/test_reg_op.cpp b/tests/lib-cpu/test_reg_op.cpp index f358948..5b993c0 100644 --- a/tests/lib-cpu/test_reg_op.cpp +++ b/tests/lib-cpu/test_reg_op.cpp @@ -22,8 +22,8 @@ #include #include -#include -#include +#include +#include #include #include diff --git a/tests/lib-cpu/test_splinc.cpp b/tests/lib-cpu/test_splinc.cpp index f486316..e4a547f 100644 --- a/tests/lib-cpu/test_splinc.cpp +++ b/tests/lib-cpu/test_splinc.cpp @@ -16,8 +16,8 @@ #include #include #include -#include -#include +#include +#include using ff::cpu::spline::type; using btype = ff::cpu::bound::type; diff --git a/tools/test-baseline.sh b/tools/test-baseline.sh index 1feac38..913bd51 100755 --- a/tools/test-baseline.sh +++ b/tools/test-baseline.sh @@ -235,7 +235,7 @@ if [ -n "$CONSOLIDATED" ]; then # fails in a way that looks like a source bug). Check a file from each # layer -- for the consolidated tree that is the -I root, not a symlink. PROBES="include/fastfields/impl/cpu/pushpull.h - include/fastfields/impl/kernels/bounds.h + include/fastfields/core/bounds.h include/fastfields/impl/kernels/restrict.h include/fastfields/core/cuda_switch.h" for probe in $PROBES; do From f982e6cf95914076f7725cccb260a73d40392462 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 12:32:52 +0000 Subject: [PATCH 3/5] refine the move script so the ten headers are pure renames, and record why CI found something no amount of reading would have: the `lint (clang-format, changed lines)` job fails on any file move, because git-clang-format does not honour rename detection. It sees include/fastfields/core/bounds.h as a path absent from the merge base, treats the whole file as changed, and demands all 800 lines conform -- for files that predate the gate and have never been clang-formatted. Measured on the prototype: 4,232 of the 4,236 reformat lines it asks for are that artifact. Only 4 are attributable to anything this change does. Two script fixes, which shrink the diff and are right on their own merits: * Leave any include alone whose existing spelling still resolves from the new location. parallel.h's "parallel_impl.h" needs no edit when both files move together. * Spell everything else absolutely rather than as a bare sibling name. core/ already refers to its own siblings that way (core/dispatch.h -> ), so the moved files should adopt the convention of the directory they land in rather than import impl/kernels/'s relative style along with them. This also stops the move from fighting #146, which made the spelling for the public interface. With both, all ten moved files are byte-identical pure renames (verified by checksum against their originals) and every include edit falls on the 71 files that did not move: 82 files, 140 insertions / 140 deletions, down from 150/150. That does NOT fix the lint -- a zero-content-change rename is flagged just as hard, which is the measurement above. The verified mitigation is to clang-format the ten files in place, in impl/kernels/, as its own PR before the move; formatting a file drops it off the linter's list entirely. That is now written up in the proposal's sequencing section as the item most likely to change how this work is scheduled, with the alternatives (teach the lint to skip R100, or exempt the one PR) and the argument for paying the debt rather than deferring it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z --- docs/proposals/core-and-naming.md | 61 ++++++++++++++++++++++++- include/fastfields/core/atomic.h | 2 +- include/fastfields/core/batch.h | 2 +- include/fastfields/core/bounds.h | 2 +- include/fastfields/core/meta.h | 2 +- include/fastfields/core/parallel.h | 2 +- include/fastfields/core/parallel_impl.h | 2 +- include/fastfields/core/spline.h | 2 +- include/fastfields/core/threadpool.h | 2 +- include/fastfields/core/threadpool.inl | 2 +- include/fastfields/core/utils.h | 2 +- tools/move-core-headers.py | 27 +++++++++-- 12 files changed, 92 insertions(+), 16 deletions(-) diff --git a/docs/proposals/core-and-naming.md b/docs/proposals/core-and-naming.md index 1f9164b..0f0ad60 100644 --- a/docs/proposals/core-and-naming.md +++ b/docs/proposals/core-and-naming.md @@ -769,6 +769,57 @@ must **not** be updated — they describe the frozen pre-consolidation layout. Similarly `MIGRATION.md`'s five references are historical and should be left as history. +### 4.1 The blocker nobody would predict: `clang-format` versus a rename + +**`lint (clang-format, changed lines)` fails on any file move, and the fix has +to be scheduled before the move rather than inside it.** + +This was found by CI, not by reading. The job runs + +``` +git-clang-format-18 --diff --extensions h,hpp,inl,cpp,cu,cuh $(git merge-base origin/main HEAD) +``` + +and **`git-clang-format` does not honour rename detection**. It sees +`include/fastfields/core/bounds.h` as a path that did not exist at the merge +base, treats the entire file as changed, and demands that all 800 lines conform +— for a file that has never been clang-formatted, because it predates the gate. + +Measured on the prototype: + +| | reformat lines demanded | +| --- | ---: | +| the 10 moved files | **4,232** | +| `impl/kernels/resize.h` (a genuinely changed include line) | 4 | +| everything else | 0 | + +So **4,232 of the 4,236 lines the linter asks for are an artifact of the rename**, +not of anything this change does. And it is not avoidable by making the move +cleaner: the figure above is measured with all ten files as *pure renames*, +byte-identical to their originals. A zero-content-change rename is flagged just +as hard as a modified one. + +**Recommended mitigation: clang-format the ten files in place, in +`impl/kernels/`, as its own PR, before the move.** Verified — formatting a file +drops it off the linter's list entirely, so the move that follows is lint-clean. +That PR is itself trivially green (the files are not renamed there, so only the +lines it changes are checked, and it changes them to be clean). + +The cost is real and should be stated: it reformats ~4,200 lines of `bounds.h`, +`spline.h`, `utils.h` and friends, churning blame across them. That is pre- +existing debt the gate has simply not billed yet — every one of these files is +one rename away from being billed for it regardless of this proposal. Doing it +deliberately, in a commit whose *only* content is formatting, is much better than +having it land inside a relocation where it would hide the twelve include lines +that actually matter. + +Two alternatives, if that churn is unwanted: teach the lint to skip files whose +diff status is `R100`, or grant this one PR an exemption. Both are CI changes +rather than source changes, and both leave the debt in place. I lean towards +paying it, because the pre-format PR is reviewable in a way the combined one is +not — but this is the owner's call, and it is the single item most likely to +change how this work is scheduled. + --- ## 5. Where I am uncertain @@ -814,7 +865,15 @@ Everything is reproducible on `85fdac7`. | Base | Delimiter emitted | Files rewritten | | --- | --- | ---: | | `85fdac7` (before #146) | `"fastfields/…"` | 81 | -| `de288a9` (after #146 + #143) | `` | 81 | +| `de288a9` (after #146 + #143) | `` | 71 | `--check` clean, idempotent and free of dependency leaks on both. The gate result is recorded in the pull request that carries this document. + +The second figure is lower because the script leaves any include alone whose +existing spelling still resolves from the new location. Two files that move +together keep their relative position, so `parallel.h`'s `"parallel_impl.h"` +needs no edit — and with that rule **all ten moved files are pure renames**, +byte-identical to their originals, with every include edit falling on the 71 +files that did *not* move. That is worth more than a smaller diff: see §4.1 +for what a modified-and-renamed file costs under the clang-format gate. diff --git a/include/fastfields/core/atomic.h b/include/fastfields/core/atomic.h index f2e1bdc..4f9df1e 100755 --- a/include/fastfields/core/atomic.h +++ b/include/fastfields/core/atomic.h @@ -5,7 +5,7 @@ #ifndef FF_ATOMIC #define FF_ATOMIC -#include "cuda_switch.h" +#include /*********************************************************************** * CPU diff --git a/include/fastfields/core/batch.h b/include/fastfields/core/batch.h index 310b653..8cdceea 100755 --- a/include/fastfields/core/batch.h +++ b/include/fastfields/core/batch.h @@ -21,7 +21,7 @@ */ #ifndef FF_BATCH #define FF_BATCH -#include "cuda_switch.h" +#include #include "utils.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/core/bounds.h b/include/fastfields/core/bounds.h index c0889ea..199f285 100755 --- a/include/fastfields/core/bounds.h +++ b/include/fastfields/core/bounds.h @@ -1,6 +1,6 @@ #ifndef FF_BOUNDS #define FF_BOUNDS -#include "cuda_switch.h" +#include #include "atomic.h" #include "utils.h" #include "meta.h" diff --git a/include/fastfields/core/meta.h b/include/fastfields/core/meta.h index efed1e5..dcb2705 100644 --- a/include/fastfields/core/meta.h +++ b/include/fastfields/core/meta.h @@ -1,6 +1,6 @@ #ifndef FF_META #define FF_META -#include "defines.h" +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(meta) diff --git a/include/fastfields/core/parallel.h b/include/fastfields/core/parallel.h index 7dc311a..e91a405 100755 --- a/include/fastfields/core/parallel.h +++ b/include/fastfields/core/parallel.h @@ -5,7 +5,7 @@ #ifndef FF_PARALLEL_H #define FF_PARALLEL_H #include -#include "defines.h" +#include #include "parallel_impl.h" /* The minimum number of elements a `parallel_for` must cover before it is diff --git a/include/fastfields/core/parallel_impl.h b/include/fastfields/core/parallel_impl.h index c22dfbf..2fcdb78 100755 --- a/include/fastfields/core/parallel_impl.h +++ b/include/fastfields/core/parallel_impl.h @@ -4,7 +4,7 @@ */ #ifndef FF_PARALLEL_IMPL_H #define FF_PARALLEL_IMPL_H -#include "defines.h" +#include #include #include diff --git a/include/fastfields/core/spline.h b/include/fastfields/core/spline.h index ad888ef..89d7364 100755 --- a/include/fastfields/core/spline.h +++ b/include/fastfields/core/spline.h @@ -25,7 +25,7 @@ #ifndef FF_SPLINE #define FF_SPLINE -#include "cuda_switch.h" +#include #include "meta.h" FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/core/threadpool.h b/include/fastfields/core/threadpool.h index 590da5d..4948eeb 100755 --- a/include/fastfields/core/threadpool.h +++ b/include/fastfields/core/threadpool.h @@ -14,7 +14,7 @@ #include #include #include -#include "defines.h" +#include FF_NAMESPACE_BEGIN(FF_NS) diff --git a/include/fastfields/core/threadpool.inl b/include/fastfields/core/threadpool.inl index f121628..3f3bc4a 100755 --- a/include/fastfields/core/threadpool.inl +++ b/include/fastfields/core/threadpool.inl @@ -3,7 +3,7 @@ #include #include #include -#include "defines.h" +#include FF_NAMESPACE_BEGIN(FF_NS) FF_NAMESPACE_BEGIN(internal) diff --git a/include/fastfields/core/utils.h b/include/fastfields/core/utils.h index a1c0f76..ab0cf7c 100755 --- a/include/fastfields/core/utils.h +++ b/include/fastfields/core/utils.h @@ -1,7 +1,7 @@ #ifndef FF_UTILS #define FF_UTILS #include -#include "cuda_switch.h" +#include #ifndef __CUDACC__ # include diff --git a/tools/move-core-headers.py b/tools/move-core-headers.py index 18c9d41..bdef08f 100755 --- a/tools/move-core-headers.py +++ b/tools/move-core-headers.py @@ -143,14 +143,31 @@ def sub(m): if target is None: return m.group(0) new_target = mapping.get(target, target) - if new_target == target and new_self == rel: - return m.group(0) # nothing about this edge moved - if os.path.dirname(new_target) == os.path.dirname(new_self): - return f'{pre}"{os.path.basename(new_target)}"' + + # Leave any edge alone whose EXISTING spelling still resolves to the + # right file from the new location. Two files that move together keep + # their relative position, so `parallel.h`'s `"parallel_impl.h"` needs + # no edit at all -- and not editing it is what keeps the moved files + # pure renames rather than modified files. That matters for more than + # tidiness: `git-clang-format --diff` treats a renamed-and-modified + # file as wholly new and demands a whole-file reformat, so a stray + # one-line rewrite here costs hundreds of lines of unrelated churn. + if not spelling.startswith("fastfields/"): + still = os.path.normpath( + os.path.join(os.path.dirname(new_self), spelling)) + if still == new_target: + return m.group(0) + elif new_target == target: + return m.group(0) + + # Otherwise spell it absolutely. Every destination here is under the + # public root, and `core/` already refers to its own siblings that way + # (`core/dispatch.h` -> ), so this matches + # the convention of the directory the files land in rather than + # importing `impl/kernels/`'s relative style along with them. if new_target.startswith("include/fastfields/"): pub = new_target[len("include/"):] return f"{pre}{delim[0]}{pub}{delim[1]}" - # Not under the public root: keep it relative to the new location. newrel = os.path.relpath(new_target, os.path.dirname(new_self)) return f'{pre}"{newrel}"' From 20799b95c1781677beb2c21d09355e255fb60404 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 12:35:06 +0000 Subject: [PATCH 4/5] docs: note that some figures were re-measured after the mid-review rebase Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z --- docs/proposals/core-and-naming.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/proposals/core-and-naming.md b/docs/proposals/core-and-naming.md index 0f0ad60..0baadf7 100644 --- a/docs/proposals/core-and-naming.md +++ b/docs/proposals/core-and-naming.md @@ -6,8 +6,10 @@ exists to turn cost estimates into measurements; it is not the migration. Two questions were asked together because they are the same question twice: *what is this layer for, and does its name say so?* -Every number below is measured on `85fdac7`, not estimated. The commands are -in [Appendix A](#appendix-a--how-the-numbers-were-measured). +Every number below is measured, not estimated — on `85fdac7` where the survey +was done, and re-measured on `de288a9` (after #146 and #143 landed mid-review) +wherever the rebase moved it. The commands are in +[Appendix A](#appendix-a--how-the-numbers-were-measured). --- From 11113c8a4bc342ace075890e14b56ab053fdd36e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 12:35:55 +0000 Subject: [PATCH 5/5] docs: correct the summary -- spline.h was on the owner's list all along The Move row claimed spline.h was an addition to the owner's eight. It was not; the eight were atomic/batch/meta/parallel/bounds/spline/threadpool/utils and spline is among them. The two genuine additions are parallel_impl.h and threadpool.inl, which are the .inl/impl partners of files already on the list and cannot be separated from them. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z --- docs/proposals/core-and-naming.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/proposals/core-and-naming.md b/docs/proposals/core-and-naming.md index 0baadf7..c18ca0e 100644 --- a/docs/proposals/core-and-naming.md +++ b/docs/proposals/core-and-naming.md @@ -18,7 +18,7 @@ wherever the rebase moved it. The commands are in | | Recommendation | | --- | --- | | **`core/` means** | everything more than one layer depends on that is not itself the maths of a named operation — device-specialised or not | -| **Move** | the owner's eight, plus `spline.h`; `bounds.h` and `spline.h` move whole | +| **Move** | all eight of the owner's, plus the two pair-partners they imply (`parallel_impl.h`, `threadpool.inl`) | | **Do not move** | `vector/` (superseded, unused), `tetrahedron.h` (dead), `splinc.h` (an operation) | | **Namespaces** | add `vox` *inside* each module: `ff::::::vox` | | **`atomic.h`** | real bug, not just inconsistency — fix during the move | @@ -74,7 +74,7 @@ dependency on `impl/kernels/`. That is checked mechanically, not asserted — | --- | ---: | --- | --- | | `utils.h` | 701 | `ff::` | Numeric/type helpers with zero domain knowledge. **52 include sites, 17 of them in `src/`** — the dispatch layer already reaches into the kernels layer for it. | | `bounds.h` | 800 | `ff::bound` **and** `ff::::bound` | `bound::type` is used **209 times across 18 files in `src/`**. Also carries the `FF_STATIC_BOUND_*` / `FF_BOUND_SEL` build-policy macros — the definition of "shared by both dispatch layers". | -| `spline.h` | 1447 | `ff::spline` **and** `ff::::spline` | Same shape as `bounds.h`: `spline::type` used 23× in `src/`. Not on the owner's list as stated, but it should be — see §1.3. | +| `spline.h` | 1447 | `ff::spline` **and** `ff::::spline` | Same shape as `bounds.h`: `spline::type` used 23× in `src/`. On the owner's list, and it belongs there — but see §1.3 for why the near-identical `splinc.h` must not follow it. | | `batch.h` | 273 | `ff::` | Linear-index ↔ sub-index conversion. Used by **12 files in `impl/cpu` and 11 in `impl/cuda`** — genuinely cross-backend. | | `atomic.h` | 366 | `ff::` / **global** | The accumulate primitive. Device-specialised by construction, which the new definition admits. Carries a real bug — §2.2. | | `meta.h` | 43 | `ff::meta` | `Pack` / `Tuple` metaprogramming. Pure infrastructure. |