Two Naive Bayes classifiers treat an empty class value as a class; their parent skips it

Lucene's classification module has four Naive Bayes classifiers, and each reads the list of classes from the terms of the class field. SimpleNaiveBayesClassifier and BM25NBClassifier skip a term of length 0; their sibling implementations CachingNaiveBayesClassifier and SimpleNaiveBayesDocumentClassifier, both subclasses of SimpleNaiveBayesClassifier, do not. So on a training index where some documents store an empty string as their class value (a blank label column indexed as it is), the two subclasses list '' as a class and can assign it, while their parent returns a real class: with fruit "apple banana", vehicle "car truck" and one blank-labelled "car bus" document, assignClass("car bus") is vehicle from the parent and BM25NB and '' from both subclasses. There is no error, and leaving the class field out instead makes all of them agree. The divergence appeared in 6.0.0: the 2015 commit that added the parent's check also made the caching class's own, unguarded code path live, and the document classifier was added seven weeks later with a copy of the loop without the check. The module is @lucene.experimental and the trigger is rare. Which direction is right (skip '' everywhere or treat it as a class everywhere) is open; adding the parent's check to the two subclasses (+7/-1 lines) was validated against the whole classification test task.

• Class field: the field whose indexed terms are the classes (classFieldName). A StringField("cat", "") is indexed as a term of length 0, the empty class term '' (the independent reproducer checks, on its own index with two blank-labelled documents, that the empty term has docFreq 2 in its class field).
• The Naive Bayes family: SimpleNaiveBayesClassifier (the parent), its subclasses CachingNaiveBayesClassifier ("with caching feature") and SimpleNaiveBayesDocumentClassifier (classifies a Document), and the separate BM25NBClassifier. All four are @lucene.experimental.

1. SimpleNaiveBayesClassifier.assignClassNormalizedList (SimpleNaiveBayesClassifier.java:136-158) enumerates the class-field terms and scores each as a class; at :147 it skips a zero-length term with if (next.length > 0). BM25NBClassifier does the same (BM25NBClassifier.java:128-129).
2. CachingNaiveBayesClassifier overrides assignClassNormalizedList (CachingNaiveBayesClassifier.java:83-93) and scores the classes in its list cclasses (:99-101, :108). reInitCache, called from the constructor (:76), fills that list with every class term and no length check (:236-241).
3. SimpleNaiveBayesDocumentClassifier.assignNormClasses (document/SimpleNaiveBayesDocumentClassifier.java:105-136) loops over every class term (:118) and adds a result for each (:132), with no length check.
4. So the parent and BM25NB see the classes {fruit, vehicle}; the two subclasses see {'', fruit, vehicle}. "car" occurs under vehicle and under '', so '' ties with vehicle; "bus" occurs only under '', so for "car bus" '' wins. A second blank-labelled document keeps these class sets and winners; only the scores change.

Training documents: "apple banana" = fruit, "car truck" = vehicle, plus one or two "car bus" with class ""; StandardAnalyzer, default IndexWriterConfig.

KNearestNeighborClassifier also returns '' on the same training documents when the class field is stored (['', 'vehicle'], assigns ''); it reads stored values, not the Naive Bayes class loop, and is not part of this bug.

A side note that fix A leaves as is: the parent's countDocsWithClass (SimpleNaiveBayesClassifier.java:166-184) uses Terms.getDocCount() of the class field, which counts blank-labelled documents too, so even in the parent a '' document is not equivalent to one without a class field (its assigned class differed in 1,387 of 55,500 seeded cases in the finder's runs).

