Skip to content

Docs: make ThreadFusion's opt-in semantics explicit (matches FKL default, per Oscar's guidance) - #3

Merged
morousg merged 2 commits into
Libraries-Openly-Fused:mainfrom
johnnynunez:docs/threadfusion-optin
Jun 11, 2026
Merged

Docs: make ThreadFusion's opt-in semantics explicit (matches FKL default, per Oscar's guidance)#3
morousg merged 2 commits into
Libraries-Openly-Fused:mainfrom
johnnynunez:docs/threadfusion-optin

Conversation

@johnnynunez

Copy link
Copy Markdown
Contributor

Summary

Documentation-only follow-up after Oscar's ThreadFusion feedback.

thread_fusion was always opt-in (default False, matching FKL's own TransformDPP<> = TF::DISABLED default), but the docstring and README oversold it ("best for wide images..."). Both now state the upstream guidance explicitly:

Disabled by default, matching FKL's own default (TransformDPP<> = TF::DISABLED). Per Oscar: ThreadFusion only improves performance in a small set of cases (wide images, trivial per-pixel chains, bandwidth-bound) — benchmark YOUR pipeline before enabling it; it is not a general speedup.

No behavior changes; no example or test enables it implicitly (the only thread_fusion=True in the repo is the dedicated correctness suite that verifies TF output is identical to scalar).

thread_fusion was ALWAYS opt-in (default False, matching FKL's own
TransformDPP<> = TF::DISABLED default), but the docstring/README sold it
too enthusiastically. Now both state explicitly: disabled by default,
matching upstream, because per Oscar it only improves performance in a
small set of cases — benchmark your pipeline before enabling.
C++-only probe using fk::CircularTensor exactly as designed (float,
COLOR_PLANES=3, NewestFirst, TensorSplit write). Values come out CORRECT
(rotation 5/4/3) but compute-sanitizer reports 33 invalid reads/writes per
run on unfixed headers: update's 1-plane read + copy's BATCH-plane read
sum to BATCH+1 launched planes. Sanitizer-clean with FKL PR #252 applied.
This is the evidence posted on FusedKernelLibrary#250.
@morousg
morousg merged commit 63c3e8f into Libraries-Openly-Fused:main Jun 11, 2026
4 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.

2 participants