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

Update FLINT to 2.3 #12173

Closed
mwhansen opened this issue Dec 17, 2011 · 443 comments
Closed

Update FLINT to 2.3 #12173

mwhansen opened this issue Dec 17, 2011 · 443 comments

Comments

@mwhansen
Copy link
Contributor

Install:

Apply:

Apply to SAGE_ROOT:

Upstream bug with OS X PPC: https://groups.google.com/forum/?hl=en&fromgroups#!topic/flint-devel/KrgY0v09SwI

Dependencies: sage-5.9.beta5

Upstream: Reported upstream. Developers acknowledge bug.

CC: @jpflori @sagetrac-spancratz @williamstein @nexttime

Component: packages: standard

Keywords: flint spkg

Author: Mike Hansen, Fredrik Johansson, Jean-Pierre Flori

Reviewer: John Cremona, Jeroen Demeyer, Burcin Erocal

Merged: sage-5.10.beta3

Issue created by migration from https://trac.sagemath.org/ticket/12173

@mwhansen mwhansen added this to the sage-5.6 milestone Dec 17, 2011
@mwhansen
Copy link
Contributor Author

comment:1

Building flint2 in Sage:

@mwhansen
Copy link
Contributor Author

Attachment: ulong_extras.patch.gz

@fredrik-johansson
Copy link
Contributor

comment:2

Attachment: zmod.patch.gz

zmod.patch replaces references to zmod_poly with nmod_poly and (hopefully) fixes places where the interface of a function changed. Completely untested at this point, of course.

Some remarks:

I deleted the zmod_poly_pow function implemented in Sage, since FLINT now provides such a function.

There is no longer a _nmod_poly_set_coeff_ui, which was used in one place (creating a polynomial from a list). I replaced it with nmod_poly_set_coeff_ui rather than manipulating coefficients directly, which is a bit slower, but shouldn't matter that much.

nmod_poly_resultant needs to be added in FLINT 2. If we don't get it done, the code calling it could just be commented out to fall back to the default Sage code that already handles the non-prime case.

I didn't change any filenames or Sage types (so the corresponding Sage files/functions/types are all still called zmod_poly).

The new pxd file uses the GMP mp_limb_t (etc) types rather than unsigned long (etc). Some search and replace might be needed, or importing the types from gmp.h.

@fredrik-johansson
Copy link
Contributor

comment:3

Attachment: fmpz.patch.gz

Added another patch for fmpz_poly functions.

fmpz_poly.pxi still needs rewriting, and I didn't touch the cases in polynomial_rational_flint.pyx (since that file needs fmpq_poly fixes as well).

@sagetrac-spancratz
Copy link
Mannequin

sagetrac-spancratz mannequin commented Feb 28, 2012

comment:5

I just want to let you know that I have incorporated the current state of progress into an installation of Sage at

/scratch/spancratz/sflint

on sage.math.washington.edu. Step by step, I have gotten rid of some of the problems, but I didn't have enough time last week to finish this. By all means, please go ahead and continue working on this copy of the work.

Best wishes

Sebastian

@jpflori
Copy link
Contributor

jpflori commented Feb 28, 2012

comment:6

That might be a stupid question, but how do I access your working copy?

Should I ask for an account on the mentioned machine (to whom?)?

@roed314
Copy link
Contributor

roed314 commented Mar 4, 2012

comment:7

You should e-mail [email protected] and say you're working on getting FLINT into Sage. He'll give you an account on sage.math.washington.edu

@jpflori
Copy link
Contributor

jpflori commented Mar 29, 2012

comment:9

Was the directory lost after the recent sage.math problems ?
I can't locate a spancratz directory on /scratch...
I'll have a look at the google groups in case I can find more info there.

@fredrik-johansson
Copy link
Contributor

comment:10

Yes, it seems to be gone.

@jpflori
Copy link
Contributor

jpflori commented Mar 29, 2012

comment:11

Any chance that one of you backed it up somewhere else ?

@roed314
Copy link
Contributor

roed314 commented Apr 4, 2012

comment:12

Sebastian says he doesn't have a backup.

@fredrik-johansson
Copy link
Contributor

comment:13

This new spkg and patch should apply to a clean install of sage-5.0, and almost completely allows building Sage against flint-2.3:

http://sage.math.washington.edu/home/fredrik/flint-2.3.spkg

http://sage.math.washington.edu/home/fredrik/flint-2.3-sage-5.0.diff

There is one small remaining issue building Sage: at one point, the definition of ulong in zn_poly.h apparently clashes with the one in flint. A quick workaround for development purposes is to add #ifndef ulong ... #endif in sage/local/include/zn_poly.h. I don't know the best proper fix. But with the workaround,

sage -f flint-2.3.spkg

sage -b

should work.

When starting Sage, there is a link time error (finding fmpz_set_ZZ), so it seems that the NTL interface is not being built or included correctly. Presumably, fixing that, Sage will start up successfully, though most likely there will be some bugs to hunt down...

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:14

I've had a very quick look and the spkg needs major simplification.
Most of the logic which was present is now in the flint configure script.
I'll try to provide an updated version.

I see that there was some restriction in the previous spkg-install reported by Michael Abshoff on an "US IIIi" with GCCC 4.3.2 which is not reflected in the configure script.
Some reason for this ?

To answer my own question, by default -funroll-loops is not included by default, which covers the solaris case.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:15

I've just reuploaded fredrik patch as an attachment so that it can be easily viewed in trac.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

Attachment: flint-2.3-sage-5.0.patch.gz

Fredrik's patch.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:16

The NTL problem is just that interfaces/NTL-interface.lo is not included into libflint anymore.
The configure and Makefile.in files should be modified.

There was some partial magic added here (*_EXTRAS):
flintlib/flint@64b5b59
but removed here:
flintlib/flint@0103ccb
if I'm not wrong.

Violently changing the Makefile.in to include NTL-interface.lo solves the problem, although this should certainly be included upstream rather than here.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:17

Now sage launches but I get a crash when quitting due to an undefined symbol flint_stack_cleanup.
I'll have a look later.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:18

Another complication might be that the headers used to be installed into $SAGE_LOCAL/include/FLINT which is quite meaningful because there are a lot of them.

It would be nice to use the install target, but it would put everything into $S_L/include/.

Somebody has a thought about that?
Modify flint Makefile? Install the headers into $S_L/include/ and modify Sage source (mainly the module thing at the top of Sage and flint*.px* files)? Continue to copy the files by hand?

@fredrik-johansson
Copy link
Contributor

comment:19

The call to flint_stack_cleanup() should be replaced with a call to _fmpz_cleanup(), I think.

I agree that it would be nice to keep the header files in a subdirectory. Dunno what the best solution is.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:20

Replying to @fredrik-johansson:

I agree that it would be nice to keep the header files in a subdirectory. Dunno what the best solution is.

My prefered solution would be that make install puts them in a separate directory by default.
I'll ask on flint-devel to get bill's preference.

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:21

Replying to @fredrik-johansson:

The call to flint_stack_cleanup() should be replaced with a call to _fmpz_cleanup(), I think.

After quickly looking at the memory management in flint 1.* and flint 2.3, this seems the right solution to me (without real insight nonetheless) and indeed fixes the problem!

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:22

Now I get a LOT of segfaults.
For example, running:

sage: GF(4, 'a')

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:23

Two things :

  • the order of the structure changed, now exp is before p, not sure this is important...
  • n_factor_t has to be initialized.

With these changes I can create GF(4)!

@jpflori
Copy link
Contributor

jpflori commented May 24, 2012

comment:24

The function n_factor is also called in descent_two_isogeny.

@jpflori
Copy link
Contributor

jpflori commented May 25, 2012

comment:25

Running make ptest raises a limited number of problem.
The first one I saw is quite bad: I get time out caused by some segfaults...

Apparently, something is now deallocated twice (I get a double linked blah from glibc, more precisely "*** glibc detected *** python: corrupted double-linked list: 0x00... ***").
(This is not caused by the _fmps_cleanup() call, it also happens without it)

This is not sytemcan be reproduced by testing ell_tate_curve.py for example.

@jpflori
Copy link
Contributor

jpflori commented May 25, 2012

comment:26

Aa far as the other bugs are concerned, it seems that #6919 is back???
At least, the same doctest fails.

@kiwifb
Copy link
Member

kiwifb commented May 8, 2013

comment:330

Needs to pass "-fno-common" on darwin. http://gcc.gnu.org/ml/gcc/2005-06/msg00375.html and follow up is interesting on that subject.

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

comment:331

OK, I am making and testing a new spkg. Please leave yours alone in order to avoid duplicate work.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

Diff flint-2.3 -> flint-2.3.p1

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

comment:333

Attachment: flint-2.3.p1.diff.gz

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

Spkg diff, for review only.

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

comment:334

Attachment: flint-2.3.diff.gz

The new spkg with -fno-common at least builds on OS X 10.4 PPC G5. I'll need some more time to actually completely test it.

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

Upstream: Not yet reported upstream; Will do shortly.

@jdemeyer
Copy link
Contributor

jdemeyer commented May 8, 2013

comment:336

FLINT's testsuite (./sage -i -c) passes on OS X 10.4 PPC G5.

@jpflori
Copy link
Contributor

jpflori commented May 9, 2013

comment:337

Jeroen, could you please test te spkg on the PPC G5 with FLINT_TUNE="-fno-common -funroll-loops"?
If that works fine, I guess we should just go with that upstream rather than the "*=16" thing.

@jpflori
Copy link
Contributor

jpflori commented May 9, 2013

comment:338

Anyway, I'm ok with Jeroen changes to the spkg/patches, so I'm putting this back to positive_review.

@jpflori
Copy link
Contributor

jpflori commented May 9, 2013

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. Developers acknowledge bug.

@jdemeyer

This comment has been minimized.

@jpflori
Copy link
Contributor

jpflori commented May 10, 2013

comment:341

FYI, some patches similar to what Jeroen did have been integrated upstream and should be in the next release (and the other patch about dylib64 as well).

@jdemeyer
Copy link
Contributor

Merged: sage-5.10.beta3

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 17, 2013

comment:343

It would probably be better to keep a symlink $SAGE_LOCAL/include/FLINT -> flint for a while... which would also have avoided this.

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 17, 2013

comment:344

The FLINT include directory issue (FLINT vs. flint) is now #14603.

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

No branches or pull requests

9 participants