lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Currens <currens.ch...@gmail.com>
Subject Re: lucenenet git commit: use proper float comparison
Date Sun, 31 May 2015 04:03:25 GMT
The .NET jitter emits different code to handle floating point instructions
in x86 vs x64. At least on my machine, I noticed that the native assembly
code generated by the jitter when running in x86 uses the x87 extensions
for floating point and in x64 it uses SSE. I believe that this is only an
issue when dealing with single-precision floating point numbers, which are
used pretty much everywhere in search. The reason is because the x87
extensions, by default, use 80-bit double-extended precision internally
(thanks, Wikipedia!) whereas x64 uses single-precision instructions (and
thus the mantissa is truncated) which means we'll get different results
between the two architectures.

Resharper defaults to x64. If I use the NUnit Test Adapter and run the unit
tests using visual studio directly, which runs in 32-bit mode, I can get
the tests to fail almost all the time.

This is a good catch. I'm not sure if we should change nunit to be x64
necessarily. It's possible that this is exposing a real code issue
somewhere, or at least an inconsistency in behavior between .NET and Java.
I think I might pull down the java code and see if there's a difference in
this test between a 32-bit and 64-bit JVM. I don't know what kind of
assembly instructions that are emitted by Java's jitter.

-Christopher

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

> 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