Skip to content

Call predictions positive at the shipped score (v1.0.7) - #11

Merged
lwaldron merged 2 commits into
mainfrom
call-shipped-threshold
Nov 9, 2026
Merged

lwaldron merged 2 commits into
mainfrom
call-shipped-threshold

Conversation

@lwaldron

@lwaldron lwaldron commented Nov 9, 2026

Copy link
Copy Markdown
Member

The 02 scripts called a predicted state positive only above a score of 0.8, while the export ships every state with a score of 0.5 or more. The MCC therefore described a stricter classifier than the one users get. This PR changes the call to 0.5 or more and re-runs the 02 and 03 steps and the summary on the v1.0.6 predictions (nothing re-predicted). Truth stays the curated state above 0.8.

Rank all: 36 of 41 discrete classes gain, median 0.03 (anaerobic 0.85 to 0.90, aerobic 0.65 to 0.68, motility 0.52 to 0.61, extreme environment 0.45 to 0.57; antimicrobial sensitivity 0.51 to 0.45). No class changes side of 0.7 at rank all; shape filamentous passes at genus. Numeric results unchanged.

docs/adr/0002 records the decision as Proposed, for Levi to accept. A follow-up copies the table into bugphyzz/inst/extdata.

Drafted with Claude Code.

The export keeps every imputed state with a score of 0.5 or more, so the
validation now calls positives the same way instead of at > 0.8. Same
predictions as v1.0.6, 02 and 03 steps re-run. ADR 0002 (Proposed).

Co-authored-by: Claude Code <noreply@anthropic.com>
@claude

claude Bot commented Nov 9, 2026

Copy link
Copy Markdown

Review notes, most important first:

  1. Binary tie at exactly 0.5 (discrete-binary/02_validation_binary.R, see inline comment). The new >= 0.5 cutoff makes ties between TRUE and FALSE rows possible. slice_max(with_ties = FALSE) resolves them by row order, so the call can change with file order. Make the tie-break explicit.
  2. No tests or checks for the new threshold. The MCC change is large (36 of 41 classes move) and is only justified in the PR description. A small fixture that checks the 0.5 boundary in both the binary and multistate scripts would catch regressions. The cutoff is also hard-coded in two places (02_validation_binary.R and 02_validation_multistate.R), so it would be better as one shared constant.
  3. Status mismatch. ADR 0002 is marked Proposed, but the README, validation_summary.tsv and the version number (v1.0.7) already treat the change as accepted. Either merge after the ADR is accepted, or state in the README that the new threshold is provisional.
  4. Bioconductor note. This repo has no exported R functions, so there are no man pages to check. The threshold constants are implicit magic numbers (truth > 0.8, prediction >= 0.5), and a named constant with a comment would make the asymmetry clear.

Comment thread discrete-binary/02_validation_binary.R Outdated
mutate(
Attribute = case_when(
grepl("TRUE$", Attribute) & Score > 0.8 ~ Attribute,
grepl("TRUE$", Attribute) & Score >= 0.5 ~ Attribute,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With the cutoff now >= 0.5, a taxon whose TRUE and FALSE rows both score exactly 0.5 ties in the later slice_max(..., with_ties = FALSE) at line 58. That keeps whichever row comes first in the file, so the call depends on row order. Make the tie-break explicit, for example arrange(desc(Score), desc(grepl("TRUE$", Attribute))) followed by slice_head(n = 1). Add a test for the exactly-0.5 case.

…nary calls

Co-authored-by: Claude Code <noreply@anthropic.com>
@lwaldron

lwaldron commented Nov 9, 2026

Copy link
Copy Markdown
Member Author
  1. Binary ties: pickBinaryCall() sorts the TRUE row first before slice_max(), so a tie at exactly 0.5 goes to TRUE, which is what the export ships. Re-running the 02 and 03 steps with the tie-break gives a byte-identical validation_summary.tsv, so no tie occurred in these folds.
  2. and 4. Both cut-offs are named constants in validation_calls.R (PRED_CALL 0.5, TRUTH_CALL 0.8, with a comment on why they differ) and used through callPredicted(), callTruth() and pickBinaryCall() in both scripts. test_validation_calls.R checks the boundaries and that the tie result does not depend on row order.
  3. Merge follows acceptance of ADR 0002; Levi asked for the change, and the ADR is for him to accept.

Drafted with Claude Code.

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.

1 participant