[MarkRose]

The self-test has a good correctness check and throws its verdict away. This makes it capable of failing, and extends it to the FFT lengths it never reached.

The core defect

Mlucas.c:4853-4855:

DONE:
	fprintf(stderr, "\n  Done ...\n\n");
	return(0);

-s and -fft/-radset always exit 0. Measured on a stock nosimd build of 418f365:

$ ./Mlucas -s tt ; echo "exit=$?"
WARNING: 1 of 3 radix-sets at FFT length 4 K passed - skipping it. PLEASE CHECK YOUR BUILD OPTIONS.
WARNING: 1 of 3 radix-sets at FFT length 5 K passed - skipping it. PLEASE CHECK YOUR BUILD OPTIONS.
...
exit=0

And ci.yml's four self-test steps gate on that status — three of them additionally pipe into tee, so even a corrected status would be swallowed by the pipeline. No self-test regression could ever have turned CI red.

Four more, underneath

2. The new-data consensus anchor is if(!new_res.sh0). A first-radset Res64 == 0 leaves the anchor falsy, so every radix set is scored as passing whatever it computed. On pristine main, -m 18776473 -fft 992 -iters 100 prints Res64: 0000000000000000. AvgMaxErr = 0.000000874 and writes that zero into mlucas.cfg as a certified "2 of 2 consensus". Exposure: 42 of 136 MvecPRP rows, the 10000-iter column of 17 MersVec rows, and every off-table -fft.
3. resFlag = 0 on a match erases an earlier Res64 mismatch.
4. No zero-residue rejection when there is no reference — the zero is adopted as the reference.
5. The "length not in the table" test is dead code: i != MvecPtr[j].fftLength is evaluated at j == numTest, against the row just written a few dozen lines earlier, so it is never true. -fft 992 -iters 100 refused to run at all.

Note that fracmax/AvgMaxErr cannot substitute for any of this: it measures distance from the nearest integer, and zero is an integer, so an all-zero residue reports perfect numerical health.

Coverage, measured rather than assumed

The "56 of 603" figure in circulation compares two different populations — 56 is the nosimd Teensy count, 603 the SIMD total. Enumerated properly via get_fft_radices() in a harness linked against the real objects: 153 legal lengths / 603 (fftlen, radix_set) pairs under SIMD, 708 under nosimd. -s a covers 454/603 (75%); all tiers together 564/603 (93.5%). Per tier on avx2: tt 26, t 93, s 127, m 145, l 63, h 67, e 43 — reproduced exactly by the new summary line.

The gap that matters is not the 6.5%. The 17 uncovered lengths are exactly the 31·2ᵏ and 63·2ᵏ families — the only lengths at which leading radices 63, 992, 1008 and 4032 are ever used, and therefore precisely where every silent-wrong-residue defect found in this campaign lives. All 27 radix sets there fail today in plain AVX2.

What this changes

• An exit contract: 1 = wrong answer, 2 = length untested, 0 = clean.
• A new -s c coverage sweep over the uncovered lengths. It needs no new reference data: Res64 depends only on (exponent, iters), so each length borrows the nearest tabulated row. Verified — p=17551099 gives AB1180428ED65EE0 at 896K and at all 12 radsets of 1024K.
• The anchor fix, zero-residue rejection, the resFlag fix, the dead-branch fix, an always-printed summary, and CI gating via ${PIPESTATUS[0]} so the tee no longer swallows the status. CI fails on status 1 only, since status 2 fires on today's 1K/2K.
• The same rule for the monthly large/huge self-test workflow, selftest-large.yml (added since by CI: move the large self-test to its own monthly workflow, and add the huge self-test #294): fail on 1, pass on 2, in POSIX form in the FreeBSD job.

Plain cross-radix-set agreement was considered and measured insufficient — at 992K both radsets share leading radix 992 and both return 0000000000000000, so they agree with each other and are both wrong.

Validated failing-first

A test that cannot fail is the subject of this PR, so each check was demonstrated to fail before being demonstrated to pass:

case	before	after
radix992 (nosimd), broken	—	8 wrong answers, Res64 Error, exit 1
radix992 with #179	—	992/1984/3968/7936/15872 all pass, 0 wrong
radix16 zero-cosine (AVX-512/SDE, -fft 8 -radset 1)	pristine main exit 0 on ERR_CARRY	exit 2; with #214's fix, correct Res64 and exit 0
-s t / -s s negative controls	—	93/93 and 127/127, exit 0

Independently reproduced here before pushing: stock main -s tt exits 0 on the 4K/5K/7K/9K/10K warnings above; this branch exits 1 on the identical set.

Runtimes (avx2, 4 threads): tt 2.2 s, t 15 s, s 44 s, m 1493 s, and -s c ≈2–4 min once those lengths work.

Known blockers for a stricter gate, not fixed here

1K and 2K are unusable in a stock AVX2 build of main — 1K gives ROE ≈0.4993 on iteration 1, and 2K radset 0 hits ERR_ASSERT "radix < 16 does not support shifted residues" against the self-test's own random RES_SHIFT. That is why CI gates on status 1 rather than != 0. AVX-512 under SDE loses 8 of 15 teensy lengths. And -s never exercises Fermat-mod at all.

Summary by CodeRabbit

• New Features

  • Added a self-test coverage mode that sweeps supported FFT lengths through the largest Large-tier length, including lengths without built-in reference data.
  • Self-tests now provide a completion summary and distinguish incorrect results from cases that could not be tested adequately.
  • Self-tests use available reference data and can select a reference for supported lengths without a table entry.

• Bug Fixes

  • Self-tests detect zero residues and disagreements between radix sets without allowing later matches to erase earlier failures.
  • FFT configuration checks continue after an untestable length and report other failures after processing all lengths.
  • Automated test runs accept self-test status 2 while still failing on other error statuses.

[MarkRose]

Rebased onto current main to clear the merge conflict.

Nine conflicts, all in ci.yml, because #103 reworked every self-test block this PR gates. Rather than hand-merge two divergent versions nine times, I took main's job structure wholesale and re-applied the gating on top of it, so the CI half of this PR is now written against the loops that actually exist — including the -prp loops and the per-word native Mfactor runs that did not exist when this branch was written.

The gate is now checked end to end

Worth showing, because the first control I ran did not actually test anything.

stock build      ./Mlucas -s tt -> exit 2   ("2 FFT lengths had too few usable radix sets")
corrupted build  ./Mlucas -s tt -> exit 1   ("17 cases returned a wrong answer")

so the job stays green on 2 and goes red on 1, which is what the gate is for.

Getting there took two false negatives worth recording. Corrupting the 1K row proved nothing: that case halts on a roundoff error in an AVX2 build before any residue is compared. Corrupting nineteen larger rows then also proved nothing, because makemake.sh refuses to overwrite an existing obj_avx2/Mlucas and I had re-run the stale binary. Only after deleting the object directory did the corruption show up. Both failures looked exactly like "the gate works" from the outside.

Build OK, and the stock -s tt summary is unchanged from main apart from the new pass/fail block.

[MarkRose]

Two follow-up commits, both fixing defects in this PR's own gating.

The status was never examined. The pipeline form

time ./Mlucas -s $s -cpu "..." |& tee -a test.log
st=${PIPESTATUS[0]}

runs under set -e with pipefail, so a nonzero status ends the step on the pipeline itself and the st= line never executes. The whole point of capturing the status was to tolerate 2; instead every job that hit it died on the first tier of the ladder and ran none of the others. The status is now captured in a || st=${PIPESTATUS[0]} tail, which set -e does not apply to.

The gate was too narrow. if (( st == 1 )) passes everything else, including the statuses the self-test never returns. A SIGSEGV part-way through -s m (139) or an abort (134) counted as a clean ladder - I have one of each in the current logs. It is now st != 0 && st != 2, so 2 stays tolerated for the stock build's 1K/2K rejections and nothing else does.

config-fermat.sh. Callers invoke it as bash -e -o pipefail -- ./config-fermat.sh, so now that Mlucas returns a nonzero status the first affected length ends the script. On ARM that is F18 at 15K, and F19 through F29 were never configured and never tested - the run simply looked finished. It now carries on past status 2 and exits nonzero at the end for anything else.

[MarkRose]

Rebased onto main now that #291 is in. Besides the conflict resolutions, two things changed because of what #291 added:

• A new commit gates four more self-test runs. Enabled -Wextra and updated CI to remove remaining error masking #291 added self-test runs to the Linux-32-bit and Linux old jobs and removed continue-on-error from both TSan jobs. All four ran the self-test bare under set -e, so with this PR they would have ended their jobs on status 2, which a stock build still returns at 1K and 2K. They now use the same gate as the other jobs: 0 and 2 pass, anything else fails.
• The FreeBSD gate no longer uses a pipe. Enabled -Wextra and updated CI to remove remaining error masking #291 added FreeBSD 10 to the matrix, and its /bin/sh has no pipefail (the step only sets it for 11 and later). Piping Mlucas into tee there would report tee's status, so every run would pass. Mlucas now writes to a file, the status is taken directly, and the file is shown and appended to test.log.

I tested both gates against a stub Mlucas returning 0, 2, 1 and 139: the bash gates under bash -e, and the FreeBSD one under busybox sh without pipefail. 0 and 2 pass, 1 and 139 fail, and the output still reaches test.log.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: c45526f

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 0c590e5d-7196-412f-88ae-722c9651be51

Commits

Reviewing files that changed from the base of the PR and between ecc7f99 and c45526f.

Files selected for processing: 1

• .github/workflows/ci.yml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Mlucas adds coverage sweeps for FFT lengths without reference rows and distinguishes wrong answers from lengths that could not be tested. The configuration script and CI workflows capture and handle the resulting self-test exit statuses.

Changes: Self-Test Behavior

Layer / File(s)	Summary
Residue checks and result accounting src/Mlucas.c, help.txt	Self-tests detect incorrect residues, count test outcomes, and return status 1 for wrong answers or status 2 when an FFT length has too few usable radix sets. The help text documents the exit statuses.
Unlisted FFT-length coverage sweep src/Mlucas.c, help.txt	The -s c, -s cover, and -s coverage options sweep supported FFT lengths without reference rows. Each length uses the nearest lower-or-equal table row for its exponent and reference residues.
Script and CI status handling config-fermat.sh, .github/workflows/ci.yml, .github/workflows/selftest-large.yml	The configuration script continues after status 2 and returns other nonzero statuses after processing. CI self-test steps capture Mlucas statuses, accept 0 and 2, and fail for other statuses.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Possibly related issues

• primesearch/Mlucas#224 — The issue connects this pull request to self-test coverage and status-gating changes.

Possibly related PRs

• primesearch/Mlucas#365 — Both pull requests rely on status 2 being accepted for self-tests in CI.
• primesearch/Mlucas#263 — Both pull requests change status propagation for tee pipelines in ASan workflow steps.

Suggested labels: enhancement, github_actions

Merge Risk: Minimal · up to c4552

The documented exit statuses now match how excessive-roundoff cases are handled. No actionable merge-blocking risk remains after normal checks.

Architecture risk: Low · up to faf88

The change affects 3 systems.

Changed systems: config-fermat.sh, help.txt, src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — config-fermat.sh (service) was modified; 1 changed file maps to changed impact.
• observed — help.txt (service) was modified; 1 changed file maps to changed impact.
• observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in help.txt: Adds -s cover/-s c for 100- or 1000-iteration coverage of supported FFT lengths lacking built-in reference-exponent rows, specifically the odd-31 and odd-63 families from 992K through 32M. Each length uses the exponent and stored residues of the nearest tabulated length at or below it; the stated runtime is roughly that of -s l.
• observed — Modified behavior in help.txt: Documents self-test exit statuses: 0 if all radix sets match references or are skipped for a reason that does not impugn the answer; 1 if any case has a wrong answer, including a residue mismatch, disagreement between radix sets at one length, or an identically zero residue; and 2 if there are no wrong answers but a length has too few usable radix sets to test. A one-line summary is printed at the end.
• observed — Modified behavior in src/Mlucas.c: Interactive self-tests now flag a residue as incorrect when all three Selfridge–Hurwitz residues are zero, except for a Mersenne primality test run for exactly (p-2) iterations. Unlike the removed comparison branches, matching any supplied reference residue no longer clears an existing failure; mismatches still set it.
• observed — Modified behavior in src/Mlucas.c: The Res35m1 reference check now tests for inequality directly; a match no longer clears a failure recorded by an earlier check.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the two main changes in the PR: making the self-test able to fail (wrong answers return status 1, untested lengths return status 2) and extending coverage to FFT lengths the self-test …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Title check

Explanation

The title names the two main changes in the PR: making the self-test able to fail (wrong answers return status 1, untested lengths return status 2) and extending coverage to FFT lengths the self-test never reached through the new -s c/cover sweep. The raw summary and PR objectives both confirm these as the primary changes. The title is 81 characters, which is slightly long but still a single descriptive sentence.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[MarkRose]

I rebased this onto main, only to fix the conflicts in .github/workflows/ci.yml with #365 and #367, so this needs your approval again. Two small commits come with it:

• 73ef063: CI: run the single-threaded jobs' tests concurrently, one process per core #365 moved the single-threaded jobs' self-tests into parallel steps, so those steps now get the same exit-status gate as the other jobs (0 and 2 pass, anything else fails).
• c45526f: while checking the rebase I noticed 1ceddcf had dropped the upload-artifact step after each TSan self-test when it replaced the lines next to them. That's restored.

Everything else is unchanged.

[MarkRose]

@coderabbitai review

[coderabbitai]

Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.