flintlib / arb

Arb has been merged into FLINT -- use https://github.com/flintlib/flint/ instead
http://arblib.org/
GNU Lesser General Public License v2.1
456 stars 138 forks source link

flint.h or flint/flint.h? #24

Closed bluescarni closed 8 years ago

bluescarni commented 10 years ago

Currently, some files in Arb (e.g., fmpr.h) include the FLINT headers as #include "flint.h". I am wondering if this should be #include "flint/flint.h" instead, as it seems that FLINT headers are commonly installed in a flint/ subdirectory (e.g., /usr/include/flint/flint.h on my setup).

The current way of including flint.h essentially prevents to compile files including the Arb headers without adding extra -I switches to the compiler command line. Or am I missing something?

fredrik-johansson commented 10 years ago

I just use extra -I switches. Including flint/flint.h would be less convenient for me, since I generally don't install flint.

bluescarni commented 10 years ago

I understand. I was just thinking that once Arb starts getting packaged by linux distro, it will not possible to just compile some code using Arb via

gcc my_code.c -lflint -lgmp -lmpfr -larb

but you will need to do

gcc my_code.c -lflint -lgmp -lmpfr -larb -I/usr/include/flint

Whereas with GMP, MPFR and FLINT code the first form would suffice. It's certainly not a big deal anyway :)

fredrik-johansson commented 10 years ago

Since this only affects the header files, I guess a configure hack to set the includes could work.

certik commented 10 years ago

I would strongly suggest to use #include "flint/flint.h". That way, no extra fiddling is necessary with the -I options, just the usual $PREFIX/include directory needs to be included. Most packages follow this idea. Some related links:

http://stackoverflow.com/questions/10692600/include-header-files-when-they-are-in-a-different-directory-structure http://stackoverflow.com/questions/24487667/including-header-file-locations-with-multiple-subdirectories

certik commented 10 years ago

But just to make sure, I have asked specifically for this case here: http://stackoverflow.com/questions/26468980/is-it-better-practice-to-include-flint-h-or-flint-flint-h

fredrik-johansson commented 10 years ago

As I mentioned above, using flint/flint.h means that I have to install flint to develop arb which is a hassle. I think it might also break building arb as an extension module of flint.

certik commented 10 years ago

@fredrik-johansson, you should look at the answers to my question at SO, they convincingly show that you should use flint/flint.h.

Ultimately, you'll have to decide if you want Arb to be a solid library that is easy for others to use. If so, it should follow the accepted practice.

You can use Hashdist to easily install and maintain various versions of Flint at once. That's what I use.

fredrik-johansson commented 10 years ago

The problem isn't installing flint once: it's that I develop flint concurrently, so I always want to work directly against the flint git repository. However, I'm well aware that I'm the only user doing this, which is why I'd be fine with a configure hack, say --local-flint-headers to include flint.h, with flint/flint.h included by default. There are very few instances of flint header files being included in arb header files, so I think the increase in complexity would be negligible.

certik commented 10 years ago

Sure, a configuration option would work.

fredrik-johansson commented 10 years ago

Another solution perhaps would be to install arb header files in flint/ as well.

certik commented 10 years ago

I would recommend to install the arb header files into its own location and in general to keep allowing arb to be installed as a separate package. Alternatively to simply merge the repository with Flint and call it Flint and always install it as part of Flint. Because currently some people install Flint without Arb (for example as a Debian package), so they can't easily then reinstall with Arb. The only way for them is to install Arb as a separate package.

certik commented 9 years ago

@fredrik-johansson any update on this one?

rickyefarr commented 9 years ago

Okay, I think I have a work around. If you agree @fredrik-johansson that it would work and be a suitable "hack", then I'll be happy to implement the idea.

At the end of the configure file, build a new header like so: echo "#ifndef ARB_DEPENDS_H__" > arb_depends.h echo "#define ARB_DEPENDS_H__" >> arb_depends.h echo "" >> arb_depends.h echo "#include \"$GMP_INC_DIR/gmp.h\"" echo "#include \"$FLINT_INC_DIR/flint.h\"" /* Do same for all other headers that are needed to build arb */ echo "#endif" >> arb_depends.h

Then, each .h file within the package would include arb_depends.h and never need to include flint.h, gmp.h, and all others.

What do you think?

SnarkBoojum commented 9 years ago

If I understand well what I have read on the flint-devel and sage-devel mailing-lists, FLINT and ARB will get more integrated -- in which case one can wonder if moving ARB within FLINT wouldn't be more sensible and solve every problem at once.

rickyefarr commented 9 years ago

If FLINT and ARB is to become more integrated in sage, I think that is a great idea.