• Who. Applications that classify with CachingNaiveBayesClassifier or SimpleNaiveBayesDocumentClassifier (per the prior-report search, Solr's classification update processor uses the latter for its "bayes" algorithm, SOLR-7739 / SOLR-8871) on a training index in which some documents carry an empty string as their class value: a blank label column indexed as it is, StringField(cls, ""), or an empty value under KeywordAnalyzer.
• Effect. getClasses returns an extra class '' and assignClass can return '' where the parent returns a real class. Under the reading that '' means "unlabelled" that is a wrong label; under the reading that '' is a class, the parent and BM25NB are the ones dropping it. Either way, swapping one Naive Bayes classifier for another (for example the caching one, for speed) changes the class set. Nothing fails or is logged.
• Default vs opt-in. The classification module is opt-in and @lucene.experimental; nothing else needs configuring (default IndexWriterConfig). No codec dependency was seen or is expected: the class loops use only the generic Terms/TermsEnum API (the gradle runs used the test framework's Asserting(Lucene104) codec; no codec matrix was run).
• Scale. None: three documents show it, deterministically.
• How likely. Rare: Lucene's own fixtures mark unlabelled documents by leaving the class field out (ClassificationTestBase.java:227, :333), and nobody has reported it in about ten years. When it happens, it happens on every call.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: CachingNaiveBayesClassifier.getClasses("car") expected:<['fruit', 'vehicle']> but was:<['', 'fruit', 'vehicle']>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:120)
       org.apache.lucene.classification.TestCachingNaiveBayesEmptyClassValue.testSameClassesAsSimpleNaiveBayes(TestCachingNaiveBayesEmptyClassValue.java:51)

• Documentation. CachingNaiveBayesClassifier is "A simplistic Lucene based NaiveBayes classifier, with caching feature" (CachingNaiveBayesClassifier.java:38) and SimpleNaiveBayesDocumentClassifier "A simplistic Lucene based NaiveBayes classifier" (document/SimpleNaiveBayesDocumentClassifier.java:45); both extend SimpleNaiveBayesClassifier. LUCENE-5736 (#6798), which added the caching class, calls it "a cache included version of the SimpleNaiveBayes classifier". The open #8723 treats a caching-versus-parent difference as a bug (another cause: no log prior).
• Tests. No upstream classification test indexes an empty class value; the fixtures leave the class field out for unlabelled documents.
• History. In every 5.x release the caching class's own assignClassNormalizedList was private and never called, so it returned its parent's results (5.0.0, 5.3.2, 5.4.0, 5.5.5 executed: consistent). 8b2e0d937d5 (LUCENE-6479, 2015-09-02, "improved cm testing, added stats, minor fixes") added if (next.length > 0) to the parent and, in the same diff, changed both assignClassNormalizedList methods (the parent's and the caching class's) from private to protected, so the caching one overrides the parent's and its class list, filled without the check since 2014, went live. 71cea88773e (LUCENE-6631, 2015-10-20) added the document classifier with the loop but not the check; two later commits rewrote that loop and kept it unguarded (9534904c250, c2c2e8a85e9), and 8808cf53735 edited the class without adding it. BM25NBClassifier (LUCENE-7823, 2017) copied the check. No commit, CHANGES entry or discussion explains the difference.

The strongest works-as-designed case (the refute-lens cross-model reviewer and the devil's advocate): getClasses promises "all the classes" (Classifier.java:40, document/DocumentClassifier.java:43); the classFieldName javadoc says "the returned class will be a token indexed for this field" (SimpleNaiveBayesClassifier.java:82-83), and '' is such a token, which IndexWriter accepts. KNearestNeighborClassifier returns and assigns '' on the same training documents with the class field stored, and ConfusionMatrixGenerator accepts '' as a correct answer (utils/ConfusionMatrixGenerator.java:80, :92-94). The parent's check is undocumented, the caching classifier already differs from its parent (#8723), and the module is experimental. So the subclasses may be right and the parent's check the odd one out.

What it shows, and what it does not: it is a strong case that '' can be a legitimate class (so fix B is a real option) and a moderate case that blank labels stored as "" are the application's data-quality problem. It does not show that the subclasses are meant to see a different class set from their parent on one index: before 6.0.0 they did not, and the change arrived as a side effect of a "minor fixes" commit. Between 2015 and 2017 the module's author treated '' as "no class" in three places: ConfusionMatrixGenerator (2015-05 to 2015-10), the parent (2015-09) and the new BM25NB (2017-05). Verdict: the divergence is not intended; whether '' is a class, and so the fix direction, is open.

Two directions make the family consistent; both were built and run in a scratch worktree at a9779ef3c193. Fix A is recommended: it is the smallest change, it touches only the two classes that differ, and it matches the behaviour of SimpleNaiveBayesClassifier and BM25NBClassifier. A third idea, also leaving blank-labelled documents out of the document count, was not written or run.

reInitCache adds a class term to cclasses only when its length is above 0, and assignNormClasses skips a zero-length class term, as SimpleNaiveBayesClassifier.java:147 does. +7/-1 lines in CachingNaiveBayesClassifier.java and document/SimpleNaiveBayesDocumentClassifier.java.

// CachingNaiveBayesClassifier.reInitCache
while ((termsEnum.next()) != null) {
  // like SimpleNaiveBayesClassifier, skip the empty term: it is not a class
  if (termsEnum.term().length > 0) {
    cclasses.add(BytesRef.deepCopyOf(termsEnum.term()));
  }
}
// SimpleNaiveBayesDocumentClassifier.assignNormClasses
while ((c = classesEnum.next()) != null) {
  if (c.length == 0) {
    continue; // like SimpleNaiveBayesClassifier, skip the empty term: it is not a class
  }

Trade-offs

• Changes results only on indexes with '' class values, and only for the two subclasses; restores the caching classifier's 5.4.0-5.5.5 class set (the empty-term filtering only; in 5.x the parent's own scoring, with its prior, ran) and matches the parent and BM25NB.
• The scores are normalized over the class list, so dropping '' also changes the scores of the remaining classes (for "car" the subclasses' 0.40/0.20 become the parent's 0.67/0.33, executed) and can change the assigned class on any index with '' values.
• Leaves countDocsWithClass alone: blank-labelled documents still count in the document total, as in the parent, so a '' document is still not exactly the same as one without a class field.
• The caching classifier's scores still differ from the parent's for the separate #8723 (no log prior); only the class set is aligned.
• Anyone who relied on '' as a class from these two classes loses it (no such use was found).

What the checks validated

Gradle in a scratch worktree at a9779ef3c193, seed DEADBEEF: the whole :lucene:classification:test task with the three repro classes added, 15 classes, 51 tests, 0 failures, 1 skipped (the @Monster 20-newsgroups test, Test20NewsgroupsClassification.java:71), with assertions on and off; the 6 repro tests that fail on stock pass. :lucene:classification:check -x test with the fix and the primary test (google-java-format, ecj lint, forbidden APIs): BUILD SUCCESSFUL. The repro runner: the primary test passes 3 of 3 with the finder's equivalent fix classes (the same two checks) shadowed; the finder's equivalent fix also makes the finder's and the independent reproducer's tests pass.

Drop the if (next.length > 0) wrapper at SimpleNaiveBayesClassifier.java:147 and BM25NBClassifier.java:129 (+9/-14 lines, 2 files).

Trade-offs

• Consistent with KNearestNeighborClassifier, ConfusionMatrixGenerator, the getClasses javadoc ("all the classes") and the 5.0.0-5.3.2 behaviour.
• Changes the results of the two classifiers that agree today, for every index with blank labels: their getClasses gains '' and assignClass can return it. Users who index blank labels meaning "unlabelled" keep getting '' back.

What the checks validated

Gradle, same worktree, assertions on only: the primary test passes 3 of 3 and the 42 upstream tests that run pass (51 tests, 4 failures: the finder's and the independent reproducer's tests, which hard-code fix A's direction, now fail at the parent's own assertions: two because the parent now offers '', two because it now assigns '' instead of vehicle). On the repro index the parent and both subclasses then assign '' to "car bus", while BM25NBClassifier, which scores differently, still assigns vehicle. Not run with assertions off; not lint-checked.

Under the reading "'' = unlabelled", countDocsWithClass (SimpleNaiveBayesClassifier.java:166-184, inherited by the parent and both subclasses) could also exclude documents whose only class value is '', so a blank label behaves exactly like a missing class field.

Trade-offs

• Changes the parent's results too (its assigned class differed between a '' document and a document without a class field in 1,387 of 55,500 seeded cases in the finder's runs).
• Needs a per-document check that Terms.getDocCount() cannot give. Not written or run: raise it as a follow-up question.

Method: bounded-exhaustive translation validation against reference models (the hunt's "semi-formal" tier).

• The harness built every training set of 1-2 small documents plus one document whose class value is '', and compared each classifier's getClasses and assignClass with an exact reference model of the same formula.
• Each variant equals its own reference (the caching one: no prior, every class term). The caching classifier's class set differed from SimpleNaiveBayesClassifier's in all 420,616 such cases and it assigned '' in 249,898; the document classifier assigned '' in 554,608 of its 1,682,464 cases (which also vary boosts and 1-2 values per field), and in the 56,840 one-field, one-value, boost-1, one-token cases where it is directly comparable its class set differed from the parent's.
• A metamorphic run (55,500 seeded cases) also showed that the parent answers differently for a '' document than for the same document without a class field in 1,387 cases (the document-count side note).
• The finder then wrote the public-API JUnit test; independent checkers reproduced it.