Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,21 @@ types, tests, infrastructure, and alignment.
- CI: pipeline extended to licence-header, formatting, lint, and commit
convention checks ahead of typecheck/test/bench/build.

### Fixed

- **Zero-depth samples poisoned the transform stage.** A sample with no reads
reached the transform, where every transform divides by a sample total:
`relative` wrote `0.0` for every feature (a value where the answer is
*undefined*), `rarefy` took `min_lib = minimum(lib_sizes) = 0` and scaled
**every** sample by zero, and `clr` took `log(0) = -Inf` that the NaN/Inf
healing later replaced with `epsilon`. All-zero samples are now healed
*before* the transform rather than after, which also removes the
inconsistency where `prepared` and `filtered_counts` described different
data under `drop_policy=impute`. Visible results change only for `impute`
runs over inputs containing a zero-depth sample; `drop` and `refuse` are
unchanged. Owner-authorised behaviour change, recorded in
`docs/statistics/behaviour-change-zero-depth-samples.md`.

## [0.1.0] — 2026-05-21 (upstream baseline)

Initial public state of the application as inherited from upstream
Expand Down
90 changes: 90 additions & 0 deletions docs/statistics/behaviour-change-zero-depth-samples.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<!--
SPDX-License-Identifier: CC-BY-SA-4.0
-->
# Behaviour change — zero-depth samples in the relative-abundance transform

**Status: decided and implemented on 2026-09-22.** This note exists because the change
alters results that saved analyses were computed from, which is not a thing to do quietly.

## What changed

A sample with **no reads at all** (a *zero-depth* sample: every count zero) used to reach
the transform stage inside `prepare_analysis_table`. Every transform divides by a sample
total, so such a sample poisoned whichever transform ran. All-zero samples are now healed
**before** the transform instead of after it.

## Why it had to change

The pipeline wrote a value where the truth is that no value exists:

| Transform | What a zero-depth sample produced |
| --- | --- |
| `relative` | `0.0` for every feature — the claim *"this feature's relative abundance is exactly 0"* about a sample whose relative abundances do not exist. `0/0` is undefined; `0.0` is a value. |
| `rarefy` | `min_lib = minimum(lib_sizes)` is `0`, so **every** sample was scaled by `0/lib`. The result was a prepared table of zeros for all samples, reported as success. |
| `clr` | `log(0) = -Inf`, which centring turns into `NaN`, which the later NaN/Inf healing replaced with `epsilon` — a wrong number presented as a healed one. |

Healing afterwards could not repair any of it. Under `drop_policy=drop` the affected
columns were discarded, but only after the damage was computed; under
`drop_policy=impute` the imputed counts were assigned while `prepared` kept the values
computed from the empty column, so **`prepared` and `filtered_counts` described different
data** — the one inconsistency a pipeline must never have.

This is a case of the principle stated in `method-conditions/exact-descriptive-summaries.md`:
reporting `0` for a zero-depth sample's relative abundance is a lie told with a straight
face, and no downstream check can catch it, because `0.0` is an ordinary number.

## Who is affected

Runs whose input contains a zero-depth sample can have different prepared
results when healing moves before the transform:

| `drop_policy` | Behaviour before | Behaviour now |
| --- | --- | --- |
| `drop` | `rarefy` included the zero-depth sample, so retained columns were scaled to zero before the sample was dropped | sample dropped before `rarefy` — **prepared result changes for `rarefy`** |
| `refuse` | refused | refused, earlier — **same refusal** |
| `impute` | counts were transformed before healing, so `prepared` could retain values for the unhealed zero-depth column | counts imputed, transform sees the imputed counts — **prepared output can change** |

For a zero-depth input, `drop_policy=impute` can change prepared output for
`none`, `relative`, `tss`, `css`, `rss`, `size_factors`, `presence_absence`, or
`rarefy`. `drop_policy=drop` changes prepared output for `rarefy`. Successful
`clr` and `ilr` runs using pseudocount replacement retain equal log-ratio values
for the zero-depth column. Analyses whose inputs had no zero-depth sample are
unchanged — the healing step returns immediately when
there is nothing to heal.