Just to throw my two cents in, I think it would be nice for arf.h to be independent of arb.h, and arb.h to be independent of acb.h...etc. So in other words, arf type would be the base type for all other types, i.e. acb.h depends on arb.h which depends on arf.h. In other words, arf would be same as arb but without using ball arithmetic, arfc type could be created to mirror acb types without ball arithmetic. I think sage would benefit a lot from this type of design, since it would enable arf to compete with mpfr, and if the Arb library is faster than the mpfr library depending on the size of the precision...tune accordingly.

Correct me if I'm wrong @fredrik-johansson but I'm assuming most of the functions implemented make a calculation using the midpoint of the interval, and the error bounds are derived independently? If so, I don't think it would be very hard to separate. Is there a reason why this would not be possible?

fredrik-johansson commented 9 years ago

Error bounds are mostly derived independently, but it's quite likely that this will change, as performance can be improved by taking the error bounds into account when doing midpoint operations.

On the other hand, at least for basic arithmetic operations, there could still be separate functions that guarantee doing midpoint and radius operations independently.

I might add a complex arf type at some point, but it's not going to compete with MPFR... it's too much work to ensure correct rounding in all edge cases. This is difficult even for arithmetic operations on complex numbers.

It's mainly interesting to have because it can be used internally for things like polynomial root-finding and linear algebra, where you first compute an approximate numerical solution and then do a rigorous verification step to determine error bounds.

As to FLINT/ARB becoming integrated, I'm not so sure... FLINT has very infrequent releases, and I don't want to be blocked by that.

SnarkBoojum commented 9 years ago

FLINT has very infrequent releases, but that's also because it doesn't move fast... if the ARB part of it changes fast... then it might merit supplementary releases. That's something to discuss with William.

jdemeyer commented 8 years ago

The problem isn't installing flint once: it's that I develop flint concurrently, so I always want to work directly against the flint git repository. However, I'm well aware that I'm the only user doing this, which is why I'd be fine with a configure hack, say --local-flint-headers to include flint.h, with flint/flint.h included by default. There are very few instances of flint header files being included in arb header files, so I think the increase in complexity would be negligible.

How about you simply do

ln -s . flint

such that flint/flint.h is the same as flint.h?

Then you can safely apply #55.

rickyefarr commented 8 years ago

How about you simply do

ln -s . flint such that flint/flint.h is the same as flint.h?

Would this work for windows as well? I guess it would work for Fredrik, since I believe he uses linux.

Perhaps I'm wrong, but I think flint headers are set up the way they are because of Windows. Is that true?

I think that a solution would be to use Autoconf for the installation part of the code, but I'm not sure that would allow for installing Arb as a part of Flint...without changing the configuration file, a lot.

argriffing commented 8 years ago

I'm not a packaging expert but I think I agree with everyone saying to use flint/flint.h. Similarly, when I did not specify an explicit --prefix for arb, arb's make install spammed its headers directly into my /usr/local/include/. I'm pretty sure that's bad form. Also I think an installed arb conflicts somehow with make check.

fredrik-johansson commented 8 years ago

Wouldn't installing arb's headers into include/flint/ solve both problems?

certik commented 8 years ago

That might work, but it creates new problems as @argriffing correctly enumerates below. In my opinion it is the cleanest to install Arb header files into include/arb and Flint files into include/flint.

argriffing commented 8 years ago

Wouldn't installing arb's headers into include/flint/ solve both problems?

That seems weird. What if I'm installing arb to a different location than flint (for example /usr/ vs. /usr/local/)? What if I don't even have permission to write into the flint directory? Is having dependent projects install their header files into the directory of their dependency even a thing?

jdemeyer commented 8 years ago

Wouldn't installing arb's headers into include/flint/ solve both problems?

I don't think so. It's not because a file in local/include/flint/foo.h has #include "bar.h" that the preprocessor will look for local/include/flint/bar.h

jdemeyer commented 8 years ago

That might work, but in my opinion it is the cleanest to install Arb header files into include/arb and Flint files into include/flint.

+1

fredrik-johansson commented 8 years ago

Jeroen: it should work since that's what flint's internal includes already do. The (small) advantage of this solution is that it's compatible with the existing mechanism of installing arb as an extension of flint.

fredrik-johansson commented 8 years ago

