Skip to content

[TEST] Increase seq1/2/3.fa and small.fa sequences and remove unused files in data/ - #340

Merged
eseiler merged 5 commits into
seqan:mainfrom
smehringer:fix
Sep 11, 2026
Merged

eseiler merged 5 commits into
seqan:mainfrom
smehringer:fix

Conversation

@smehringer

@smehringer smehringer commented Sep 10, 2026

Copy link
Copy Markdown
Member

First commit changes tests because the data changed (Claude corrected the tests).

Next commits delete unused files.

Extra Info to the first commit


Some of the small tests in chopper are failing when actually using MinHashes, because not enough hashes could be generated for the sketches from the short sequence. Fine, I thought at first — I'll just make the sequence bigger. But for a single bucket that still wasn't enough, even though the others had long since been full. I set the AI loose on it, and at the end I'll copy in here what it found out.

First, minHashes: I have 16 buckets and want 40 hashes per bucket (sketch size).

Membership in a bucket is determined by the last 4 bits.

static constexpr uint64_t register_id_mask{15}; /// ...00001111
// ...
    for (uint64_t const hash : smallest_values)
    {
        auto & hash_table = table[hash & register_id_mask];
        if (hash_table.size() < sketch_size)
            hash_table.push_back(hash >> 4);
    }

The sequence is hashed with the minimizer_hash view, k = 15, w = 15, meaning canonical k-mers.

Now exactly one bucket simply didn't reach the 40 sketches, even though my sequence had about 4000 hashes. I thought maybe I'd accidentally typed in some redundancies. So I asked the AI to check whether it works with perfectly random DNA, and if not, what it thinks is going on.

The AI now says this is a problem that's just annoying for small-number examples, and that I could simply increase the sequence length. I'm wondering whether this non-uniformity is actually okay. Could you take a look at it too?

This is what the AI found out:

The mechanism

For an odd k (15 here), a canonical k-mer's hash is min(fwd, revcomp), where:

fwd's most-significant digit is rank(w[0]) (the window's first base), and its least-significant digit (the one that ends up in the bucket) is rank(w[14]) (its last base).

revcomp's most-significant digit is 3 - rank(w[14]) (complement of the last base), and its least-significant/bucket digit is 3 - rank(w[0]) (complement of the first base).

So the two boundary bases each play two roles: w[14] decides how "small" the reverse candidate looks and, if forward wins instead, becomes forward's own bucket digit. w[0] is the mirror case for the reverse candidate.

Since adjust_seed XORs the same fixed seed onto both fwd and rev before the comparison, and XOR doesn't preserve digit-magnitude ordering, "who wins" stops following the natural A<C<G<T rank order and instead becomes a seed-specific, per-base coin flip. I measured it directly on seq1.fa (k=15, chopper's real seed 0x23cfdced):

fwd win rate, conditioned on last base w[14]:
A: 36.5% C: 13.6% G: 88.9% T: 63.8%

(conditioning on the first base gives the mirror-symmetric numbers for rev's win rate). Overall it's ~51/49, nicely balanced — but base-by-base it's wildly skewed and non-monotonic in rank, purely because of how this particular seed's bits happen to fall.

Bucket 0 (post-seed) specifically requires either "forward wins AND window ends in TC" or "reverse wins AND window starts with GA". Both TC-ending and GA-starting windows are perfectly ordinary in frequency (~218 and ~244 out of 3826, right around the expected 1/16 share) — but because a window ending in C only wins the forward race 13.6% of the time, and a window starting with G only loses the forward race (i.e. rev wins) 11.9% of the time, almost none of those otherwise-common windows ever get to contribute to bucket 0. That's the entire story: 16 out of 3826, matching the real binary exactly.
Why regenerating the sequence didn't help

This isn't a property of your fasta content at all — it's a property of k=15 + this exact fixed seed. I confirmed it's seed-driven, not data-driven: with seed=0 a different bucket (15) gets starved instead, but some bucket always does, regardless of which random sequence I fed in. Repetition vs. randomness in the input is irrelevant; the seed's bit pattern relative to the base-rank encoding is what matters.

It also isn't just an artifact of looking at the "smallest N" (an order-statistics effect) — I checked the entire canonical hash population, not just the smallest slice, and bucket 0 tops out at 12–16 total occurrences over ~3800–4200 windows, roughly a 15–20x deficit versus the ~240–265 you'd expect under uniformity. So this bucket fills at roughly 1/15th the normal rate — reaching 40 would need something like 3–4x more total k-mers than the other 15 buckets need, i.e. a genuinely bigger corpus, not just cleaner content.

@seqan-actions seqan-actions added lint and removed lint labels Sep 10, 2026
@seqan-actions

Copy link
Copy Markdown
Member

Documentation preview available at https://docs.seqan.de/preview/seqan/chopper/340

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.35%. Comparing base (f0c38a4) to head (f8db960).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #340   +/-   ##
=======================================
  Coverage   93.35%   93.35%           
=======================================
  Files          20       20           
  Lines         753      753           
  Branches       18       18           
=======================================
  Hits          703      703           
  Misses         50       50           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@seqan-actions seqan-actions added lint and removed lint labels Sep 10, 2026
@seqan-actions seqan-actions added lint and removed lint labels Sep 11, 2026

@eseiler eseiler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the code.
I'll double check the algorithmic part later.

PS: Your branches are 130+ commits behind upstream main :)

@eseiler
eseiler enabled auto-merge (squash) September 11, 2026 13:16
@eseiler
eseiler disabled auto-merge September 11, 2026 13:17
@eseiler
eseiler enabled auto-merge September 11, 2026 13:17
@eseiler
eseiler merged commit b77c373 into seqan:main Sep 11, 2026
28 checks passed
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.

3 participants