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

Distributions sagemath-{flint-arb,homfly,giac,gap}, add "sage_setup: distribution" to Cython modules #30666

Open
mkoeppe opened this issue Sep 26, 2020 · 84 comments · May be fixed by #35095
Open

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 26, 2020

Basically, we are making all external libraries optional, except for a minimal set of "core" libraries such as cysignals and gmpy2 (see #29865).

(from #30371): See #29705 under the items titled "Deploy mildly modularized distributions", "Further modularization" for a sketch of such distributions. Basically, one distribution for each major C/C++ library (such as sagemath-ntl) - but some libraries come in packs that would not make much sense to separate (such as sagemath-flint-arb-e_antic).

List of distributions:

  • sagemath-flint-arb
    - 'sage/libs/arb/arb_version.pyx',
    - 'sage/libs/arb/arith.pyx',
    - 'sage/matrix/matrix_complex_ball_dense.pyx',
    - 'sage/rings/complex_arb.pyx',
    - 'sage/rings/number_field/number_field_element_quadratic.pyx',
    - 'sage/rings/polynomial/polynomial_complex_arb.pyx',
    - 'sage/rings/real_arb.pyx'
    - etc.
  • sagemath-homfly: 'sage/libs/homfly.pyx'
  • sagemath-giac (ex giacpy_sage in sage.libs.giac + dependency on libgiac)
    - sage/libs/giac/giac.pyx
  • sagemath-gap:
    - 'sage/coding/codecan/codecan.pyx',
    - 'sage/combinat/enumeration_mod_permgroup.pyx',
    - 'sage/combinat/root_system/reflection_group_c.pyx',
    - 'sage/combinat/root_system/reflection_group_element.pyx',
    - 'sage/graphs/spanning_tree.pyx',
    - 'sage/groups/libgap_wrapper.pyx',
    - 'sage/groups/matrix_gps/group_element.pyx',
    - 'sage/groups/perm_gps/permgroup_element.pyx',
    - 'sage/groups/perm_gps/partn_ref/automorphism_group_canonical_label.pyx',
    - 'sage/groups/perm_gps/partn_ref/canonical_augmentation.pyx',
    - 'sage/groups/perm_gps/partn_ref/data_structures.pyx',
    - 'sage/groups/perm_gps/partn_ref/double_coset.pyx',
    - 'sage/groups/perm_gps/partn_ref/refinement_binary.pyx',
    - 'sage/groups/perm_gps/partn_ref/refinement_graphs.pyx',
    - 'sage/groups/perm_gps/partn_ref/refinement_lists.pyx',
    - 'sage/groups/perm_gps/partn_ref/refinement_matrices.pyx',
    - 'sage/groups/perm_gps/partn_ref/refinement_python.pyx',
    - 'sage/groups/perm_gps/partn_ref/refinement_sets.pyx',
    - 'sage/groups/perm_gps/partn_ref2/refinement_generic.pyx',
    - 'sage/libs/gap/element.pyx',
    - 'sage/libs/gap/libgap.pyx',
    - 'sage/libs/gap/util.pyx',
    - 'sage/matrix/matrix_gap.pyx',
    - 'sage/sets/disjoint_set.pyx'

To test:

  • make SAGE_WHEELS=yes SAGE_CHECK=yes sagemath_flint_arb
  • make SAGE_WHEELS=yes SAGE_CHECK=yes sagemath_gap
  • make SAGE_WHEELS=yes SAGE_CHECK=yes sagemath_giac
  • make SAGE_WHEELS=yes SAGE_CHECK=yes sagemath_homfly

In follow-up tickets:

  • sagemath-brial (ex sage-brial in sage.rings.polynomial.pbori + sage.libs.polybori, sage.crypto.boolean_function)
  • sagemath-ntl
  • sagemath-ecl (sage.libs.ecl)
  • sagemath-maxima: requires sage-ecl
  • sagemath-singular: (some of these files actually may be better put in the pynac distribution)
    - 'sage/algebras/letterplace/free_algebra_element_letterplace.pyx',
    - 'sage/algebras/letterplace/free_algebra_letterplace.pyx',
    - 'sage/algebras/letterplace/letterplace_ideal.pyx',
    - 'sage/libs/pynac/constant.pyx',
    - 'sage/libs/pynac/pynac.pyx',
    - 'sage/libs/singular/function.pyx',
    - 'sage/libs/singular/groebner_strategy.pyx',
    - 'sage/libs/singular/option.pyx',
    - 'sage/libs/singular/polynomial.pyx',
    - 'sage/libs/singular/ring.pyx',
    - 'sage/libs/singular/singular.pyx',
    - 'sage/matrix/matrix_mpolynomial_dense.pyx',
    - 'sage/rings/polynomial/multi_polynomial_ideal_libsingular.pyx',
    - 'sage/rings/polynomial/multi_polynomial_libsingular.pyx',
    - 'sage/rings/polynomial/plural.pyx',
    - 'sage/symbolic/comparison.pyx',
    - 'sage/symbolic/constants_c.pyx',
    - 'sage/symbolic/expression.pyx',
    - 'sage/symbolic/function.pyx',
    - 'sage/symbolic/getitem.pyx',
    - 'sage/symbolic/ring.pyx',
    - 'sage/symbolic/series.pyx',
    - 'sage/symbolic/substitution_map.pyx',
  • sagemath-pynac
  • sagemath-linbox:
    - 'sage/libs/linbox/linbox_flint_interface.pyx',
    - 'sage/matrix/matrix_integer_sparse.pyx',
    - 'sage/matrix/matrix_modn_dense_double.pyx',
    - 'sage/matrix/matrix_modn_dense_float.pyx',
    - 'sage/matrix/matrix_modn_sparse.pyx'
  • sagemath-znpoly (subject to removal in zn_poly removal #32841?)
    - 'sage/modular/modsym/p1list.pyx',
    - 'sage/modular/pollack_stevens/dist.pyx',
    - 'sage/rings/fraction_field_FpT.pyx',
    - 'sage/rings/polynomial/polynomial_zmod_flint.pyx',
    - 'sage/schemes/hyperelliptic_curves/hypellfrob.pyx',
  • sagemath-mari (also requiring pbori and pbori-groebner):
    - 'sage/matrix/matrix_gf2e_dense.pyx',
    - 'sage/matrix/matrix_integer_dense.pyx',
    - 'sage/matrix/matrix_mod2_dense.pyx',
    - 'sage/matrix/matrix_rational_dense.pyx',
    - 'sage/modules/vector_mod2_dense.pyx',
    - 'sage/rings/polynomial/pbori/pbori.pyx',
    - 'sage/rings/polynomial/polynomial_gf2x.pyx'

This is preparation for Meta-ticket #29705 (Modularization) and #30371.

Depends on #31031
Depends on #34855
Depends on #34839

CC: @tobiasdiez @dimpase

Component: refactoring

Keywords: sd111

Author: Matthias Koeppe

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Sep 26, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@tobiasdiez
Copy link
Contributor

comment:5

Good idea to separate this into a new ticket.

While working on #30371, I noticed that most cython files using singular cannot be compiled without pynac. So maybe these should be combined.

Also, I think, there should be distributions for GAP, CCObject, linbox, ratpoints, zn_poly and mari.

What do you think about renaming sage_setup: distribution to sage_setup: dependency? I think this better reflects the purpose. The distribution is then generated based on which dependencies are available. Another idea would be to use distutils: libraries which already contains some of the dependencies anyway (e.g. arb, gmp, flint).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 26, 2020

comment:6

Replying to @tobiasdiez:

While working on #30371, I noticed that most cython files using singular cannot be compiled without pynac. So maybe these should be combined.

I think they should be separate because I really want to be able to make singular optional. pynac, on the other hand, is essential for our symbolics implementation.

Also, I think, there should be distributions for GAP, CCObject, linbox, ratpoints, zn_poly and mari.

Yes, please add to the list in the ticket description with explanation (what is CCObject?)

What do you think about renaming sage_setup: distribution to sage_setup: dependency?

No, I wouldn't like that. As soon as you start with declaring "dependencies", there will be combinatorial explosion from the combinations of such dependencies. I really want a well-designed partition of the source files into (relatively few) disjoint distributions. This is crucial for the modularization -- each distribution will eventually be separately compilable and installable package on PyPI.

@tobiasdiez

This comment has been minimized.

@tobiasdiez
Copy link
Contributor

comment:8

Ok, makes sense.
Which distribution should be assigned to files that have two dependencies (e.g use pynac and singular)?

My idea was that the cython file declares which dependencies it has, and in the setup.py file you have a list of distributions and their dependencies. If all dependencies are met, that particular distribution is built. E.g. distribution "sage-singular" relies on "pynac" and "singular". (That's only a feeling, but it feels strange to have distributions determined by dependencies and not by a theme or subject, e.g. "sage-singular" vs "sage-symbolic calculation". But you have a better overview, so I trust you there.)

I've added the above distributions to the list.
(Forget about CCObject, it was a missing import in #30371 that confused me.)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 27, 2020

comment:9

Replying to @tobiasdiez:

Which distribution should be assigned to files that have two dependencies (e.g use pynac and singular)?

Distributions can certainly depend on other distributions. sage-singular could depend on sage-pynac. Let's include this in the ticket description. (In the modularized build, sage-singular's setup.py or setup.cfg would have install_requires = sage-pynac.)

Also, often it is relatively easy to remove some of these dependencies by small changes to a few source files, moving a few methods around. See for example #29911. We can take work on this in follow-up tickets.

it feels strange to have distributions determined by dependencies and not by a theme or subject, e.g. "sage-singular" vs "sage-symbolic calculation".

Perhaps, but there is actually an important "political" (open source community) reason for this design too. By naming these components after major libraries that provide the features, we make these libraries and their important role for Sage more visible to end users. Attribution tends to be an important motivating factor to developers of these libraries.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 27, 2020

comment:10

To expand on this a little bit: I wouldn't rule out distributions named after topics. The present ticket only provides a "skeleton" of distributions that are (1) technically needed to take care of compile-time dependencies, (2) provides attribution. Later on (in follow-up tickets), for example, there could be a distribution sage-feature-padics or something like this that has sage-ntl as a dependency and packages sage.rings.padics.

@tobiasdiez

This comment has been minimized.

@tobiasdiez

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2020

comment:13

Replying to @mkoeppe:

Replying to @tobiasdiez:

While working on #30371, I noticed that most cython files using singular cannot be compiled without pynac. So maybe these should be combined.

I think they should be separate because I really want to be able to make singular optional. pynac, on the other hand, is essential for our symbolics implementation.

When I wrote this, I didn't recall that singular's libfactory is actually a dependency of pynac. So I think you are right, it's best to have pynac and singular in one distribution (for now, at least).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

Changed keywords from none to sd111

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 9, 2020

Dependencies: #31031

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2021

comment:17

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 14, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 1, 2022
@mkoeppe mkoeppe removed this from the sage-9.7 milestone Aug 31, 2022
@dimpase
Copy link
Member

dimpase commented Dec 7, 2022

comment:53

there are a number of files with m4 suffix, which does not seem to be m4 files.
Should they rather get in suffux?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

comment:54

They are processed by m4, see build/pkgs/sagelib/bootstrap

@dimpase
Copy link
Member

dimpase commented Dec 7, 2022

comment:55

Imho they should have .in suffix. Otherwise, looks OK.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

comment:56

The .in files in use with autotools are not processed by m4.

These .m4 files are processed by m4. For example, in setup.cfg.m4 you'll see the m4 macro call esyscmd.

@dimpase
Copy link
Member

dimpase commented Dec 7, 2022

comment:57

ah, OK. I misunderstood what "processed" means in this context.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

comment:58

Thanks!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

Reviewer: Dima Pasechnik

@kiwifb
Copy link
Member

kiwifb commented Dec 13, 2022

comment:60

When I build the full sagelib with this ticket included (but I do not build those sub distribution yet) in sage-on-gentoo, building the documentation fails. After inspection, I have the sources installed for sage/libs/flint, sage/libs/arb etc, but none of compiled module are installed. Is there something I am missing or doing wrong?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2022

comment:61

Are you building from src/ or from pkgs/sagemath-standard/?

@kiwifb
Copy link
Member

kiwifb commented Dec 13, 2022

comment:62

src with the setup.py from sagemath-standard. This is when I build against develop or the Volker merging branch. The main reason I am starting from src in those cases, is that patch does not like links.

@kiwifb
Copy link
Member

kiwifb commented Dec 13, 2022

comment:63

I also delete src/sage_setup so it is not used instead of the installed one.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2022

comment:64

OK, that's something that I'll have to fix in pkgs/sagemath-standard/setup.py.
Thanks for catching this

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 14, 2022

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

790c12bMerge tag '9.8.beta5' into t/30666/add__sage_setup__distribution__directives_to_all_cython_modules_needing_external_libraries
8b1cf25pkgs/sagemath-standard/setup.py: Include files marked as # sage_setup: distribution = sagemath-flint-arb etc.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 14, 2022

Changed commit from 112e29e to 8b1cf25

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 19, 2022

Changed dependencies from #31031 to #31031, #34855

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 19, 2022

Changed commit from 8b1cf25 to 1531fda

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 19, 2022

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

2b39b3dMerge #34858
c59622abuild/pkgs/sagemath*/spkg-src: Use build
7fc93d0build/pkgs/sagemath*/spkg-src: Use --skip-dependency-check
4940db4build/make/Makefile.in: Add targets SPKG-src for script packages
573fcfdbuild/make/Makefile.in: Move targets pypi-sdists, wheel, pypi-wheels here; use SPKG-sdist targets
88e18d0Merge #34855
0bd13acbuild/make/Makefile.in: Remove duplicate setting of PYPI_WHEEL_PACKAGES
f712d67Merge #34855
5fc1addbuild/pkgs/sagemath*/spkg-src: Use build
1531fdabuild/pkgs/sagemath_flint_arb/dependencies: Fix up

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 20, 2022

Changed dependencies from #31031, #34855 to #31031, #34855, #34839

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 23, 2023

Removed branch from issue description; replaced by PR #35663

@mkoeppe mkoeppe removed this from the sage-10.1 milestone Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment