Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test-suite fails #47

Closed
Fuuzetsu opened this issue Oct 15, 2014 · 15 comments
Closed

Test-suite fails #47

Fuuzetsu opened this issue Oct 15, 2014 · 15 comments

Comments

@Fuuzetsu
Copy link
Member

 sum: [Failed]
*** Failed! Falsifiable (after 6 tests): 
fromList [-2.85685358419018,0.9288729747988657,33.51582034523111,18.079680881481565,-4.872267803281682]
(used seed -931204248)
  product: [Failed]
*** Failed! Falsifiable (after 6 tests): 
fromList [12.798552827341206,-4.694175866482457,-3.27888675478366]
(used seed -330711891)

http://hydra.nixos.org/build/15860082/nixlog/1/raw

The x86_64 box seems to have tested it fine.

@cartazio
Copy link
Contributor

likely https://ghc.haskell.org/trac/ghc/ticket/8195
given that the failure onlyhappens for Storable/Primitive/Unboxed Double vectors

@cartazio
Copy link
Contributor

does the problem go away when you build / test after configure with the ghc option -msse2?

@Fuuzetsu
Copy link
Member Author

I can't test, the log is from NixOS build bot.

@cartazio
Copy link
Contributor

honestly theres 2-3 things going on

  1. prior to the most recent releases of vector, the test suite wasn't even in the main cabal file! (so cabal test was a noop before)

  2. I'm not sure if those tests should be using exact equality on floating point!

testNumFunctions :: forall a v. (COMMON_CONTEXT(a, v), Num a) => v a -> [Test]
testNumFunctions _ = $(testProperties ['prop_sum, 'prop_product])
where
prop_sum :: P (v a -> a) = V.sum `eq` sum
prop_product :: P (v a -> a) = V.product `eq` product

is the offending line in question

  1. I suspect the tests would pass fine if compiled with
    cabal configure --enable-tests --ghc-options="-msse2" ; cabal test
    or the like

@Fuuzetsu
Copy link
Member Author

cc @peti who has Hydra powers

if -msse2 is needed for proper function of vector on i686 then perhaps vector should impose the flag by default?

@cartazio
Copy link
Contributor

the problem is more like the test should use some notation of approximate equality rather than exact,

though forcing -msse2 for the build would also work

@dolio
Copy link
Contributor

dolio commented Oct 15, 2014

-msse2 is needed for deterministic floating point arithmetic on 32-bit x86, period, it seems.

I'm not against sticking -msse2 on the test suite (or what have you) to force the issue, though (if we determine it's the best course of action). I just want to know that it actually fixes this problem before releasing a new package version. :)

@cartazio
Copy link
Contributor

ermmm, we'd only want to enable that for 32bit intel (if at all). I'd slightly favor pushing for some sort of approximate equality test. a la https://hackage.haskell.org/package/ieee754-0.7.3/docs/src/Data-AEq.html#AEq

though because the test suite should be deterministic per se, maybe the -msse2 route is the right solution for nix.

@cartazio
Copy link
Contributor

to repeat/clarify: the only way to get deterministic floating point computations done via an optimizing compiler on x86_32 is to compile all code to use the compiler's equivalent of -msse2. From that perspective, the test suite should certainly be run with -msse2 on applicable systems.

@peti
Copy link

peti commented Oct 15, 2014

Building the package with -msse2 fixes the test suite error on i686-linux: http://hydra.cryp.to/build/203733/log/raw

peti added a commit to NixOS/cabal2nix that referenced this issue Oct 15, 2014
peti added a commit to NixOS/nixpkgs that referenced this issue Oct 15, 2014
@Shimuuar
Copy link
Contributor

I think this is one of few cases where we do want floating point equality. We don't test numerical calculations but rather fact that perform same calculation in the same order. So it does make sense to check that two implementation of same algorithm produce same results. (assuming determinitsic FP arithmetics)

AEq type class is in my opinion misguided. There's no such thing as these two floating point numbers are approximately equal. It's always: these numbers are equal in context of given problem. And same pair of numbers could be considered equal or not equal depending on problem. Furthemore there's no single algorithm for comparing two numbers. For example |x-y| / |x| < ε breaks down when x crosses zero, and it's practical problem.

@cartazio
Copy link
Contributor

True, very true. Otoh, I think solution is basically just that on 32bit x86, the tests will fail if they're not built with -msse2. @dolio I think we can close this ticket, thoughts?

@Shimuuar
Copy link
Contributor

Yeah. We did same for the math-functions test suite.

@dolio
Copy link
Contributor

dolio commented Nov 17, 2014

Do we want to ad -msse2 to the test suite so that it can't fail except on systems that don't support it?

Also, is it just the test suite that needs to be built with -msse2? Or do both the tests and the main package need it?

@cartazio
Copy link
Contributor

yeah, i think its just the test suite that needs be built that way. (though maybe folks are using vector on realllly old systems, in which case ONLY the test suite should have that flag? I dont know how old the hardware people might have could be!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants