lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Laimonas Simutis <lai...@gmail.com>
Subject Re: lucenenet git commit: use proper float comparison
Date Sat, 30 May 2015 20:28:05 GMT
On Sat, May 30, 2015 at 4:01 PM, Itamar Syn-Hershko <itamar@code972.com>
wrote:

> And when you refactor _scorer.Score() to be in a different line it passes
> 100% of the time on all platforms? that doesn't sound right.
>

It continues to pass on mine (I can never get those to fail locally), and
ran the test several times on TC and it passed. I know, it sounds odd, I am
at a loss to explain it.


>
> Also, not in front of VS now, but AreEquals should already be doing this
> epsilon thing no?
>

That's what I thought too. The only odd thing there is no "float" overload
and only "double" so not sure if conversion from float to double might be
introducing rounding issues here too. That's why I replaced it with epsilon
just to see what would happen and it still failed so then I went with
precalculating scorer_.Score() before comparison just to see what would
happen.

And check this out. I put the comparison back like it used to be
(Assert.AreEquals) and wrapped in catch to output to console the values:

float skipToScore = scorer_.Score();
try
{
    Assert.AreEqual(skipToScore, scorer_.Score(), MaxDiff, "unstable
skipTo(" + i + ") score!");
}
catch (AssertionException ex)
{
    Console.WriteLine("Failed, these two were deemed not equal:");
    Console.WriteLine(skipToScore.ToString("R"));
    Console.WriteLine(scorer_.Score().ToString("R"));
    throw;
}

Look at the output on TC:

Test(s) failed.   unstable skipTo(3) score!
  Expected: 115019.984375d +/- 0.0010000000474974513d
  But was:  115019.98828125d

------- Stderr: -------
Failed, these two were deemed not equal:
115019.984
115019.984

You can see how the floats were converted to doubles and furthermore how
when I call Score() in catch section, it returns 115019.984 yet when it was
called in Assert it is outputting 115019.98828125d. and 0.988 and is off
from 0.984 by more than 0.001 (which is the value of MaxDiff).




>
> --
>
> Itamar Syn-Hershko
> http://code972.com | @synhershko <https://twitter.com/synhershko>
> Freelance Developer & Consultant
> Lucene.NET committer and PMC member
>
> On Sat, May 30, 2015 at 10:46 PM, Laimonas Simutis <laimis@gmail.com>
> wrote:
>
> > Itamar,
> >
> > These float comparison are killing me :) I am pretty sure all the
> remaining
> > failures in core tests are related to float issues.
> >
> > I am trying to use epsilon here by changing
> >
> > AreEqual(skipToScore, scorer_.Score(), MaxDiff) to
> > IsTrue(Math.Abs(skipToScore - scorer_.Score()) < MaxDiff).
> >
> > It is similar to the link you provided except I am not
> > handling infinite and values close to 0, which are not expected and do
> not
> > occur in this test.
> >
> > I can get this test to pass by taking out scorer_.Score() calculation and
> > calculating it separately and then comparing, like this:
> >
> > var secondScore = scorer_.Score();
> > IsTrue(Math.Abs(skipToScore - secondScore) < MaxDiff).
> >
> > In this case, the scorer_.Score() is doing a bunch of float adds  /
> > multiplies (
> >
> >
> https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Core/Search/DisjunctionMaxScorer.cs#L58
> > )
> > so I can see where rounding error could come in but still cannot explain
> > how it consistently fails on some env and not the others. Also have no
> idea
> > how to proceed with this issue besides changing the order of
> calculations,
> > like I did with the above to get it to pass. Just don't feel confident
> that
> > there is no bigger issue somewhere else.
> >
> >
> > Laimis
> >
> >
> >
> > On Sat, May 30, 2015 at 2:56 PM, Itamar Syn-Hershko <itamar@code972.com>
> > wrote:
> >
> > > Float comparison is not as trivial - you should probably use epsilon --
> > see
> > > http://stackoverflow.com/a/3875619/135701 for example
> > >
> > > --
> > >
> > > Itamar Syn-Hershko
> > > http://code972.com | @synhershko <https://twitter.com/synhershko>
> > > Freelance Developer & Consultant
> > > Lucene.NET committer and PMC member
> > >
> > > On Sat, May 30, 2015 at 9:50 PM, <laimis@apache.org> wrote:
> > >
> > > > Repository: lucenenet
> > > > Updated Branches:
> > > >   refs/heads/failingtests bdf2899a0 -> 6a81f8606
> > > >
> > > >
> > > > use proper float comparison
> > > >
> > > >
> > > > Project: http://git-wip-us.apache.org/repos/asf/lucenenet/repo
> > > > Commit:
> > http://git-wip-us.apache.org/repos/asf/lucenenet/commit/6a81f860
> > > > Tree: http://git-wip-us.apache.org/repos/asf/lucenenet/tree/6a81f860
> > > > Diff: http://git-wip-us.apache.org/repos/asf/lucenenet/diff/6a81f860
> > > >
> > > > Branch: refs/heads/failingtests
> > > > Commit: 6a81f860671ab98fb7cd595af317b3d8521acc21
> > > > Parents: bdf2899
> > > > Author: Laimonas Simutis <laimis@gmail.com>
> > > > Authored: Sat May 30 14:49:35 2015 -0400
> > > > Committer: Laimonas Simutis <laimis@gmail.com>
> > > > Committed: Sat May 30 14:49:35 2015 -0400
> > > >
> > > >
> ----------------------------------------------------------------------
> > > >  src/Lucene.Net.TestFramework/Search/QueryUtils.cs | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> ----------------------------------------------------------------------
> > > >
> > > >
> > > >
> > > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/lucenenet/blob/6a81f860/src/Lucene.Net.TestFramework/Search/QueryUtils.cs
> > > >
> ----------------------------------------------------------------------
> > > > diff --git a/src/Lucene.Net.TestFramework/Search/QueryUtils.cs
> > > > b/src/Lucene.Net.TestFramework/Search/QueryUtils.cs
> > > > index 1156eee..6615d4c 100644
> > > > --- a/src/Lucene.Net.TestFramework/Search/QueryUtils.cs
> > > > +++ b/src/Lucene.Net.TestFramework/Search/QueryUtils.cs
> > > > @@ -478,8 +478,8 @@ namespace Lucene.Net.Search
> > > >                          Assert.IsTrue(scorer_.Advance(i) !=
> > > > DocIdSetIterator.NO_MORE_DOCS, "query collected " + doc + " but
> > skipTo("
> > > +
> > > > i + ") says no more docs!");
> > > >                          Assert.AreEqual(doc, scorer_.DocID(), "query
> > > > collected " + doc + " but skipTo(" + i + ") got to " +
> > scorer_.DocID());
> > > >                          float skipToScore = scorer_.Score();
> > > > -                        Assert.AreEqual(skipToScore,
> scorer_.Score(),
> > > > MaxDiff, "unstable skipTo(" + i + ") score!");
> > > > -                        Assert.AreEqual(score, skipToScore, MaxDiff,
> > > > "query assigned doc " + doc + " a score of <" + score + "> but
> skipTo("
> > > + i
> > > > + ") has <" + skipToScore + ">!");
> > > > +                        Assert.IsTrue(Math.Abs(skipToScore -
> > > > scorer_.Score()) < MaxDiff, "unstable skipTo(" + i + ") score!");
> > > > +                        Assert.AreEqual(Math.Abs(score -
> skipToScore)
> > <
> > > > MaxDiff, "query assigned doc " + doc + " a score of <" + score + ">
> but
> > > > skipTo(" + i + ") has <" + skipToScore + ">!");
> > > >
> > > >                          // Hurry things along if they are going slow
> > (eg
> > > >                          // if you got SimpleText codec this will
> kick
> > > in):
> > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message