## What to do

Re-run the affected analysis. The previous numbers for a zero-depth sample were not
measurements of a quantity that exists, so there is no correction to apply: the
computation itself was refusing to say "undefined" and saying "zero" instead.

If you are unsure whether an analysis is affected, check its diagnostics for the
recorded all-zero-sample healing and its `drop_policy`. Re-run `rarefy` analyses
with a `Dropped ... all-zero samples` entry (`drop_policy=drop`), and re-run
`none`, `relative`, `tss`, `css`, `rss`, `size_factors`, `presence_absence`, or
`rarefy` analyses with an `Imputed ... all-zero samples` entry
(`drop_policy=impute`). Successful pseudocount-based `clr` and `ilr` runs retain
equal log-ratio values, as recorded in the affected method/policy combinations
above.

## What deliberately did NOT change

- **All-zero *taxa* are still healed after the transform.** The asymmetry is intentional.
A zero-count taxon in a sample that *has* reads has a relative abundance of exactly
`0.0` — that value is true, so there is no lie to remove — while moving the drop earlier
would change the geometric mean that CLR centres on, altering results for analyses that
were already correct. Fixing a defect is not a licence to change the numbers around it.
- **The legacy path is otherwise byte-identical.** The exact-precision layer
(`exact_summaries.jl`) is a separate, opt-in module that nothing in the pipeline calls;
this change does not touch it.
- **No method is switched silently.** The healing is recorded in `healings`, surfaced in
diagnostics and the manifest, and raises the DANGER self-healing banner as before.

## Provenance

The repository owner (`@hyperpolymath`) authorised this change on 2026-09-22, choosing
"fix it now, change the results" over leaving the defect documented or hiding it behind an
opt-in flag, with this note as the accompanying migration record. This is an explicit,
narrow exception to the standing instruction that existing pipeline algorithms and
previously saved analyses not be silently changed: the change is not silent, and the
results it changes were wrong.
136 changes: 103 additions & 33 deletions src/analysis/Execution.jl
Original file line number Diff line number Diff line change
Expand Up @@ -636,13 +636,18 @@ function heal_all_zero_samples(counts::Matrix{Float64}, sample_ids::Vector{Strin
end
end

function heal_all_zero_taxa(counts::Matrix{Float64}, taxa_ids::Vector{String}, drop_policy::DropPolicy, epsilon::Float64)
check = check_all_zero_taxa(counts)
if !check["has_all_zero_taxa"]
function heal_all_zero_taxa(
counts::Matrix{Float64},
taxa_ids::Vector{String},
drop_policy::DropPolicy,
epsilon::Float64;
indices::Union{Nothing,Vector{Int}}=nothing
)
indices = isnothing(indices) ? check_all_zero_taxa(counts)["all_zero_taxa_indices"] : indices
if isempty(indices)
return (counts, taxa_ids, String[], Int[])
end

indices = check["all_zero_taxa_indices"]
healings = String[]

if drop_policy == DROP
Expand Down Expand Up @@ -903,6 +908,59 @@ function prepare_analysis_table(
effective_pseudocount = config.normalization.method in ("clr", "ilr") ? pseudocount : adv_pseudocount
effective_epsilon = epsilon # could also use adv_epsilon, but use normalization epsilon for now

# ----------------------------------------------------------------------
# All-zero SAMPLES are healed here — before the transform, not after
# ----------------------------------------------------------------------
# This was the defect. Healing used to run after the transform, so `prepared`
# was computed from counts that still contained zero-depth samples, and every
# transform below divides by a sample total:
#
# * relative wrote `0.0` into each feature — the statement "this feature's
# relative abundance is exactly 0" about a sample whose relative
# abundances do not exist at all. 0/0 is undefined; 0 is a value.
# * rarefy took `min_lib = minimum(lib_sizes)`, which is 0 when any sample
# is empty, and then scaled EVERY sample by 0/lib — emptying the
# whole prepared table while reporting success.
# * clr took log(0) = -Inf, which centring turns into NaN, which the
# NaN/Inf healing later replaced with epsilon: a wrong number
# presented as a healed one.
#
# Healing afterwards could not repair any of this. Under drop_policy=drop the
# affected columns were discarded, but only after the damage was computed;
# under drop_policy=impute the imputed counts were assigned while `prepared`
# kept the values computed from the empty column, so `prepared` and
# `filtered_counts` described different data — the exact inconsistency a
# pipeline must never have.
#
# Healing first removes the class rather than the instance: past this point no
# zero-depth column can reach a transform, under any policy.
#
# Visible results change only where they were wrong. Under drop_policy=drop the
# sample is still dropped; under refuse the same refusal is raised, earlier;
# under impute the sample is imputed and the transform now sees the imputed
# counts, so its relative abundances are a uniform distribution rather than a
# column of 0.0.
#
# All-zero TAXA are deliberately still healed after the transform, and the
# asymmetry is intentional. A zero-count taxon in a sample that has reads has a
# relative abundance of exactly 0.0 — that value is true — so there is no lie
# to remove, while moving the drop earlier would change the geometric mean CLR
# centres on and therefore change results for analyses that were already
# correct. Fixing a defect is not licence to change the numbers around it.
healings = String[]
is_dangerous_diag = false
# Preserve the original all-zero taxa before imputing zero-depth samples.
# Otherwise an epsilon inserted for an empty sample makes an all-zero taxon
# appear nonzero when diagnostics and taxon healing run after the transform.
all_zero_taxa_indices = check_all_zero_taxa(filtered_counts)["all_zero_taxa_indices"]
zero_depth = check_all_zero_samples(filtered_counts)
if zero_depth["has_all_zero_samples"]
(filtered_counts, filtered_sample_ids, sample_healings, _) =
heal_all_zero_samples(filtered_counts, filtered_sample_ids, dp, effective_epsilon)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
append!(healings, sample_healings)
is_dangerous_diag = true
end

# Apply zero policy
counts_after_zero = copy(filtered_counts)

Expand Down Expand Up @@ -1003,7 +1061,18 @@ function prepare_analysis_table(
if lib_sizes[j] > 0
prepared[:, j] = counts_after_zero[:, j] ./ lib_sizes[j]
else
prepared[:, j] .= 0.0
# Unreachable since all-zero samples are healed before the transform,
# and deliberately a refusal rather than an assignment. This branch
# used to write `prepared[:, j] .= 0.0`, which states that every
# feature has a relative abundance of exactly 0 -- a value -- for a
# sample whose relative abundances do not exist. 0/0 is undefined,
# and "0" and "undefined" are different claims; the old value was
# wrong in a way no downstream check could detect, because 0.0 is a
# perfectly ordinary number. If a future path reintroduces a
# zero-depth column here, that is a bug to surface, not to paper
# over with a plausible-looking number.
sample_name = j <= length(filtered_sample_ids) ? filtered_sample_ids[j] : string(j)
throw(ArgumentError("INTERNAL: sample '$sample_name' (column $j) has zero total counts at the relative-abundance transform. All-zero samples are healed before this point, so reaching here means the healing was bypassed — the relative abundances of an empty sample are undefined and must not be reported as 0. Please report this with the config id $(config.id)."))
end
end
# Offset for NB_GLM if needed? For relative, no offset, but for TSS offset we would use log(lib_sizes)
Expand Down Expand Up @@ -1099,6 +1168,12 @@ function prepare_analysis_table(
# Rarefy to min library size
lib_sizes = vec(sum(counts_after_zero, dims=1))
min_lib = minimum(lib_sizes)
# A single zero-depth sample would make min_lib 0, and every sample would then
# be scaled by 0/lib — emptying the entire prepared table while reporting
# success. All-zero samples are healed before this point; this refusal exists
# so that if that guarantee is ever broken the failure is loud rather than a
# matrix of zeros that still passes every shape check downstream.
min_lib <= 0 && throw(ArgumentError("INTERNAL: rarefaction target is $(min_lib) — at least one sample has zero total counts. All-zero samples are healed before this point, so reaching here means the healing was bypassed. Rarefying to an empty sample would set every value to 0. Please report this with the config id $(config.id)."))
# For stub, rarefy by subsampling proportionally to min_lib (not exact, just scaling)
for j in 1:size(counts_after_zero, 2)
if lib_sizes[j] > 0
Expand All @@ -1114,9 +1189,15 @@ function prepare_analysis_table(
# ----------------------------------------------------------------------

(checks, warnings, errors) = self_diagnostics(filtered_counts, prepared, config; sample_metadata=sample_metadata, raw_counts=counts)
checks["all_zero_taxa"] = OrderedDict{String,Any}(
"all_zero_taxa_indices" => all_zero_taxa_indices,
"all_zero_taxa_count" => length(all_zero_taxa_indices),
"has_all_zero_taxa" => !isempty(all_zero_taxa_indices)
)

healings = String[]
is_dangerous_diag = false
# `healings` and `is_dangerous_diag` are initialised earlier, before the
# all-zero sample healing that happens before the transform; re-initialising
# them here would silently discard those entries.
banner = nothing

# If errors present, hard-stop with DANGER banner
Expand Down Expand Up @@ -1152,34 +1233,23 @@ function prepare_analysis_table(
is_dangerous_diag = true
end

# Heal all-zero samples/taxa based on drop_policy
# For counts (not prepared), check all-zero
if checks["all_zero_samples"]["has_all_zero_samples"]
try
(healed_counts, healed_sample_ids, heal_s, _) = heal_all_zero_samples(filtered_counts, filtered_sample_ids, dp, effective_epsilon)
# Need to also heal prepared table accordingly — for simplicity, we re-apply transform after healing counts?
# For stub, we just record healing and drop corresponding columns from prepared if DROP
if dp == DROP
# Drop columns from prepared
keep_cols = [j for j in 1:size(prepared,2) if !(j in checks["all_zero_samples"]["all_zero_samples_indices"])]
prepared = prepared[:, keep_cols]
filtered_sample_ids = healed_sample_ids
filtered_counts = healed_counts
else
# Impute: keep prepared but healing already applied to counts, need to re-prepare? For stub, just keep
filtered_counts = healed_counts
end
append!(healings, heal_s)
is_dangerous_diag = true
catch e
# REFUSE policy throws
throw(e)
end
end

# All-zero SAMPLES were healed before the transform, above. The block that used to
# stand here healed them afterwards, and in doing so had to guess how to reconcile
# `prepared` with the healed counts -- the code said so: "for simplicity, we
# re-apply transform after healing counts?" and, for the impute case, "for stub,
# just keep". That guess is why a zero-depth sample could keep a column of 0.0
# while its counts were imputed. Nothing is removed by deleting it: the guard
# below cannot fire, because after healing there are no all-zero samples left for
# `check_all_zero_samples` to find.
if checks["all_zero_taxa"]["has_all_zero_taxa"]
try
(healed_counts, healed_taxa_ids, heal_t, _) = heal_all_zero_taxa(filtered_counts, filtered_taxa_ids, dp, effective_epsilon)
(healed_counts, healed_taxa_ids, heal_t, _) = heal_all_zero_taxa(
filtered_counts,
filtered_taxa_ids,
dp,
effective_epsilon;
indices=all_zero_taxa_indices
)
if dp == DROP
keep_rows = [i for i in 1:size(prepared,1) if !(i in checks["all_zero_taxa"]["all_zero_taxa_indices"])]
# For ILR, prepared has n-1 rows, so need to handle differently — for stub, skip if ILR
Expand Down
Loading
Loading