Skip to content

Refactor july - #62

Merged
cvigilv merged 20 commits into
mainfrom
refactor_july
Jul 14, 2026
Merged

Refactor july#62
cvigilv merged 20 commits into
mainfrom
refactor_july

Conversation

@MichielStock

@MichielStock MichielStock commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Fixing all kinds of issues

  1. Full repo audit → created TODO.md with every bug reproduced by execution, not just read (tests were 268/268 green while hiding all of them).
  2. Constructor convention (HV(this; D) = token-encoding, never a dimension): documented canonically on AbstractHV and all 7 types, README Usage rewritten with real outputs, all encoding.jl example blocks regenerated, plus a one-time-per-session @warn for integer tokens — all locked in by a convention testset. (The @warn was later superseded by a hard error — see item 12.)
  3. Docstring overhaul: all 8 type docstrings follow one template (signatures → identity → algebra → convention → indexing → examples → see-also → references), with 1–2 seeded jldoctests each — every output machine-generated via doctest(fix = true), never hand-written. bundle/bind/unbind got docstrings too. Doctests now run in CI (dedicated job + docs build).
  4. Display/extension architecture: the extension now precompiles (single show in the package dispatching via get_extension), plain display uses per-type Base.summary + Base's array machinery, unicodeheatmap/unicodehistogram are exported and directly callable, FHRR gets an honest plain header instead of meaningless μ±σ, and StatsBase was dropped. Display finally has tests — plain and rich suites in separate Julia processes.
  5. Indexing: scalar getindex returns T (verified for all 7 types); non-scalar returns a plain Vector, never a hypervector — decided, documented, tested. Settled the BinaryHV element question factually: {false, true} (Bool), displayed as 0/1.
  6. RNG modernization: Xoshiro everywhere, default_rng(), rng takes instances, deterministic constructor is HV(this; D) with no rng — token construction now ~0.6 µs instead of allocating a 2.5 KB MersenneTwister state each call. All seeded outputs regenerated; quasi-orthogonality tests added.
  7. Bugs found by verification, resolved by your decisions: unbind(::RealHV, ::RealHV) (was a raw MethodError) now throws an explicit ArgumentError pointing to similarity/FHRR/BipolarHV, with a full unbind testset across all types. Your BipolarHV bit-flip (true ↦ -1) is verified sound — bind-as-XOR is now exactly the ±1 product.
  8. BipolarHV polarity flip completed systematically: audited all 25 sites touching the .v storage, each with an explicit verdict. Fixed: summary labels (were inverted), the data constructor (was negating input), stale doctests/README. Verified correct without change: bundle's majority vote (mapping-invariant), the bipolar dot formula, isapprox, hash/isequal, shift/perturbate. New polarity-locking tests are deliberately not polarity-blind (all(x * x .== 1), construction/indexing round-trip, summary counts) — the original bug survived a green suite because XOR-self-inversion and cosine are invariant under the flip. Tie-break outcome change noted under Add another stragery for tie-breaking #15.
  9. Six more bugs fixed, one commit each, every one reproduced by execution first and locked by a regression test: perturbate on FHRR (now resamples phases, stays on the unit circle), instance-path decodelevel/convertlevel (kwarg mis-dispatch), bundle/bind silently dropping custom distr (locked by the numerical consequence: normalize! rescales to the original spread), in-place ops returning raw vectors (shift! and the three clamp!-based normalize! methods; all perturbate! argument forms locked), const δ, and cross-type isequal/hash. Adjacent bug found and flagged, not fixed: instance-path convertlevel builds encoder and decoder over different random ladders — measured decode(encode(x)) error up to 1.0 (TODO §1.4b, retired properly by the encoder follow-up below).
  10. Cross-type equality is now strictly false: Julia's numeric true == 1 let an all-true BinaryHV equal an all-+1 BipolarHV even though their stored bits are opposite since the flip. ==/isequal between different hypervector types return false; same-family different-parameter (TernaryHV{Int8} vs {Int64}) compares by value; hashing stays element-based because isequal(hv, ::Vector) can legitimately be true and the hash/equality contract must hold against plain vectors too. All locked by tests.
  11. perturbate resampled from the TYPE-default distribution instead of hv.distr (found in the §1.5 pattern sweep — right metadata, wrong-distribution elements; measured std ≈ 1 where 5 was configured). Fixed with instance eldist methods and locked by resampled-statistics tests.
  12. The encode interface (design change): the package now has an explicit layer taxonomy — primitives (operations.jl) → combinators (encoding.jl, unchanged) → encoders (encode.jl, new: raw data in, hypervector out). encode(HV, x) is the canonical deterministic token path; HV(x) is sugar for it. Constructors have one meaning each: HV(n::Number) throws an ArgumentError naming both alternatives (D = n / encode(HV, n)) — replacing the @warn from item 2 — and data constructors are widened to AbstractVector{<:Real} with per-type element validation (BinaryHV {0,1} — BinaryHV([1, 0]) is finally a 2-element hypervector, not a silent 10,000-dim hash; BipolarHV strictly ±1 with zero pointing to TernaryHV; TernaryHV {-1,0,+1} with permissive inner constructors for operation results; graded ranges rejected instead of silently clamped; FHRR unit-modulus). Tuples of reals read as data. Sequence strategies dispatch on AbstractEncoding: KMer(k) resolves New k-mers encoder #53 (windows as atomic hashed tokens) and is deliberately distinct from NGram(n) (symbol-level shift-binding via ngrams); plus Sequence() and BagOfSymbols(). Extension point: one struct + one encode method. The new validation immediately caught two latent wrongs: a test using the old trap idiom and a tutorial example constructing a TernaryHV from 0:9.
  13. Tracking: TODO.md and CLAUDE.md document the architecture, every decision, and the flagged follow-ups.

