Bump version to 0.5.0 by GiacomoPope · Pull Request #84 · flintlib/python-flint · GitHub

Version bump to show new type added. Removed a TODO from the README and another message in the change log to discuss the integer roots from #72

Potentially we want to fix #74 before updating the version, but I don't have a good idea how to fix this myself.

We should also add notes for other PRs here.

Big refactor across several versions:

Mod experiment #75 Parserst #77 Refactor functions into submodule #67 Introduce submodules into python-flint #61

For 0.4.4 there should be

Include files in sdist and bump version to 0.4.4 #71 (adds Cython files again in sdist

For 0.4.3

Bump version to 0.4.3 #69 (adds OSX arm64 wheels)

We should get better at adding release notes here. It is important for people to see these so that they can see that development is happening. Also I think that each note should list the names of the authors. It is informative for anyone who wants to understand the project to see that there are multiple people working on things.

#63 was for 0.4.3 not 0.5.0

For 0.4.2

Fix typo #57 (Adds manylinux wheels)

For 0.4.1

Add tests for installing on Linux #48 Removes Linux wheels, updates instructions for building from source.

For 0.4.0

Bump version to 0.4.0 #45 Adds wheels for Windows, OSX and manylinux but the Linux wheels are broken.

For 0.4.3 #74 should be noted.

The Windows tests failed:

test_fmpz_mod...Flint exception (General error):
    mkstemp failed

That is because of #80. Note that my original PR for #80 noted this: #43 (comment) although somehow the tests passed.

It is good that we spotted this before release: we can't use flint 3-alpha1 because of the Windows Flint bug that was fixed in flintlib/flint#1416

So we either revert #80 or wait for a new (pre-)release of Flint 3 before making a new release of python-flint.

Do we have a timeline on the next pre-release? I guess that determines the best thing to do. I will find time today to address the comments above and do a more detailed change log for all these PR.

For "authors" how would you like to do it? I'm happy with whatever

For "authors" how would you like to do it? I'm happy with whatever

I don't know really. Maybe just linking to the PRs is fine.

For progress, I want to try and do fmpz_mod_poly, maybe if I finish this and the 3.0 bug remains we can revert #80 and if it's fixed by then it's all OK? I doubt the small addition for fmpz_mod will really help anyone right now.

We can consider reverting #80 at some point if it takes a while for a new release of Flint. for now I don't think it is urgent.

Especially if we expect that many new features might be added in a relatively short time there is no rush to get a release out for each one.

Rerun CI after gh-94 was merged

Some new Windows problem:

test_fmpz_mod...OK
test_fmpz_mod_dlog...
Error: Process completed with exit code 1.

Further investigation needed...

Twice now this pull request has picked up Windows problems that do not seem to show up elsewhere in CI. The changes here should not affect anything significant so I don't understand why that happens.

Is it always the discrete log tests which crash? The only thought is there’s a bug either in Flint3 which means windows can’t do something in this instance or there’s a memory issue in the python-flint which is causing issues. It’s mysterious… I’ll have some time next week I hope to look into it.

I can reproduce the problem on Windows locally with master so it is not this PR that causes the problem but there is a bug that causes a crash on Windows.

It is always the discrete log test that crashes and specifically it crashes here:

python-flint/src/flint/test/test.py

Lines 1831 to 1837 in e6c0078

    
           for _ in range(10): 
        
               g = F(random.randint(0,p)) 
        
               for _ in range(10): 
        
                   i = random.randint(0,p) 
        
                   a = g**i 
        
                   x = g.discrete_log(a) 
        
                   assert g**x == a

The crash happens in x = g.discrete_log(a) for the 1st iteration of the inner loop on the 2nd or 3rd iteration of the outer loop.

I think that this is most likely a memory bug in python-flint. The memory management for fmpz_mod and fmpz_mod_ctx is suspicious:

python-flint/src/flint/types/fmpz_mod.pxd

Lines 8 to 19 in e6c0078

    
           cdef class fmpz_mod_ctx: 
        
               cdef fmpz_mod_ctx_t val 
        
               cdef bint _is_prime 
        
               cdef fmpz_mod_discrete_log_pohlig_hellman_t *L 
        
               cdef set_any_as_fmpz_mod(self, fmpz_t val, obj) 
        
               cdef any_as_fmpz_mod(self, obj) 
        
               cdef _precompute_dlog_prime(self) 
        
           cdef class fmpz_mod(flint_scalar): 
        
               cdef fmpz_mod_ctx ctx 
        
               cdef fmpz_t val 
        
               cdef fmpz_t *x_g

I think this is something to do with either L or x_g which will be initialised on a 1st call to g.discrete_log(a) for a given g.

Here when an fmpz_mod is created x_g is set to NULL and when it is deallocated a test if self.x_g is used:

python-flint/src/flint/types/fmpz_mod.pyx

Lines 236 to 244 in e6c0078

    
           def __cinit__(self): 
        
               fmpz_init(self.val) 
        
               self.x_g = NULL 
        
           def __dealloc__(self): 
        
               fmpz_clear(self.val) 
        
               if self.x_g: 
        
                   fmpz_clear(self.x_g[0]) 
        
                   libc.stdlib.free(self.x_g)

I think firstly that the test should be if self.x_g is NULL although it is not clear if that is the problem.

A call to discrete_log will initialise this:

python-flint/src/flint/types/fmpz_mod.pyx

Lines 360 to 376 in e6c0078

    
           if not self.ctx.L: 
        
               self.ctx._precompute_dlog_prime() 
        
           # Solve the discrete log for the chosen base and target 
        
           # g = y^x_g and  a = y^x_a 
        
           # We want to find x such that a = g^x => 
        
           # (y^x_a) = (y^x_g)^x => x = (x_a / x_g) mod (p-1) 
        
           # For repeated calls to discrete_log, it's more efficient to 
        
           # store x_g rather than keep computing it 
        
           if not self.x_g: 
        
               self.x_g = <fmpz_t *>libc.stdlib.malloc( 
        
                   cython.sizeof(fmpz_t) 
        
               ) 
        
               fmpz_mod_discrete_log_pohlig_hellman_run( 
        
                   self.x_g[0], self.ctx.L[0], self.val 
        
               )

Again I think is NULL should be tested.

We also have:

python-flint/src/flint/types/fmpz_mod.pyx

Lines 140 to 151 in e6c0078

    
               cdef _precompute_dlog_prime(self): 
        
                   """ 
        
                   Initalise the dlog data, all discrete logs are solved with an  
        
                   internally chosen base `y` 
        
                   """ 
        
                   self.L = <fmpz_mod_discrete_log_pohlig_hellman_t *>libc.stdlib.malloc( 
        
                       cython.sizeof(fmpz_mod_discrete_log_pohlig_hellman_struct) 
        
                   ) 
        
                   fmpz_mod_discrete_log_pohlig_hellman_init(self.L[0]) 
        
                   fmpz_mod_discrete_log_pohlig_hellman_precompute_prime( 
        
                       self.L[0], self.val.n 
        
                   )

I think firstly that it would be better to avoid the use of malloc/free wherever possible in favour of using CPython-style memory management.

This all seems to be working with flint 3.0.0-rc1 now after gh-95.

I've tested locally on Windows that everything seems to be working.

Rerun after #98 with Flint 3.0.0 final.

All tests have passed. Previously this PR strangely seemed to flag up errors but I don't see any now.

I think that fmpz_mod_poly should be added to the changelog but otherwise this is good for a new release of python-flint.

I think that fmpz_mod_poly should be added to the changelog but otherwise this is good for a new release of python-flint.

I went to do this just now, but looks like you beat me to it!

It is expected that the build from PyPI job will fail because it is trying to build in an environment that has Flint 3 and does not have Arb (as a separate library). Until the PyPI sdist is updated that job should fail with the changes here. Previously it was passing because it was installing the binary wheel but I have just added --no-binary to force it to build properly.

Okay I think this is ready for a 0.5.0 release.

Yeah I agree.

Okay, let's do it!

@fredrik-johansson the docs for python-flint have not been updated sine 0.3.0 (5 years ago).

As of 0.5.0 there are some significant new sections in the docs for fmpz_mod and fmpz_mod_poly as well as changes to the build instructions (although those might actually need updating again).

Ideally docs updates would be handled automatically in CI along with pushing to PyPI but for now at least I am pushing manually to PyPI.

The docs are hosted on your website fredrikj.net though so I don't know if you would prefer to push the updates there yourself manually. Alternatively we could designate a repo somewhere for hosting the built docs that could be pulled automatically by fredrikj.net or something.

Maybe host the docs on readthedocs or somesuch? We could do something automatic on my server but I'm not too eager to set that up right now.

I'll have a look at setting up read the docs.

I have finally managed to upload the release files to PyPI:

https://pypi.org/project/python-flint/#files

It took 3 hours with a lot restarting twine to get those files up there. Maybe there is a better way to do the uploading.

I need to go do other things now but I'll put out announcements later.

GiacomoPope added 2 commits September 16, 2023 10:49

Bump to version 0.4.5

fc37eed

Remove fmpz_mod from todo

c9042d8

GiacomoPope marked this pull request as ready for review September 16, 2023 09:52

oscarbenjamin reviewed Sep 16, 2023

View reviewed changes

Add more detailed changelog

05a2e33

oscarbenjamin closed this Oct 13, 2023

oscarbenjamin reopened this Oct 13, 2023

oscarbenjamin mentioned this pull request Oct 14, 2023

fix(fmpz_mod): avoid using malloc/free #95

Merged

oscarbenjamin closed this Oct 14, 2023

oscarbenjamin reopened this Oct 14, 2023

oscarbenjamin closed this Oct 20, 2023

oscarbenjamin reopened this Oct 20, 2023

Bump version to 0.5.0

fd20135

oscarbenjamin changed the title Bump version to 0.4.5 Bump version to 0.5.0 Oct 21, 2023

oscarbenjamin and others added 2 commits October 21, 2023 13:16

Update pip install CI test

2e3e156

Merge branch 'flintlib:master' into 0.4.5

2102d0c

Use :all: in pip test

f75f2f6

oscarbenjamin closed this Oct 21, 2023

oscarbenjamin reopened this Oct 21, 2023

oscarbenjamin added 2 commits October 22, 2023 00:00

Merge remote-tracking branch 'upstream/master' into HEAD

21c64be

Update README

05f57cd

oscarbenjamin merged commit 9252d17 into flintlib:master Oct 22, 2023

GiacomoPope deleted the 0.4.5 branch October 23, 2023 14:20

oscarbenjamin mentioned this pull request Nov 1, 2023

flint.good can not be imported (python-flint 0.5) #102

Closed

		- gh-63: The `roots` method of `arb_poly`, and `nmod_poly` is no longer supported. Use `acb_roots(p).roots()` to get the old behaviour of returning the roots as `acb`. Note that the `roots` method of `fmpz_poly` and `fmpq_poly` currently returns the complex roots of the polynomial.
		- gh-72: The roots method of `arb_poly` is not supported. Use either the `complex_roots` method or `acb_roots(p).roots()` to get the old behaviour of returning the complex roots. The `roots` method on `fmpz_poly` and `fmpq_poly` now return integer and rational roots respectively. To access complex roots on these types, use the `complex_roots` method. For `acb_poly`, both `roots` and `complex_roots` behave the same.

Bump version to 0.5.0 by GiacomoPope · Pull Request #84 · flintlib/python-flint · GitHub

Repository navigation

Bump version to 0.5.0 - #84

Bump version to 0.5.0#84oscarbenjamin merged 9 commits intoflintlib:masterflintlib/python-flint:masterfrom GiacomoPope:0.4.5GiacomoPope/python-flint:0.4.5Copy head branch name to clipboard

Conversation

GiacomoPope commented Sep 16, 2023

Uh oh!

GiacomoPope commented Sep 16, 2023

Uh oh!

oscarbenjamin Sep 16, 2023

Choose a reason for hiding this comment

Uh oh!

oscarbenjamin Sep 16, 2023

Choose a reason for hiding this comment

Uh oh!

oscarbenjamin Sep 16, 2023

Choose a reason for hiding this comment

Uh oh!

oscarbenjamin commented Sep 16, 2023

Uh oh!

oscarbenjamin commented Sep 16, 2023

Uh oh!

GiacomoPope commented Sep 16, 2023

Uh oh!

oscarbenjamin commented Sep 16, 2023

Uh oh!

GiacomoPope commented Sep 16, 2023

Uh oh!

oscarbenjamin commented Sep 16, 2023

Uh oh!

oscarbenjamin commented Oct 13, 2023

Uh oh!

oscarbenjamin commented Oct 14, 2023

Uh oh!

oscarbenjamin commented Oct 14, 2023

Uh oh!

GiacomoPope commented Oct 14, 2023 via email

Uh oh!

oscarbenjamin commented Oct 14, 2023

Uh oh!

oscarbenjamin commented Oct 14, 2023

Uh oh!

oscarbenjamin commented Oct 15, 2023

Uh oh!

oscarbenjamin commented Oct 20, 2023

Uh oh!

oscarbenjamin commented Oct 20, 2023

Uh oh!

GiacomoPope commented Oct 21, 2023

Uh oh!

oscarbenjamin commented Oct 21, 2023

Uh oh!

oscarbenjamin commented Oct 22, 2023

Uh oh!

GiacomoPope commented Oct 22, 2023

Uh oh!

oscarbenjamin commented Oct 22, 2023

Uh oh!

oscarbenjamin commented Oct 22, 2023

Uh oh!

fredrik-johansson commented Oct 22, 2023

Uh oh!

oscarbenjamin commented Oct 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Bump version to 0.5.0#84
oscarbenjamin merged 9 commits into
flintlib:masterfrom
GiacomoPope:0.4.5