Uh oh!
There was an error while loading. Please reload this page.
chore: delete the unused impl/kernels/vector/ headers - #153
Merged
Conversation
2,389 lines across 11 headers, plus the 25-line scratch program that was their only consumer and the Makefile target that built it. Nothing in the library has ever included them. Why they can go --------------------------------------------------------------------------- The decisive check is the preprocessor's own dependency graph, not grep: `clang++ -MM` over all 46 translation units the build compiles -- 34 host (src/lib, src/lib-cpu, tests/lib, tests/lib-cpu) and 12 under nvcc's flags (src/lib-cuda, tests/impl-cuda) -- resolves zero of the 11 headers, directly or transitively. Running the same scan against tests/kernels/vector/test.cpp does list them, so the method detects a real reference rather than silently finding nothing. Grepping for the type names is not conclusive on its own and is worth saying so: `vector/` declares `type`, `value`, `pointer`, `iterator`, `Vector`, `Impl`, `final_type` and `this_type`, all of which occur in unrelated code -- `reg_flow.h` has its own `Impl`, `posdef.h` its own `guess_type`, `mesh_utils.h` its own `final_type`/`this_type`. In a header-only library the only way to reach a declaration is to include the header that carries it, so the include closure is the question that actually settles it. The scratch program was never part of the gate. `Makefile` described it as "a hand-run scratch program ... compiled (not run)", it appears nowhere in `tools/test-baseline.expected`, and it is absent from every CI job. What is lost --------------------------------------------------------------------------- Recoverable at de288a9, the last commit where the files exist: git show de288a9:include/fastfields/impl/kernels/vector/abstract_vector.h Worth knowing before anyone goes looking: * The code was imported wholesale in 7da9e7e ("Copy all lib files from jitfields", 2025-07-18), given a mechanical CUDA-annotation pass the same day in 865d1c5, and never functionally touched again. The only three commits since are tree-wide sweeps -- the consolidation path rewrite, the FF_ macro prefixing, and #146's include spelling. Thirteen months, zero functional changes. * The one idea here with no equivalent anywhere in the surviving tree is `SwitchStride<T, S, D>` with its `DynamicStride` specialisation: stride as a compile-time-OR-runtime axis, the same pattern `bound::type::Dynamic` and `spline::type::Dynamic` apply to boundary conditions and spline orders. `distance/mesh_utils.h`, which otherwise duplicates this hierarchy, has no such axis. * Also going: an owning container (`concrete_vector.h`, `new T[size]`) alongside the non-owning views -- everything in `mesh_utils.h` is a view over caller memory -- and `stream.h`'s ostream printing for debugging. * The two hierarchies are stylistic siblings, not a redesign: both use `FF_CUHOSTDEV virtual` abstract bases (34 virtuals here, 30 in mesh_utils.h). Whatever made `mesh_utils.h` the one that survived, it was not a polymorphism-vs-CRTP decision. Deliberately not touched --------------------------------------------------------------------------- * `tools/consolidate.sh` still names these paths in four places. It is the frozen record of the six-repo migration and describes a tree that existed on 2026-08-19; editing it would falsify history, not tidy it. * `.github/workflows/ci.yml`'s `^tests/kernels/` path filter stays. `tests/kernels/atomic/` remains and is built by the tsan leg. Verified --------------------------------------------------------------------------- All 46 TU include graphs still resolve after the deletion, and `make test-atomics` -- the other program under tests/kernels/ -- still builds and passes. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
balbasty
commented
Aug 20, 2026
CollaboratorAuthor
can merge |
Modify/delete conflict on all 11 impl/kernels/vector/ headers: #145 added #pragma once to them while this branch deletes them. Resolved in favour of the deletion -- they are unreferenced by include/ and src/, and their only consumer (a compiled-but-never-run scratch target) goes with them. Verified after resolution: no surviving reference to kernels/vector anywhere in include/, src/, tests/, make/ or the Makefile.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It is not. 13 files, −2,421 lines: the 11 headers, the scratch program that
was their only consumer, and the Makefile target that built it.
The proof, re-run on
de288a9Not inherited from an earlier survey —
mainhas moved (#143 and #146 mergedsince), so this was re-measured at the commit the deletion is made against.
The decisive check is the preprocessor's own dependency graph, not grep.
clang++ -MMover all 46 translation units the build compiles resolveszero of the 11 headers, directly or transitively:
impl/kernels/vector/src/lib,src/lib-cpu,tests/lib,tests/lib-cpu)src/lib-cuda,tests/impl-cuda)Control, so the scan is not silently finding nothing:
The scratch program does show up. The method detects a real reference.
Why grepping for type names is not conclusive, and why the include closure
is the right question.
vector/declarestype,value,pointer,iterator,Vector,Impl,final_type,this_type— all of which occurin unrelated code (
reg_flow.hhas its ownImpl,posdef.hits ownguess_type,mesh_utils.hits ownfinal_type/this_type). A symbol grepreturns ~1,966 hits, none of them real. In a header-only library the only way
to reach a declaration is to include the header carrying it, so the include
closure settles it and the symbol grep cannot.
Not part of the gate.
Makefilecalled it "a hand-run scratch program …compiled (not run)"; it appears nowhere in
tools/test-baseline.expectedandin no CI job.
What is lost
Recoverable at
de288a9, the last commit where the files exist:Four things worth knowing before anyone goes looking:
7da9e7e("Copy alllib files from jitfields", 2025-07-18), given a mechanical CUDA-annotation
pass the same day in
865d1c5, and never functionally touched again. Theonly three commits since are tree-wide sweeps — the consolidation path
rewrite, the
FF_macro prefixing, and refactor: spell the public interface <fastfields/...> #146's include spelling. Thirteenmonths, zero functional changes.
SwitchStride<T, S, D>and itsDynamicStridespecialisation: stride as acompile-time-or-runtime axis — the same pattern
bound::type::Dynamicandspline::type::Dynamicapply to boundary conditions and spline orders.distance/mesh_utils.h, which otherwise duplicates this hierarchy, has nosuch axis. If the teeny work wants a reference for a strided-view
abstraction, that is the piece to read.
concrete_vector.h,new T[size])alongside the non-owning views — everything in
mesh_utils.his a view overcaller memory — and
stream.h'sostreamprinting for debugging.to find that
mesh_utils.hreplaced virtuals with CRTP and that this waswhy it won; it did not. Both use
FF_CUHOSTDEV virtualabstract bases (34virtuals here, 30 there). Whatever decided which survived, it was not that.
Deliberately not touched
tools/consolidate.shstill names these paths in four places. It is thefrozen record of the six-repo migration and describes a tree as it stood on
2026-08-19; editing it would falsify history rather than tidy it.
ci.yml's^tests/kernels/path filter stays.tests/kernels/atomic/test.cppremains and is built by thetsanleg, sothe filter is still load-bearing. Only the
vector/subdirectory goes.Verification
make test-atomics— the other program undertests/kernels/— stillbuilds and passes (4 checks).
59,886 / 13 / 0 untouched is itself the confirmation. CI is the authority
here; result recorded in a comment below.
Note for the
teenyagentteenyis 159 commits ahead and may still reference these headers. I have notconsulted or accommodated that branch — its agent owns its own merges — but
flagging the deletion here so it is not a surprise at merge time.
Generated by Claude Code