NaN (or ArgumentError) from QGram distances for short strings #61

Activity

1. robertfeldt commented on Dec 20, 2022

   

   robertfeldt

   on Dec 20, 2022

   CollaboratorAuthor

   More actions

   Note that it goes even deeper since all but QGram returns NaN if both inputs are shorter than q:

   julia> filter(d -> isnan(d(2)("", "")), [QGram, Cosine, Jaccard, Overlap, SorensenDice, MorisitaOverlap, NMD])
   6-element Vector{DataType}:
    Cosine
    Jaccard
    Overlap
    SorensenDice
    MorisitaOverlap
    NMD
   
   julia> QGram(1)("", "")
   0
   
   julia> QGram(2)("a", "b")
   0
2. matthieugomez commented on Dec 22, 2022

   

   matthieugomez

   on Dec 22, 2022

   Last edited by matthieugomez

   Owner

   More actions

   I am not sure there is an issue with the current implementation. The way I think about it is that there is a formula for each Qgram distance (given in the docs), which is valid even when the set of qgrams is empty. In some distances, the length of qgrams appears in the denominator, which is why the distance returns NaN when the set of qgrams is empty.
3. robertfeldt commented on Dec 22, 2022

   

   robertfeldt

   on Dec 22, 2022

   CollaboratorAuthor

   More actions

   Mathematically I agree.

   I see dangers in actual use but ok, people will have to handle it themselves. I guess a simple solution might be to just highlight somewhere in the documentation that one can add a safe evaluate method like so:

   julia> function safeevaluate(D::Union{Cosine, Overlap, MorisitaOverlap}, s1, s2)
              length(s1) >= D.q && length(s2) >= D.q && return(evaluate(D, s1, s2))
              throw(ArgumentError("An argument is shorter than q ($(D.q)): \"$s1\", \"$s2\""))
          end
   safeevaluate (generic function with 1 method)
   
   julia> D = Cosine(2)
   Cosine(2)
   
   julia> @assert safeevaluate(D, "aa", "bb") == evaluate(D, "aa", "bb")
   
   julia> safeevaluate(D, "", "bb")
   ERROR: ArgumentError: An argument is shorter than q (2): "", "bb"
   Stacktrace:
    [1] safeevaluate(D::Cosine, s1::String, s2::String)
      @ Main ./REPL[2]:3
    [2] top-level scope
      @ REPL[5]:1
   
   julia> function safeevaluate(D::Union{Jaccard, SorensenDice, NMD}, s1, s2)
              (length(s1) >= D.q || length(s2) >= D.q) && return(evaluate(D, s1, s2))
              throw(ArgumentError("An argument is shorter than q ($(D.q)): \"$s1\", \"$s2\""))
          end
   safeevaluate (generic function with 2 methods)
   
   julia> safeevaluate(Jaccard(2), "", "")
   ERROR: ArgumentError: An argument is shorter than q (2): "", ""
   Stacktrace:
    [1] safeevaluate(D::Jaccard, s1::String, s2::String)
      @ Main ./REPL[11]:3
    [2] top-level scope
      @ REPL[12]:1
4. matthieugomez commented on Dec 22, 2022

   

   matthieugomez

   on Dec 22, 2022

   Owner

   More actions

   Sure. I'm also open to change things — following what other libraries typically do in this case. I will leave this issue open.
5. matthieugomez commented on Apr 8, 2026

   

   matthieugomez

   on Apr 8, 2026

   Owner

   More actions

   Fixed in 20aeac7
6. matthieugomez

   closed this as completedon Apr 8, 2026