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 and promote spkg primecount #25009

Closed
videlec opened this issue Mar 18, 2018 · 109 comments
Closed

update and promote spkg primecount #25009

videlec opened this issue Mar 18, 2018 · 109 comments

Comments

@videlec
Copy link
Contributor

videlec commented Mar 18, 2018

The prime_pi function is currently a symbolic function that can hold its argument as in

sage: n = SR.var('n')
sage: prime_pi(n)
prime_pi(n)

that also contains the Cython code for evaluating it

sage: prime_pi(13543)
1603

Unfortunately, it is buggy, see #24960 (fixed in #32894, using this ticket). We will promote the spkg primecount to standard and use it instead. This ticket does this part, and reverts deprecations from #32412. It also removes obsolete deprecation warning, and the corresponding no longer available parameter.

Upstream patch: kimwalisch/primesieve#107 (rejected)


Below is the original part, no longer too relevant.


We move the evaluation part as a standalone function in arith/ and provide alternative implementations

  • using PARI/GP (only efficient in the range of tabulated primes)
  • using the different algorithms in the library primecount that is packaged in package primecount 4.3 #24966

Upstream: Fixed upstream, but not in a stable release.

CC: @mkoeppe @miguelmarco @isuruf

Component: basic arithmetic

Author: Dima Pasechnik

Branch/Commit: b9b4eaf

Reviewer: Matthias Koeppe

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

@videlec videlec added this to the sage-8.2 milestone Mar 18, 2018
@videlec

This comment has been minimized.

@videlec

This comment has been minimized.

@videlec
Copy link
Contributor Author

videlec commented Mar 19, 2018

New commits:

1297abapackage primecount
3e2d53ddraft for 25009

@videlec
Copy link
Contributor Author

videlec commented Mar 19, 2018

Commit: 3e2d53d

@videlec
Copy link
Contributor Author

videlec commented Mar 19, 2018

Branch: u/vdelecroix/25009-draft

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 6, 2021

comment:4

The package primecount has been provided with Sage since version 8.3, has been updated several times and is already outdated https://repology.org/project/primecount/versions

Is it time to start using it?

Because sage.libs.primecount uses an optional library, in the modularization of sagelib it would have to be split out to a separate package. But since src/sage/libs/primecount.pxd and src/sage/interfaces/primecount.pyx do not depend on Sage at all (except for a use of sage.misc.superseded), it would be best to make it a standalone Python library, like you did with pplpy.

@mkoeppe mkoeppe changed the title move prime_pi as a standalone function move prime_pi as a standalone function, use library primecount Jul 6, 2021
@mkoeppe mkoeppe modified the milestones: sage-8.2, sage-9.4 Jul 6, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@dimpase
Copy link
Member

dimpase commented Nov 12, 2021

comment:7

prime_pi is slow, quite buggy (see #24960), and should be ditched in favour of calling primecount.

@dimpase
Copy link
Member

dimpase commented Nov 15, 2021

Changed dependencies from #24966 to none

@dimpase
Copy link
Member

dimpase commented Nov 15, 2021

Changed commit from 3e2d53d to 1e24a4a

@dimpase
Copy link
Member

dimpase commented Nov 15, 2021

comment:8

bump to 7.1, and standard, remove deprecations


New commits:

1e24a4abump primecount to ver. 7.1, and standard

@dimpase
Copy link
Member

dimpase commented Nov 15, 2021

@dimpase
Copy link
Member

dimpase commented Nov 15, 2021

comment:10

this is a start for fixing #24960

@dimpase
Copy link
Member

dimpase commented Nov 16, 2021

comment:11

I am not sure whether there is much value in the branch u/vdelecroix/25009-draft,
as functions mentioned there are no longer in primecount.hpp, they are in primecount-internal.hpp, cf. https://github.com/kimwalisch/primecount

That is, they are not for public consumption. Let us use this ticket to update and promote the package to standard.

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Nov 16, 2021

Author: Dima Pasechnik

@dimpase dimpase changed the title move prime_pi as a standalone function, use library primecount update and promote spkg primecount Nov 16, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 16, 2021

Changed commit from 1e24a4a to a660ee4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 16, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

a660ee4removed deprecations

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 16, 2021

comment:13

Changes from optional to standard require a vote on sage-devel

@dimpase
Copy link
Member

dimpase commented Nov 16, 2021

comment:14

It fixes a serious bug, I can't imagine any objections. Feel free to call the vote, though.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 28, 2021

comment:74

primesieve does not build on some of our still supported platforms. I have a patch (kimwalisch/primesieve#107) for fixing most of them, but upstream does not want it. With the patch, there is one remaining failing platform, debian-jessie (kimwalisch/primesieve#108), upstream indicates wontfix.

@dimpase
Copy link
Member

dimpase commented Nov 28, 2021

comment:75

time to drop gcc 4-based systems...

@dimpase
Copy link
Member

dimpase commented Nov 28, 2021

comment:76

Please feel free to add patches.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 28, 2021

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 28, 2021

comment:78

Replying to @dimpase:

time to drop gcc 4-based systems...

Yes, soon, I'd say.
It would just feel a bit excessive to do so just for a single function.

I'll see if the failure on debian-jessie is as easy to fix as the other failures were. If not, we can drop debian-jessie in 9.5 already, it seems to have a particularly messy compiler.


New commits:

b9b4eafbuild/pkgs/primesieve: Add patch to support ubuntu-trusty, linuxmint-17

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 28, 2021

Changed commit from f754d06 to b9b4eaf

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Nov 29, 2021

comment:81

IMHO the patch for std:align should go into gcc spkg, not here.
Test if it's there, if not, put it into std namespace, instead of patching around all the C++ code
affected by it.

While strictly speaking putting new functions into namespace std is undefined behaviour, with gcc is just works (IMHO).

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 29, 2021

comment:82

primesieve is the only the package that needs it.

And patching the system compiler sounds like a terrible idea.

@dimpase
Copy link
Member

dimpase commented Nov 29, 2021

comment:83

the compiler is OK in this case, it's STL that it's not.

If I recall correctly, that's how we patched STLs for CGAL back in 1999:

All you need to to is to put -I foo/bar/ into CXXFLAGS, and put into foo/bar a file named memory, where one puts #include <memory> and an implementation of align() inside namespace std {...} block.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 29, 2021

comment:84

Sure, that's another way to do this, but I don't think it's better than what's already done and tested

@dimpase
Copy link
Member

dimpase commented Nov 29, 2021

comment:85

OK, I've just rebased the branch of #32894 over the branch here, and will do a run on GH Actions there.
It will test this ticket along the way, too.

@dimpase
Copy link
Member

dimpase commented Nov 29, 2021

comment:86

tests on https://github.com/sagemath/sagetrac-mirror/actions/runs/1517758345 (with primecountpy)

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 2, 2021

Changed reviewer from https://github.com/sagemath/sagetrac-mirror/actions/runs/1500740315 to Matthias Koeppe

@isuruf
Copy link
Member

isuruf commented Dec 2, 2021

comment:88

Can you add primecount to distros/conda.txt?

@dimpase
Copy link
Member

dimpase commented Dec 2, 2021

comment:89

Replying to @isuruf:

Can you add primecount to distros/conda.txt?

done on #32894 (to minimize rebasing :))

@vbraun
Copy link
Member

vbraun commented Dec 12, 2021

Changed branch from u/mkoeppe/packages/primecount/update-to-standard to b9b4eaf

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

5 participants