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

D4.1: Python/Cython bindings for PARI and its integration in Sage #83

Closed
minrk opened this issue Sep 8, 2015 · 42 comments
Closed

D4.1: Python/Cython bindings for PARI and its integration in Sage #83

minrk opened this issue Sep 8, 2015 · 42 comments

Comments

@minrk
Copy link
Contributor

@minrk minrk commented Sep 8, 2015

The SageMath project includes many different subsystems, mostly written in C/C++. For each subsystem, Sage provides a low-level interface, usually written in Cython, through which the higher level components access the subsystem. The mathematical community would immensely benefit if the low-level interfaces were maintained outside of the Sage project, as separate Python packages. Indeed such decoupling would enable other Python projects to build upon those externalized interfaces, thus helping to improve them, and share maintenance effort.

Of all the subsystems, the case of PARI is of particular interest. Since its inception, Sage has had a low-level Cython interface to PARI, which has evolved over time in a highly coupled way. Around 2012, some Sage developers forked this interface into a project they called CyPari. One of the primary goals of the CyPari fork was to make independent Python bindings to PARI for use in a project called Snappy. Over time, the CyPari fork has diverged from the Sage/PARI interface, and has gotten behind in terms of functionality.

The goal of this deliverable is to reconcile the fork by externalizing the Sage/PARI interface into an independent package, maintained by the Sage community, which may ultimately replace CyPari inside Snappy. The task happened to be more difficult than originally thought. The high level of coupling between Sage internals and the PARI interface makes it very delicate to pull the latter out of the SageMath codebase. The process of making this possible has led to a great amount of refactoring inside the Sage project, which is summarized in Trac ticket 20238.

Because of the high degree of coupling, and thanks to the availability of Snappy, this deliverable constitutes a highly valuable case study for future externalizations of low-level interfaces in SageMath. To bring this deliverable to completion, we have decided to split it in several steps:

  • Move SageMath's C signalling api to a separate Python/Cython package. The package is called cysignals, and is integrated to SageMath 7.1.
  • Decouple SageMath's PARI interface from the coercion model. This has been achieved in SageMath 7.4.
  • Clean up the interface API, by removing unneeded object orientation and external dependencies. This has been achieved, and is integrated to SageMath 7.5.
  • Move SageMath's PARI interface to a separate Python/Cython package depending on cysignals. The package is called CyPari2, and will replace the old PARI interface starting from SageMath 7.6.

The CyPari2 package is not ready to replace the PyPi package CyPari yet. The most important missing functionality is Windows compatibility. A full replacement to CyPari is the goal of deliverable D4.10 #84.

@minrk minrk added this to the D4.1 milestone Sep 8, 2015
@nthiery nthiery mentioned this issue Sep 11, 2015
3 of 3 tasks complete
@minrk minrk self-assigned this Nov 3, 2015
@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Nov 17, 2015

I think it would be good to better work out what this deliverable could mean.

The proposal says "standalone bindings for PARI in Python and integrate it in Sage". There are really two things here:
(A) Stand-alone bindings for PARI in Python.
(B) Bindings for PARI in Sage.

We already have (B) in Sage. And (A) is done by CyPari, which is essentially a fork of the Sage bindings (using an older version of Sage and PARI).

However, I think that it is not possible to make a stand-alone library like CyPari function within Sage. The main obstacle I see is the coercion model: you just cannot plug arbitrary classes into the Sage coercion model, you need to inherit from Element. A second potential obstacle is the signal and error handling.

