Bump flint to version 3 by oscarbenjamin · Pull Request #43 · flintlib/python-flint · GitHub

Following on from #35 (comment)

Bumps the flint version to 3.0.0-alpha1. Mostly all that is needed is removing -larb from setup.py and renaming the Arb headers. Also the build scripts need to not build Arb separately any more.

We can use this to see in CI that python_flint is ready for flint 3 but it should only be merged after flint 3 is released.

At first CI will fail because of Cython 3 (gh-35 needs to be merged first or the Cython version should be pinned again).

The extension module fails to compile on OSX:

     INFO: gcc: src/flint/pyflint.c
      src/flint/pyflint.c:97845:3: error: implicit declaration of function 'nmod_poly_factor_init' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        nmod_poly_factor_init(__pyx_v_fac);
        ^
      src/flint/pyflint.c:97845:3: note: did you mean 'fmpz_poly_factor_init'?
      /private/var/folders/76/zy5ktkns50v6gt5g8r0sf6sc0000gn/T/cirrus-ci-build/.local/include/flint/fmpz_poly_factor.h:30:6: note: 'fmpz_poly_factor_init' declared here
      void fmpz_poly_factor_init(fmpz_poly_factor_t fac);
           ^
      src/flint/pyflint.c:97864:20: error: implicit declaration of function 'nmod_poly_factor_with_berlekamp' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
          __pyx_v_lead = nmod_poly_factor_with_berlekamp(__pyx_v_fac, __pyx_v_self->val);
                         ^
      src/flint/pyflint.c:97893:20: error: implicit declaration of function 'nmod_poly_factor_with_cantor_zassenhaus' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
          __pyx_v_lead = nmod_poly_factor_with_cantor_zassenhaus(__pyx_v_fac, __pyx_v_self->val);
                         ^
      src/flint/pyflint.c:97913:20: error: implicit declaration of function 'nmod_poly_factor' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
          __pyx_v_lead = nmod_poly_factor(__pyx_v_fac, __pyx_v_self->val);
                         ^
      src/flint/pyflint.c:97913:20: note: did you mean 'fmpz_poly_factor'?
      /private/var/folders/76/zy5ktkns50v6gt5g8r0sf6sc0000gn/T/cirrus-ci-build/.local/include/flint/fmpz_poly_factor.h:82:6: note: 'fmpz_poly_factor' declared here
      void fmpz_poly_factor(fmpz_poly_factor_t fac, const fmpz_poly_t G);
           ^
      src/flint/pyflint.c:98044:3: error: implicit declaration of function 'nmod_poly_factor_clear' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        nmod_poly_factor_clear(__pyx_v_fac);
        ^
      src/flint/pyflint.c:98044:3: note: did you mean 'fmpz_poly_factor_clear'?
      /private/var/folders/76/zy5ktkns50v6gt5g8r0sf6sc0000gn/T/cirrus-ci-build/.local/include/flint/fmpz_poly_factor.h:38:6: note: 'fmpz_poly_factor_clear' declared here
      void fmpz_poly_factor_clear(fmpz_poly_factor_t fac);
           ^
      src/flint/pyflint.c:230687:22: warning: comparison of integers of different signs: 'long' and 'mp_limb_t' (aka 'unsigned long') [-Wsign-compare]
            if ((__pyx_t_5 > __pyx_t_6)) {
                 ~~~~~~~~~ ^ ~~~~~~~~~
      src/flint/pyflint.c:259851:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:259862:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:260148:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:260159:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:262322:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:262333:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:262452:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:262463:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:264143:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      src/flint/pyflint.c:264149:21: warning: fallthrough annotation in unreachable code [-Wunreachable-code-fallthrough]
                          CYTHON_FALLTHROUGH;
                          ^
      src/flint/pyflint.c:515:34: note: expanded from macro 'CYTHON_FALLTHROUGH'
            #define CYTHON_FALLTHROUGH __attribute__((fallthrough))
                                       ^
      11 warnings and 5 errors generated.
      error: Command "gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -g -arch arm64 -arch arm64 -I/private/var/folders/76/zy5ktkns50v6gt5g8r0sf6sc0000gn/T/cibw-run-ku3c16ly/cp39-macosx_arm64/build/venv/include -I/Library/Frameworks/Python.framework/Versions/3.9/include/python3.9 -c src/flint/pyflint.c -o build/temp.macosx-11.0-arm64-cpython-39/src/flint/pyflint.o" failed with exit status 1
      [end of output]
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for python-flint
  Running setup.py clean for python-flint
Failed to build python-flint
ERROR: Failed to build one or more wheels

I saw at least some of those errors but as warnings locally on Linux and the Linux job completes successfully in CI. I guess this is clang being stricter than gcc.

The functions being complained about are all after the newline here:
https://github.com/fredrik-johansson/python-flint/blob/027dd1fdae698c95ca8b7f8cadb095536e5c013b/src/flint/_flint.pxd#L154-L163
It complains about each of them except for the first nmod_poly_is_irreducible.

The functions being complained about are all after the newline here:

I guess it is because these were moved from nmod_poly.h to nmod_poly_factor.h.

Okay, I think that might be it. In the previous commit Linux and OSX were working but the Windows build failed at the final repair wheel step because the DLLs for Arb are now placed in bin rather than lib (I think that is correct for Windows - GMP, MPFR and Flint already put their DLLs in bin).

Hopefully the last commit will have the Windows build working and then everything here is ready and working with flint 3.0.0-alpha1.

It is good to have a branch here with CI passing ready for the flint 3.0.0 release but I don't propose to merge this until flint itself is released. When there is either a new prerelease or a final release of 3.0.0 the version that is pinned here to 3.0.0-alpha1 in bin/build_variables.sh should be updated to see that CI still passes.

So the Windows build succeeded and the normal tests passed but the doctests have crashed somehow:

python test/dtest.py
  shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
  env:
    pythonLocation: C:\hostedtoolcache\windows\Python\[3](https://github.com/fredrik-johansson/python-flint/actions/runs/5761150407/job/15619523225?pr=43#step:6:3).9.13\x6[4](https://github.com/fredrik-johansson/python-flint/actions/runs/5761150407/job/15619523225?pr=43#step:6:4)
    PKG_CONFIG_PATH: C:\hostedtoolcache\windows\Python\3.9.13\x[6](https://github.com/fredrik-johansson/python-flint/actions/runs/5761150407/job/15619523225?pr=43#step:6:6)4/lib/pkgconfig
    Python_ROOT_DIR: C:\hostedtoolcache\windows\Python\3.9.13\x64
    Python2_ROOT_DIR: C:\hostedtoolcache\windows\Python\3.9.13\x64
    Python3_ROOT_DIR: C:\hostedtoolcache\windows\Python\3.[9](https://github.com/fredrik-johansson/python-flint/actions/runs/5761150407/job/15619523225?pr=43#step:6:9).13\x64
Flint exception (General error):
    mkstemp failed
Error: Process completed with exit code 1.

I'll see if I can reproduce that locally.

I can reproduce the doctest crash locally on Windows with the wheel downloaded from CI (we should add -v in CI):

$ python ../../test/dtest.py -v
...
Trying:
    fmpz(10**35 + 1).factor()
Expecting:
    [(11, 1), (9091, 1), (909091, 1), (4147571, 1), (265212793249617641, 1)]
Flint exception (General error):
    mkstemp failed

I guess that error comes from here:
https://github.com/flintlib/flint2/blob/ef89816abda221fc40cb4a960430cb570ef0f43c/src/qsieve/factor.c#L222-L225
The question is what exactly has changed in flint-3.0.0-alpha1 that would cause that to fail.

git blame points to flintlib/flint#1334

It is expected that the "Build / Install from PyPI sdist on Ubuntu" CI job would fail here. Until a sdist containing this PR is pushed to PyPI the build instructions in the script doing that install will not work because they do not build arb separately and that will not work with the python-flint 0.4.1 sdist that is still on PyPI. The Build / Install from git checkout on Ubuntu job works and so that (hopefully) implies that the install from PyPI script will work after python-flint issues a flint-3 compatible release.

It is also expected that the tests will fail on Windows with mkstemp failed. That was a Flint bug which was fixed in flintlib/flint#1416. When this PR can bump from Flint 3.0.0-alpha1 to the next prerelease of Flint then the Windows tests should work (I have tested this locally).

It is also expected that the tests will fail on Windows with mkstemp failed.

Actually now that CI completes the Windows tests have not failed. I am not sure why they work now when they did not before...

The PR still builds with Flint: 3.0.0-alpha1 that does not contain the fix for the failure seen previously and I haven't changed anything in python-flint that should affect this so having the Windows tests pass in CI is a mystery.

The one failure that is seen (build from PyPI) is still expected though. I just changed the Linux build instructions/demo script in this PR in a way that is not compatible with the current release of python-flint and won't be until a new release is made based on flint-3.

Rebased after #65

@deinst I needed to change this for clang (OSX) and Flint 3.

I'm not sure why the problem only shows up with clang and Flint 3. Probably it should changed on master as well: see the last commit in this PR. I don't think it's urgent given that a fix is here ready for Flint 3 when it gets released (I guess that is soon).

My wife's sister and her husband are visiting for a few days so my work will be sporadic, but the new nmod.pxd will land next week at the latest. I have been updating the flintlib submodule with all the functions from the documentation, and have found a couple of doc bugs. This should be done in the next few days

No need to rush!

I doubt there will be a release of python-flint before flint 3 so at least the change above will be merged by then from this PR if not another.

Actually I'm putting out a release right now (#69) due to (#66).

It's all fine though I think for now.

With the new scraping tool for flintlib, is the migration from flint2 -> flint3 going to happen soon? Happy to help if this is the case :)

I you would like to fix this PR (which has merge conflicts now) and open a new PR I would be happy to merge it and move to flint3.

I guess this PR can now be closed?

oscarbenjamin force-pushed the pr_flint3 branch from cd3c157 to edf6dae Compare August 3, 2023 18:33

This was referenced Aug 4, 2023

Change TMPPATH to the standard TMPDIR in configuration flintlib/flint#1334

Merged

Testing fails on MinGW flintlib/flint#1206

Closed

Remove TMPDIR and use Win API on Windows flintlib/flint#1416

Merged

Uploading wheels to pypi #1

Closed

oscarbenjamin mentioned this pull request Aug 11, 2023

Changes for flint 3.0 #49

Closed

oscarbenjamin force-pushed the pr_flint3 branch from 2b671c3 to cc391ff Compare August 11, 2023 18:45

oscarbenjamin force-pushed the pr_flint3 branch from b91822a to 07bd650 Compare August 15, 2023 17:07

oscarbenjamin mentioned this pull request Aug 17, 2023

Adding Fmpz mpoly #59

Merged

oscarbenjamin mentioned this pull request Sep 6, 2023

Mod experiment #65

Merged

oscarbenjamin added 9 commits September 6, 2023 21:59

Bump flint to 3.0.0-alpha1

969c884

chmod +x configfsf.guess

ca909ff

brew install automake

5f1a2bf

brew install libtool

53a2e3e

Add base-devel in mingw

83a4efd

Add autoconf-wrapper in mingw

aa7d632

Add automake-wrapper to minge

f88460e

Add libtool to mingw

2badf4b

Only source DLLs from .local/bin

6daf435

oscarbenjamin added 2 commits September 6, 2023 22:04

Update the Linux build demo script for flint 3

ddfafbd

Use flint/ for arb headers

6b3a711

oscarbenjamin force-pushed the pr_flint3 branch from 07bd650 to 6b3a711 Compare September 6, 2023 21:26

oscarbenjamin added 2 commits September 6, 2023 22:48

Add nmod_poly_factor.h

81ca58a

nmod_vec -> nmod

a635d59

oscarbenjamin commented Sep 6, 2023

View reviewed changes

GiacomoPope mentioned this pull request Sep 11, 2023

Bump to Flint version 3 #80

Merged

oscarbenjamin closed this Sep 12, 2023

oscarbenjamin mentioned this pull request Sep 16, 2023

Bump version to 0.5.0 #84

Merged

oscarbenjamin deleted the pr_flint3 branch February 1, 2024 23:26

               from flint._flint cimport mp_limb_t, mp_bitcnt_t
-              cdef extern from "flint/nmod_vec.h":
+              cdef extern from "flint/nmod.h":

Bump flint to version 3 by oscarbenjamin · Pull Request #43 · flintlib/python-flint · GitHub

Repository navigation

Bump flint to version 3 - #43

Bump flint to version 3#43oscarbenjamin wants to merge 13 commits intoflintlib:masterflintlib/python-flint:masterfrom oscarbenjamin:pr_flint3oscarbenjamin/python-flint:pr_flint3Copy head branch name to clipboard

Conversation

oscarbenjamin commented Aug 3, 2023

Uh oh!

oscarbenjamin commented Aug 3, 2023

Uh oh!

oscarbenjamin commented Aug 3, 2023

Uh oh!

oscarbenjamin commented Aug 3, 2023

Uh oh!

oscarbenjamin commented Aug 4, 2023

Uh oh!

oscarbenjamin commented Aug 4, 2023

Uh oh!

oscarbenjamin commented Aug 4, 2023

Uh oh!

oscarbenjamin commented Aug 4, 2023

Uh oh!

oscarbenjamin commented Aug 4, 2023

Uh oh!

oscarbenjamin commented Aug 11, 2023

Uh oh!

oscarbenjamin commented Aug 11, 2023

Uh oh!

oscarbenjamin commented Aug 11, 2023

Uh oh!

oscarbenjamin commented Sep 6, 2023

Uh oh!

oscarbenjamin Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

deinst Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

oscarbenjamin Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

oscarbenjamin Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

GiacomoPope commented Sep 11, 2023

Uh oh!

oscarbenjamin commented Sep 11, 2023

Uh oh!

GiacomoPope commented Sep 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Bump flint to version 3#43
oscarbenjamin wants to merge 13 commits into
flintlib:masterfrom
oscarbenjamin:pr_flint3