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 Sun, 31 May 2015 01:47:07 GMT
FINALLY I am able to reproduce it locally. Looking through TC build I
noticed this:

Running NUnit-2.6.3 tests under .NET Framework v4.0 x86

Note x86... So instead of running test via Resharper and built in NUnit, I
ran it  with nunit 2.6.3 via command line. Tests fail with the odd float
issues if I run it with nunit-x86, and pass if I run it with nunit.exe
(both version 2.6.3). I am on a 64 bit machine, and so are the TC build
agents it seems.

I am still not sure why this causes the failures to occur, but do we need
to adjust what nunit build we use to run the tests?

On Sat, May 30, 2015 at 4:28 PM, Laimonas Simutis <laimis@gmail.com> wrote:

>
>
> 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