basvandijk / scientific

Arbitrary-precision floating-point numbers represented using scientific notation
BSD 3-Clause "New" or "Revised" License
73 stars 40 forks source link

scientific cannot be used with alternative Integer implementations #15

Closed ygale closed 10 years ago

ygale commented 10 years ago

GHC allows using an implementation for Integer that is different than the default implementation based on GNU GMP. scientific is becoming a common dependency - because it's a great library, thanks! - but at the same time, it is also becoming increasingly common to use alternative Integer implementations. So scientific should have a cabal flag that allows bypassing the efficient implementation in Math.NumberTheory.Logarithm which depends on GMP internals, and using instead a more naive implementation that only uses the generic Integer API.

ygale commented 10 years ago

The original motivation for allowing a non-GMP implementation of Integer was to avoid GPL encumberment. But now it has become clear that there are serious technical reasons why GMP is often very problematic.

For example, the webkitgtk3 library depends indirectly via FFI on libgnutls, which uses libgmp for crypto. Since libgmp uses a global static function pointer for replacing malloc for allocation, and GHC needs to do that, use of TLS via Webkit results in segfaults if you are using the GMP-based Integer implementation.

ygale commented 10 years ago

See also ekmett/lens#454.

k0ral commented 10 years ago

See pull request https://github.com/basvandijk/scientific/pull/18 .

ygale commented 10 years ago

Hmm. Thanks, but it's not just the dependency in the cabal file. The code itself has hard-coded dependencies on GMP internals.

After looking into this further, it seems to me that this whole part of the scientific package, which implements Daniel Fishcer's algorithm for integer logarithms, isn't needed at all. About three years ago, Daniel integrated his algorithm directly into both the integer-gmp and integer-simple packages. (It was needed for the new faster implementations of toRational and fromRational.) It looks like this has been part of GHC at least since 7.2. See GHC Trac ticket 5122.

So instead of that whole big hard-coded GMP-based implementation of the function Math.NumberTheory.Logaritms.integerLog10' in this package, why not just use the function GHC.Integer.Logarithms.integerLogBase# instead? That is Daniel's implementation of his own algorithm. It is available, with the same type signature but different internal implementation details, both in integer-simple and in integer-gmp.

With that additional change, plus lower bounds on the integer-simple and integer-gmp dependencies to ensure that we get the log function, I think #18 would work. It would probably be a good idea also to add a few more tests to the test suite though ;)

basvandijk commented 10 years ago

I merged #18 and did some related work.

@ygale & @k0ral can one of you test whether -finteger-simple works for you. I don't have a GHC handy with integer-simple.

And thanks for your efforts so far!

k0ral commented 10 years ago

This works fine on my side, I'm using GHC 7.8.2 built with integer-simple. As expected, the -finteger-simple flag doesn't need to be explicitly set, cabal automatically sets it when needed. Thank you !

basvandijk commented 10 years ago

Great, thanks for testing!

ygale commented 10 years ago

For the record:

@basvandijk left in the integer log implementation in this package, but only for use in older versions of GHC. It appears that scientific now works in all of the following cases:

For GHC >= 7.2, scientific now uses the integer log implementation from either integer-gmp or integer-simple, and works with either one of those.

For GHC < 7.2, scientific still works as before with GHC's built-in GMP-based Integer.

This is great work; thanks!

basvandijk commented 10 years ago

Almost, __GLASGOW_HASKELL__ < 702 actually means GHC < 7.2 instead of GHC < 7.0.2. Yes, I know, very confusing.

ygale commented 10 years ago

Oops, sorry about that. Yes, it's documented here. I fixed my comment above, "for the record".

quyse commented 7 years ago

Hi, scientific now have integer-logarithms as a dependency, which in turn depends on integer-gmp unconditionally, so now again scientific cannot be used with integer-simple. May it be fixed somehow?

basvandijk commented 7 years ago

Hi @quyse, thanks for the heads-up. @phadej could you make integer-logarithms work with integer-simple. I'll open a issue for it.

BTW as explained in this thread on the Haskell libraries mailinglist, I'm thinking of publishing an integer package which reexports the common parts of integer-simple and integer-gmp. This should make it easier to define packages like integer-logarithms and scientific because those then don't need to add cabal flags.

phadej commented 7 years ago

I can. On it.

phadej commented 7 years ago

@quyse can you test whether your setup works with https://github.com/Bodigrim/integer-logarithms/pull/2 ?

phadej commented 7 years ago

@basvandijk for the record: http://hackage.haskell.org/package/integer-logarithms-1/docs/GHC-Integer-Logarithms-Compat.html seems to be one of the module you want from integer. OTOH it has to do shim on GHC-7.0, cannot be just re-export.

(I'm :+1: for integer to re-export GHC.Integer, but I'm not sure how compatible apis there actually are across versions / integer-gmp/integer-simple)

quyse commented 7 years ago

@phadej Seems working! I checked with stack that scientific-0.3.4.10 and integer-logarithms from your PR are built successfully with --ghc-variant integer-simple --flag scientific:integer-simple --flag integer-logarithms:-integer-gmp. Thank you!

phadej commented 7 years ago

@quyse http://hackage.haskell.org/package/integer-logarithms-1.0.1 here you go

basvandijk commented 7 years ago

Thanks @phadej and @quyse!