Repository navigation
Implement two-group ANCOM-BC and strengthen DA verification - #12
Conversation
Correct ALDEx2 log2 units and input boundaries. Add an independent paper-derived ANCOM-BC estimator, reference comparisons, scoped algebraic proofs and explicit release gates for MetaManifold issue 23. Draft pending full conformance and registration.
|
Caution Review failedThe pull request is closed. Summary
Merge Risk:
 |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fea949b74f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| input <- phyloseq::phyloseq(phyloseq::otu_table(reads, taxa_are_rows=TRUE), | ||
| phyloseq::sample_data(meta)) |
There was a problem hiding this comment.
On a clean oracle runner, the workflow installs only ANCOMBC, but this adapter unconditionally loads phyloseq; ANCOMBC 2.12.0 declares phyloseq only under Suggests, so the normal BiocManager::install(p) call does not guarantee it is present (Bioconductor package metadata). Consequently, the new ANCOMBC oracle and the documented reference adapter fail before running a comparison unless the environment happens to contain that optional package; install it explicitly or pass the matrix and metadata directly to ancombc.
Useful? React with 👍 / 👎.
| g = string.(groups) | ||
| any(isempty ∘ strip, f) && throw(CompositionError("feature names cannot be empty")) | ||
| any(isempty ∘ strip, g) && throw(CompositionError("group labels cannot be empty")) | ||
| allunique(f) || throw(CompositionError("feature names must be unique")) | ||
| levels = sort(unique(g)) |
There was a problem hiding this comment.
For numeric labels whose lexical and numeric orders differ, such as groups [2, 2, 10, 10], converting them to strings before sorting produces levels "10", "2". Both methods therefore report the opposite contrast and negate every directional effect relative to the previous numeric ordering (10 vs 2), which can invert the interpretation of an experiment. Determine group indices and order from the original labels, then stringify only the displayed or R-facing values.
Useful? React with 👍 / 👎.
| end | ||
|
|
||
| function CompositionalDA.ancombc_reference(counts::AbstractMatrix{<:Integer}, features::AbstractVector, | ||
| groups::AbstractVector; prevalence::Real=0.1, structural_zeros::Bool=false, |
There was a problem hiding this comment.
When a taxon is absent from one group, calling ancombc and ancombc_reference with their defaults compares different analyses: the native method defaults structural_zeros=true, removes that taxon from the bias fit, and reports undefined numerical statistics, while this wrapper defaults to false and asks R to analyze it numerically. The oracle tests explicitly pass false to both calls, so they do not expose this default-path disagreement; use the same default in both public APIs.
Useful? React with 👍 / 👎.
| groups::AbstractVector; prevalence::Real=0.1, structural_zeros::Bool=false, | ||
| negative_lower_bound::Bool=false, conserve::Bool=true, tolerance::Real=1e-7, | ||
| max_iterations::Integer=1000, runner=f -> f()) |
There was a problem hiding this comment.
The native API exposes pseudocount, and ANCOMBC 2.12.0 exposes the corresponding pseudo argument, but this wrapper neither accepts nor forwards it. Thus ancombc_reference(...; pseudocount=0.5) raises an unknown-keyword error, while omitting the keyword silently compares a native non-default run against R's default pseudo-count of 1; add the keyword, validate it, and pass it as pseudo=pseudocount (ANCOMBC reference manual).
Useful? React with 👍 / 👎.
| fit = ancombc_bias(moments.difference, moments.variance; tolerance, max_iterations) | ||
| d = length(kept) | ||
| effect, se, statistic, p = (fill(NaN, d) for _ in 1:4) | ||
| effect[tested] = moments.difference .- (conserve ? fit.wls : fit.bias) |
There was a problem hiding this comment.
With the default conserve=true, this switches the point estimate from the fitted middle-component/EM bias (fit.bias) to the WLS approximation (fit.wls). In original ANCOM-BC, the coefficient is corrected by the EM estimate regardless of conserve; the WLS estimator is used to approximate the EM estimator's variance, and conserve changes the standard error rather than the effect (ANCOM-BC paper). This changes reported effects and can change signs and significance whenever the two bias estimates differ; always subtract fit.bias here and reserve fit.variance for the conservative SE.
Useful? React with 👍 / 👎.
The existing ALDEx2 implementation used natural logarithms for descriptive columns, while the reference reports log2 values. This corrects those units and input boundaries, adds an independent implementation of the original two-group ANCOM-BC model from Lin and Peddada (2020), and supplies reference adapters and stronger comparison gates.
Related to JoshuaJewell/MetaManifold-WebUI#23. This does not close that issue. Package registration and the downstream integration remain separate requirements; no URL-pinned dependency is proposed.
Validation completed locally: Julia 1.12.5 package suite, 197/197 assertions including Aqua; Agda 2.6.4.3 / standard library 2.1 type-check of the expanded entry module. The new algebraic proofs concern posterior aggregation and bias cancellation/observability under explicit assumptions, not statistical calibration or floating-point correctness.
Draft gates: complete the expanded proof gate and both pinned Bioconductor comparisons (R 4.5.0, Bioconductor 3.22, ALDEx2 1.42.0, ANCOMBC 2.12.0), verify Julia 1.10, and resolve any disagreement without widening observed-failure tolerances. The local WSL environment became unresponsive during reference setup, so CI runs these gates independently. ANCOM-BC equivalence is not yet established. See docs/ANCOMBC-SPEC.md and docs/ISSUE-23-ACCEPTANCE.md for exact scope and omissions.