Stop class masking from repeating an identification when the species does not change#1377
Stop class masking from repeating an identification when the species does not change#1377mihow wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughClass masking gains an ChangesClass masking taxon-change control
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
✅ Deploy Preview for antenna-preview canceled.
|
64979a9 to
c5ab976
Compare
Masking shifts probability whenever a class the taxa list drops held any, so the run recorded a new terminal classification for nearly every prediction it examined. In one run on a deployment with real data, 948 of 3,814 recorded predictions (24.9%) named the same taxon as the prediction they replaced. Those appear in an occurrence's identification history as a repeat of the taxon already shown, each with its own Confirm control, and each stores another copy of the logits and scores arrays. `only_when_taxon_changes` (default True, exposed as an admin checkbox) records a re-score only when the winning taxon actually moves. The old behaviour is still available by unchecking it, which is what comparing confidence across taxa lists needs. Two details worth noting for review. The taxon comparison uses `taxon_id` rather than the related instance, because the scope query does not fetch the taxon and touching it would load one per row. The setting is deliberately not part of the masking algorithm's identity, unlike the reweight mode: it changes how many predictions are recorded, not what a recorded prediction means. Co-Authored-By: Claude <noreply@anthropic.com>
c5ab976 to
92d8318
Compare
✅ Deploy Preview for antenna-ssec canceled.
|
Summary
Class masking was recording a new identification for nearly every prediction it looked at, and most of those identifications named the species that was already there. Opening an occurrence showed the same species twice — once as the masked result and once as the intermediate prediction it replaced — each with its own Confirm button, which reads as a re-identification that never happened. In one run against real data, 948 of the 3,814 recorded results (24.9%) named the same taxon as the prediction they replaced.
This happens because masking moves probability around whenever a species the taxa list drops held any at all, and the existing check only skipped a prediction when the scores came back completely unchanged. Whether scores are re-weighted or not makes no difference; both settings produce the repeats.
Class masking now takes an option, on by default and exposed as a checkbox in the admin, to record a result only when the winning species actually changes. Runs write far fewer rows, and an occurrence's identification history only gains an entry when masking genuinely changed the answer. Unchecking it restores the previous behaviour, which is what you want when comparing confidence scores across taxa lists.
List of Changes
only_when_taxon_changesconfig field (default True) with an admin checkboxonly_when_taxon_changes=Falsethrough to the taskDetailed Description
Stacked on #1376; review that one first. This branch's own diff is the last commit.
Why the previous check did not catch these
The existing short-circuit compared the masked probability vector against the unmasked softmax and skipped only if they matched. That fires when every dropped class already scored about zero, which is rare — any dropped class holding real probability changes the vector, so a result was recorded even though the top species was untouched. The new check adds the condition that matters to a reader: did the winning species move?
Two decisions worth a look
The comparison uses
taxon_id, not the related object. The scope query does notselect_relatedthe taxon, so readingclassification.taxonwould fetch one row per classification — an extra query for each of the tens of thousands of rows in a large run. Comparing ids reads a column already present on the row.The setting is deliberately not part of the masking algorithm's identity. The re-weight mode is part of it, because the two modes persist different score semantics and a result's provenance has to be able to distinguish them. This setting only decides how many results get recorded; a result recorded under either setting means the same thing, so both resolve to the same
Algorithmrow.test_both_skip_settings_share_one_masking_algorithmpins that.On existing data
Results already written are left alone; this only affects future runs. If the repeated entries on already-masked occurrences should be cleaned up, that is a separate data-management pass and I am happy to write it — worth deciding before this ships, since it determines whether the option's default matters for what is already in the database.
Testing
ami.ml.post_processingandami.ml, including five new ones.black,isortandflake8clean.test_new_classification_when_the_taxon_changespins the transition masking exists for, so a guard that always skipped would fail.test_occurrences_updated_counts_only_changed_determinationsnow runs with the option off, because it is testing the metric rather than the option, and needs the re-scored occurrence to reach the counting step.Summary by CodeRabbit
New Features
Bug Fixes