Test suite: 268 → 669 assertions (plus 37 extension tests in a separate process and CI-enforced doctests). Breaking changes: seeded RNG streams (MersenneTwister → Xoshiro), BipolarHV polarity + strict data validation, graded types reject instead of clamp, cross-type equality, numeric constructor tokens throw.

Follow-ups (tracked in TODO.md)

  • Stateful encoders as an AbstractEncoder{HV} hierarchy (RandomProjection, LevelEncoder) with encode/decode — retires §1.4b properly; encode's first-arg slot is reserved for it.
  • Classification workflow (train/predict) — the JuliaCon demo.
  • Registration track: Aqua.jl + the three §2 API decisions (Base.bind, similar, normalize clash), CI matrix, coverage, CompatHelper/TagBot → register in General (Release package #9).
  • Docs polish: README badge, docs restructure (On improving the documentation #32), intro-tutorial ngrams example (Documentation issues #36).

@MichielStock
MichielStock requested a review from cvigilv July 14, 2026 10:14
github-actions[bot]

This comment was marked as resolved.

github-actions[bot]

This comment was marked as resolved.

MichielStock and others added 10 commits July 14, 2026 14:30
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bit true now maps to -1, so XOR on stored bits is exactly the +-1 product.
Sign-based real-vector constructor (zero elements throw, pointing to
TernaryHV), corrected summary labels, regenerated doctests, and
polarity-locking tests (x * x is the all-+1 identity, construction/indexing
round-trip, summary counts).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
perturbate(::FHRR, ...) threw a MethodError (no eldist); dedicated methods
now resample e^(2pi*i*rand()) at the selected positions. Locked by tests
asserting unit modulus, untouched positions, and exact resample counts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The vector decodelevel methods now accept (and ignore) testbound so the
generic forwarding no longer mis-dispatches into a MethodError. Adjacent
bug flagged, not fixed: the instance path builds encoder and decoder over
different random ladders (TODO 1.4b).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RealHV/GradedHV/GradedBipolarHV results carried the default distr, which
silently changed normalize! numerics. Locked by metadata assertions and a
numerical test that normalize! rescales to the original spread.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generic shift! and the clamp!-based normalize! methods returned the raw
wrapped vector. Locked by op!(hv) === hv tests for all types and all
in-place operations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
δ was an exported non-const global (type-unstable at call sites).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
isequal compared raw storage across types, so a BinaryHV equalled a
BipolarHV whenever the bits matched; the one-arg hash override was
inconsistent with two-arg hashing. isequal is now a same-type storage
fast path, cross-type comparisons fall back to Base element semantics,
and hashing is element-based. Locked by same-bits/different-type tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions[bot]

This comment was marked as resolved.

@MichielStock

Copy link
Copy Markdown
Collaborator Author

Please don't push to this branch today, still work in progress

MichielStock and others added 5 commits July 14, 2026 14:59
The previous fix (2e952a7) let cross-type comparisons fall back to Base
elementwise semantics, but Julia treats Bool as numeric (true == 1), so an
all-true BinaryHV still equalled an all-+1 BipolarHV — whose stored bits are
the exact opposite since the polarity flip. == and isequal between different
hypervector types now return false unconditionally; same-family comparisons
(e.g. TernaryHV{Int8} vs TernaryHV{Int64}) compare by value, and comparisons
against plain vectors keep Base elementwise semantics. Hashing stays on the
element-based AbstractArray fallback: isequal(hv, ::Vector) can be true, so a
type-salted hash would break the hash/equality contract.

Locked by tests covering the numeric-coincidence case, the same-family case,
and the hash contract for all seven types (including BipolarHV, where storage
and elements disagree) and against plain vectors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the missing encoder layer (raw data in, hypervector out) and gives every
constructor form exactly one meaning:

- encode(HV, x; D) is the canonical deterministic token path (hash -> seed);
  HV(x) is shorthand for it. encode(HV, x, strategy) dispatches on the new
  AbstractEncoding strategies: KMer(k) (windows as atomic hashed tokens —
  resolves #53), NGram(n) (symbol-level shift-binding via ngrams), Sequence()
  (bundlesequence) and BagOfSymbols() (multiset). KMer and NGram are genuinely
  different operations and are documented as such. Extension point: one struct
  subtyping AbstractEncoding plus one encode method.
- HV(n::Number) throws an ArgumentError naming both alternatives (D = n /
  encode(HV, n)); the one-time @warn and its testset are gone.
- Data constructors are widened to AbstractVector{<:Real} and validate each
  type's element domain: BinaryHV {0,1}; BipolarHV strictly ±1 (zero points to
  TernaryHV, no more silent sign-taking); TernaryHV {-1,0,+1} (inner
  constructors stay permissive for operation results); Graded types
  range-checked instead of silently clamped; FHRR unit modulus. Invalid arrays
  throw instead of silently token-encoding — BinaryHV([1, 0]) is now a
  2-element hypervector, killing that trapdoor. Tuples of reals read as data.

BREAKING: numeric constructor tokens throw; graded out-of-range data throws
instead of clamping; bipolar non-±1 data throws instead of sign-coercion;
Vector{<:Real} data construction added for BinaryHV. Also fixes a tutorial
example that constructed an invalid TernaryHV from 0:9 (now RealHV).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The §1.6 fix (9049cb9) is verified correct for every argument form; the
locking test only covered the count form — now also fraction, mask, and
index-vector.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eldist(hv) now returns hv.distr for RealHV, GradedHV and GradedBipolarHV
instead of falling back to the type default, so perturbate (and level
ladders built from custom-distr hypervectors) draw replacement elements
from the vector's own distribution. Locked by a resampled-element
statistics testset. Closes TODO §1.5c.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@MichielStock

Copy link
Copy Markdown
Collaborator Author

Big change @cvigilv, you might take a look and see if we can commit before finishing tutorials.

@cvigilv

cvigilv commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Big change @cvigilv, you might take a look and see if we can commit before finishing tutorials.

I'll review it now and let you know. Thanks for the PR

@cvigilv

cvigilv commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Looks great! I honestly have no problem with merging this. That said, we will have some work merging the tutorials branch, so we can (again) make use of Claude for that. I'll approve this, merge it to the tutorial branch, solve the conflicts, and ask for a review again.

@MichielStock

MichielStock commented Jul 14, 2026 via email

Copy link
Copy Markdown
Collaborator Author

@cvigilv
cvigilv merged commit 5dc690b into main Jul 14, 2026
4 checks passed
@MichielStock MichielStock mentioned this pull request Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New k-mers encoder

2 participants