So I discussed this with Bill and he agreed that it would be fine to drop support for arb as a flint extension module (he's not using that feature anymore). However, we should keep the ability to easily build arb against an uninstalled flint.

jdemeyer commented 8 years ago

Jeroen: it should work since that's what flint's internal includes already do.

You're right: relative include paths do work like that (this is for the case that you want to add the arb header files in the flint directory).

argriffing commented 8 years ago

Some comments in this thread mention packaging for Linux distros. I'm looking forward to that!

SnarkBoojum commented 8 years ago

I've already debian packages for ARB 2.7.0 available here: http://mentors.debian.net/package/arb

I'm mostly waiting to see how the dust settles on the issue of header files before I propose them officially.

argriffing commented 8 years ago

http://mentors.debian.net/package/arb

I noticed that there's already a debian package called arb. Does this mean Fredrik's arb would have to be proposed to Debian under a different name? Or if the existing arb is unmaintained non-free research software then maybe it could be dropped? I don't know much about Debian's processes except that they are very formalized.

For what it's worth, I've written a small domain-specific research code that depends on arb and uses autotools for build/install, and the arb/flint header issue has caused some complications. This issue causes make dist; make distcheck to fail, but if I were more clever at writing autoconf scripts then I suppose I could fix this on my end by digging through the user's file system by default at configuration time. My current approach is to require the user to pass the flint header directory to configure via CPPFLAGS.

SnarkBoojum commented 8 years ago

Hi,

Le lundi 18 janv. 2016 à 10:48:37 (-0800), argriffing a écrit :

http://mentors.debian.net/package/arb

I noticed that there's already a debian package called arb. Does this mean Fredrik's arb would have to be proposed to Debian under a different name? Or if the existing arb is unmaintained non-free research software then maybe it could be dropped? I don't know much about Debian's processes except that they are very formalized.

Yes, I proposed a 2.8.1 package just today, with the source package named flint-arb, and the binary packages named libflint-arb1 and libflint-arb-dev, to avoid the confusion.

That means code written to compile against arb will have to be patched on Debian, but I don't think it's reasonable to expect better : after all, collisions of three-letters acronyms are just normal...

Snark on #debian-science

fredrik-johansson commented 8 years ago

To reiterate, there seem to be two viable solutions for the header path issue:

  1. Change the arb source code to include flint/flint.h instead of flint.h
  2. Change the arb install script to install flint/arb.h instead of arb.h

Preferences?

SnarkBoojum commented 8 years ago

I'd prefer the arb headers in /usr/include/flint/ instead of directly in /usr/include, since some of them are just three-letter names...

certik commented 8 years ago

I think you should do:

3) include flint/flint.h and install all arb header files into arb/

Since Arb is a separate package from Flint, it makes no sense to install into flint/

Sent from my mobile phone. On Jan 19, 2016 7:28 AM, "Julien Puydt" notifications@github.com wrote:

I'd prefer the arb headers in /usr/include/flint/ instead of directly in /usr/include, since some of them are just three-letter names...

— Reply to this email directly or view it on GitHub https://github.com/fredrik-johansson/arb/issues/24#issuecomment-172869918 .

rwst commented 8 years ago

This now affects compilation of symengine.py with https://github.com/symengine/symengine/pull/787 installed, as well.

rwst commented 8 years ago

See also https://github.com/wbhart/flint2/issues/217

argriffing commented 8 years ago

Preferences?

I think @certik's option (3) makes sense.

rickyefarr commented 8 years ago

Just so everyone knows (please let me know if you've already planned to do this), I'm planning on packaging Arb for the Gentoo and funtoo linux distribution. Also, eventually I'll do the same for Arch linux. If you'd prefer me not to do this Fredrik, please let me know.

Figured I'd let you all know since there were discussion of deb packages.

Thanks,

Rick

On Mon, Feb 1, 2016 at 12:12 PM, argriffing notifications@github.com wrote:

Preferences?

I think @certik https://github.com/certik's option (3) makes sense.

— Reply to this email directly or view it on GitHub https://github.com/fredrik-johansson/arb/issues/24#issuecomment-178073734 .

Ricky Eugene Farr University of North Carolina at Greensboro Mathematics Department

fredrik-johansson commented 8 years ago

I've switched from flint.h to flint/flint.h in the git master, with a configure hack to create a symbolic link which still allows working against an uninstalled flint. Please test.

BTW, if someone wants to install headers to include/arb/arb.h instead of include/arb.h, I'll accept a patch that allows doing so optionally for now.

argriffing commented 8 years ago

Please test.

Works for me. I've reconfigured/rebuilt/reinstalled arb and rebuilt and tested a project that depends on arb. Everything seems to work and no longer requires the extra -I flags. The arb configuration output is

$ ./configure
FLINT_LIB_DIR set to /usr/local/lib
FLINT_INC_DIR set to /usr/local/include
Configuring...x86_64-Linux
Testing __builtin_popcountl...yes
Testing native popcount...yes
Testing __thread...yes
ARB was successfully configured.
fredrik-johansson commented 8 years ago

Closing this absent violent protests.

certik commented 8 years ago

I didn't test it, but thanks for fixing it @fredrik-johansson!