One thing we could do is to factor out all the common stuff between (A) and (B) into some template (C) (I say template because it wouldn't be a complete Python module) which can then be made into either (A) or (B).

@defeo
Copy link
Contributor

@defeo defeo commented Nov 17, 2015

Reading the description of T4.12, I really understand this deliverable asks for (B). I think this is more about the social issue (who maintains what), than the technical problem. The Pari devs (and me too) would be very happy if something like CyPari received more official support from the Sage community, rather than being an underground project.

As you point out, it is impossible to plug CyPari directly into Sage: we would need to wrap every CyPari class in an Element wrapper, which is essentially doing the same work twice.

So we probably have to go the (C) way, which feels like a lot of work for an uncertain benefit, but this is going to pave the way for other libraries (Pari was picked here simply because we have Pari devs in the project): if from this work we can come up with a standard way to write python wrappers so that they can be easily integrated into Sage's class system, then we win!

Now, this is just my vision on this task. I didn't write it (and I don't remember why, I didn't even sign UVSQ up for it), so it might not be in line with the vision of the WP and task leaders. So I'm waiting for input from the leaders, and I'm happy to work on it.

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Nov 18, 2015

Reading the description of T4.12, I really understand this deliverable asks for (B).

I don't read it that way. In any case, (B) already exists. I think the proposal asks for (A) and then use (A) in Sage to implement (B).

Let me also add that I'm totally +1 to more cooperation between the Sage and PARI projects.

@nthiery nthiery assigned videlec and unassigned minrk Nov 18, 2015
@defeo
Copy link
Contributor

@defeo defeo commented Nov 18, 2015

Sorry, I misread your comment. Of course s/(B)/(A)/ in my previous comment.

@nthiery
Copy link
Contributor

@nthiery nthiery commented Nov 18, 2015

Oups, T4.12, and its two deliverables D4.1 and D4.10 really belongs to WP 3 Component Architecture, not WP4 User Interfaces. We should have noticed this earlier! Oh well, no big deal, and it's to late to fix it anyway. But please reassign the issues to their appropriate leader (@videlec?).

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Nov 20, 2015

This might be a good project for the PARI/GP workshop.

@defeo
Copy link
Contributor

@defeo defeo commented Nov 20, 2015

@videlec
Copy link
Contributor

@videlec videlec commented Nov 23, 2015

Sadly I will not be at the PARI/GP workshop (too far away from Santiago). A templating approach (C) suggested by Jeroen looks like the only viable one for the moment.

I wrote the WP description in order to gather the developments of
(1) Python/Cython backend for pari (i.e. an up to date CyPari more flexible and with more features)
(2) SnapPy and Sage integration
Of course, I consulted Karim who thought it was a good idea. It was also clear that he had no time to work on it. Whether (1) should be part of the PARI/GP project should definitely be discussed in January.

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Jan 13, 2016

For reference, I forked CyPari at https://github.com/jdemeyer/cypari and @defeo asked the CyPari devs what they want to do with the CyPari project.

@embray
Copy link
Collaborator

@embray embray commented Feb 11, 2016

I'm still catching up on this issue, but let me know if there's anything I can do to help on it, especially the packaging.

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Feb 11, 2016

Nothing has happened really... I have many ideas though.

My first priority now is to finish https://github.com/sagemath/cysignals which could be seen as dependency of CyPari, since PARI uses the signals framework.

@defeo
Copy link
Contributor

@defeo defeo commented Feb 11, 2016

My first priority now is to finish https://github.com/sagemath/cysignals which could be seen as dependency of CyPari, since PARI uses the signals framework.

Quite some work here! I'd be happy to help, but I don't know how I can be useful.

Maybe I can start working on extracting the Sage-Pari interface without waiting for cysignals to be completed?

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Feb 11, 2016

without waiting for cysignals to be completed?

cysignals itself is complete, the last piece is just integrating it in Sage.

@defeo
Copy link
Contributor

@defeo defeo commented Feb 25, 2016

I've pushed a attempt at rebooting cypari here: https://github.com/defeo/cypari. Very dirty and not yet functional. @jdemeyer, let's try and deliver this thing by the end of next week.

@bpilorget
Copy link
Contributor

@bpilorget bpilorget commented Mar 4, 2016

Hi there @defeo & @jdemeyer
Do you think you can deliver this soon? FYI @embray says he can help you with this if needed

@defeo
Copy link
Contributor

@defeo defeo commented Mar 4, 2016

@jdemeyer is coming to Orsay on March 14-18. We plan to deliver that week (I know, that's a 2-3 weeks delay, sorry).

@bpilorget
Copy link
Contributor

@bpilorget bpilorget commented Mar 21, 2016

You can track the progress here: http://trac.sagemath.org/ticket/20238

@nthiery nthiery modified the milestones: Month 6: 2016-02-29, D4.1 Mar 22, 2016
@nthiery nthiery assigned defeo and unassigned videlec Mar 22, 2016
@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Aug 4, 2016

I managed step 2 of our plan: uncouple PARI from the coercion model. The actual uncoupling was almost trivial, but the changes to the coercion model are not.

I plan to wait until #21158 is closed before continuing to work on CyPari. It's not a good idea to work on top of too many unreviewed tickets.

@defeo
Copy link
Contributor

@defeo defeo commented Aug 4, 2016

Perfect timing, I was right back on this too! I'll review it tomorrow.

@bpilorget
Copy link
Contributor

@bpilorget bpilorget commented Aug 18, 2016

@defeo @jdemeyer @nthiery Hi! How are things going?

@defeo
Copy link
Contributor

@defeo defeo commented Aug 18, 2016

Working on this. @jdemeyer will you be there at SD75? That'd be a good time to finalize.

@videlec
Copy link
Contributor

@videlec videlec commented Aug 18, 2016

I am looking at #20686.

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Aug 18, 2016

will you be there at SD75?

No, I cannot. I could come to Paris in the beginning of september.

@defeo
Copy link
Contributor

@defeo defeo commented Aug 18, 2016

No, I cannot. I could come to Paris in the beginning of september.

All right. I guess it can wait that little. I'll keep working on this at SD75 anyway.

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Aug 19, 2016

@videlec What about https://trac.sagemath.org/ticket/21005? We still need a reviewer for that.

@bpilorget
Copy link
Contributor

@bpilorget bpilorget commented Oct 24, 2016

Deliverables update

@jdemeyer @defeo As you know , during the interim review in Bremen, reviewers gave advice to improve deliverables. UPSud tried to follow their advice by updating D5.1 #107
The main critic is that deliverables should be static document. Therefore we went through the #107 report to check if all links were still working and also if the information contained on the links was necessary for the understanding of the report.
As a result when it was necessary annexes were added to the #107 report.

Can you please do the same for this deliverable and add annexes when need be?

@nthiery
Copy link
Contributor

@nthiery nthiery commented Jan 4, 2017

Hi Luca, Jeroen!
I am finally in the process of uploading the reports. Is the report for this deliverable final?
Do we want to check the last item on the above TODO list or comment further on it?

@defeo
Copy link
Contributor

@defeo defeo commented Jan 4, 2017

Uh, there's still a couple of tickets open from before Xmas. I don't expect the report to change, but we both would feel more comfortable if those tickets were closed, before saying this deliv is complete. It's not much work, however I do not have time for this before 1 or 2 weeks.

@nthiery
Copy link
Contributor

@nthiery nthiery commented Jan 4, 2017

Ok, this sounds fair enough. Thanks for the update!

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Jan 5, 2017

There are a few things to be done but I think that the hardest part has been done. At this point it's just a matter of priorities. The last months I worked more on building & packaging Sage (WP3) instead of this.

@defeo
Copy link
Contributor

@defeo defeo commented Jan 5, 2017

I'll work on this at the PARI/gp workshop next week. Can't promise I'll finish it, but I can try.

@nthiery
Copy link
Contributor

@nthiery nthiery commented Jan 5, 2017

Hi Jeroen!

Good question about priorities. Building&packaging progress is indeed great long term investment; thanks for all your work in this direction!

The hard deadline of Month 18 (February 28th) is coming very soon though, and there are several deliverables PSud/Versailles/Gent are involved in that remain to be finalized (this one, SCSCP #62, Jupyter kernels #93, Sage/Jupyter convergence #94 ) or more (SMC!!! #61). So unless there are tight deadlines on the B&P side (such as the recent debian release), it would be safe to focus some energy on the above deliverables.

We will brainstorm about that later today with Luca; let us know if you are available, and we can do a videoconference with you.

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Jan 5, 2017

We will brainstorm about that later today with Luca; let us know if you are available, and we can do a videoconference with you.

I'm only reading this now. You still mean today or should we do it another day? In any case, I guess we will meet in Edinburgh (I will come tuesday evening).

@nthiery
Copy link
Contributor

@nthiery nthiery commented Jan 6, 2017

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Jan 6, 2017

Luca will have a sprint on next week at the Pari days.

I will be there too, working possibly on this but preferably on the PARI part of D4.7 (#96).

@kohlhase kohlhase mentioned this issue Jan 19, 2017
19 of 19 tasks complete
@nthiery
Copy link
Contributor

@nthiery nthiery commented Feb 6, 2017

Dear M6 deliverable leaders,

Just a reminder that reports are due for mid-february, to buy us some time for proofreading, feedback, and final submission before February 28th. See our README for details on the process.

In practice, I'll be offline February 12-19, and the week right after will be pretty busy. Therefore, it would be helpful if a first draft could be available sometime this week, so that I can have a head start reviewing it.

Thanks in advance!

@defeo
Copy link
Contributor

@defeo defeo commented Feb 8, 2017

@nthiery, I've updated the report. I think it is ready for submission, maybe @jdemeyer wants to comment on it.

@nthiery
Copy link
Contributor

@nthiery nthiery commented Feb 9, 2017

@defeo
Copy link
Contributor

@defeo defeo commented Feb 9, 2017

Done

@nthiery
Copy link
Contributor

@nthiery nthiery commented Feb 10, 2017

Thanks Luca! I submitted the report to the EU portal after tiny format edits.

Thanks everyone for all the hard work on this challenging deliverable!

@nthiery nthiery closed this Feb 10, 2017
@bpilorget
Copy link
Contributor

@bpilorget bpilorget commented Feb 10, 2017

Yay! Congratulations!

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

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.