`sage.knots`: Modularization fixes (imports), `# needs` #38118

mkoeppe · 2024-05-31T01:57:40Z

Also some doctest cosmetics.

Cherry-picked from Modularization of sagelib: Distributions sagemath-{brial,combinat,eclib,flint,gap,giac,glpk,graphs,groups,homfly,lcalc,libbraiding,libecm,linbox,modules,mpmath,ntl,pari,plot,polyhedra,schemes,singular,standard-no-symbolics,symbolics} #35095.

sage --fixdoctests --verbose now shows doctest tagging statistics.

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

⌛ Dependencies

github-actions · 2024-05-31T02:36:42Z

Documentation preview for this PR (built with commit b196b22; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

src/sage/knots/link.py

kwankyu · 2024-05-31T06:09:14Z

What distribution package (K) provides sage.knots?

knot theory heavily depends on groups. I think the distribution package (K) should depend on the distribution package (G) that provides groups. What is (G)?

mkoeppe · 2024-05-31T06:19:26Z

In #35095, I have sage.knots in sagemath-graphs

kwankyu · 2024-05-31T06:33:52Z

I think sage.knots should be in sagemath-standard-no-symbolics for now. I am not an expert, of course, but I guess knot theorist would not regard their subject as part of graph theory.

kwankyu · 2024-05-31T06:36:08Z

@soehms ?

mkoeppe · 2024-05-31T06:50:48Z

Per design of the modularized distributions, sagemath-graphs is not a package only for "graph theory". Rather it's a package that defines graphs and recognizable generalizations thereof and contains code that directly makes use of such structures. And the most basic code in sage.knots.link is built on DiGraph (see for example the methods Link.arcs).

kwankyu · 2024-05-31T07:10:49Z

Yes. The theory of knots and links uses graphs. But more importantly it uses groups, braid groups, and also polynomials.

... it's a package that defines graphs and recognizable generalizations thereof

Knots and links are not generalizations of graphs...

mkoeppe · 2024-05-31T07:29:17Z

this part applies to sage.knots:

contains code that directly makes use of such structures.

kwankyu · 2024-05-31T07:54:27Z

Similarly sage.knots makes heavy use of groups, which is why we are forced to add # needs sage.groups to so many doctests.

Let's wait for an opinion from an expert though.

kwankyu · 2024-05-31T07:59:06Z

By the way, there are 39 instances of the word "graph" in sage.knots while there are 161 of "group".

soehms · 2024-06-03T16:59:21Z

Similarly sage.knots makes heavy use of groups, which is why we are forced to add # needs sage.groups to so many doctests.

Let's wait for an opinion from an expert though.

I'm not really a knot theory expert either, but I think to know enought about the knots subfolder to discuss the question.

I think sage.knots should be in sagemath-standard-no-symbolics for now. I am not an expert, of course, but I guess knot theorist would not regard their subject as part of graph theory.

That's right. According to Wikipedia knot theory is a part of Geometric toplology. On that page it is listed as a subfield of its own, while on the Low-dimensional topology page it is mentioned as a subfield as well.

As you already explained the connections to graph theory, group theory and polynomial rings just appear to be tool-boxes.

Maybe the problem with modularization is an inconsistency between the developer's and the user's perspective. If a user interested in knot theory or geometric topology doesn't want to install the whole of the Sage library to work with topologically relevant parts of it, it won't help him that we've split the functionality in terms of dependencies.

To facilitate modularization for such use cases, we should find a way to declare meta-packages as collections of existing standard and optional packages that define parts of Sage that cover a certain mathematical area. For example, such a meta-package for knot theory would consist of the standard packages sagemath-standard, sagemath__graphs, sagemath__groups, sagemath__rings... and optional packages like snappy, database_knotinfo...

But I'm not sure if such use cases are intended by our modularization project. If modularization only adresses developers, then I agree that we should not give preference to any of the tool-boxes (e.g. sagemath__graphs over sagemath__groups).

Other opinions, @culler , @NathanDunfield , @tscrim , @miguelmarco , @fchapoton ?

mkoeppe · 2024-06-03T17:52:55Z

@soehms Thanks for sharing this analysis.

Maybe the problem with modularization is an inconsistency between the developer's and the user's perspective. If a user interested in knot theory or geometric topology doesn't want to install the whole of the Sage library to work with topologically relevant parts of it, it won't help him that we've split the functionality in terms of dependencies.

To facilitate modularization for such use cases, we should find a way to declare meta-packages as collections of existing standard and optional packages that define parts of Sage that cover a certain mathematical area. For example, such a meta-package for knot theory [...] But I'm not sure if such use cases are intended by our modularization project. If modularization only adresses developers, then I agree that we should not give preference to any of the tool-boxes (e.g. sagemath__graphs over sagemath__groups).

It's true that my development focus in this "modularization" phase of the modularization project has been in getting the modularized distributions to ship and build in isolation. The dependencies are the hard technical constraints for this, and this is where the main technical difficulty lies. I have done most of this work already by myself (with the help of all who have supported the upstreaming of this work in their roles as PR reviewers); it's a necessary "bootstrapping" step.

Sometimes decisions have to be taken that are arbitrary to some degree. So there is a component (knots) that has two hard runtime dependencies, groups and graphs. Well, one of the distributions has to ship it. It would be a mistake to defer the decision and keep knots for now in a catch-all, not-yet-modularized distribution until "we know better". Breaking the vicious cycle, making everything shippable in practical distributions, is exactly the point of this phase.

Defining convenient and stable ways to advertise functionality to users is a largely separate question, and this is one where I indeed hope that communities of domain experts -- developers and users -- will start to engage with the modularization project. This is really the "integration" phase of the modularization. Most of the technical difficulty is gone: defining meta-packages is just a matter of writing a pyproject.toml file; with "optional dependencies" that advertise features. So a different hard problem comes to the fore: Reaching out to users, shaping and leading communities.

NathanDunfield · 2024-06-03T17:58:44Z

I agree with @soehms and @kwankyu that, mathematically, knot theory is not usually viewed as part of (or even strongly connected to) graph theory. There's more overlap with modern combinatorial/geometric group theory, especially the aspects that sage.knots implements.

On the technical question of where sage.knots should go in the modularization, I have no informed opinion.

culler · 2024-06-03T18:09:41Z

Here is my position on modularization: 1. Modularization is NOT for users. A user who wants to use Sage should install Sage. Period. (And the Sage project should put effort into making it as easy as possible to install Sage.) 2. Modularization is very useful for developers who would like to include some parts of Sage, either as a dependency or as a "vendorization" in the project that the developer is developing. Including all of Sage, in either way, turns small or medium sized projects into enormous projects and therefore is not practical. A guiding example is CyPari, which is a dependency of SnapPy and also is embedded in the SnapPy app. - Marc PS Classifying the sub-disciplines of mathematics is hard, and is not the job of the Sage developers, and should not be a prerequisite of the modularization project. The decision of which Sage components to group into a module should be based on the software. The decision of which Sage modules to include in a given project should be made by the project developer based on what the project needs.

…

On Mon, Jun 3, 2024 at 12:59 PM Nathan Dunfield ***@***.***> wrote: I agree with @soehms <https://github.com/soehms> and @kwankyu <https://github.com/kwankyu> that, mathematically, knot theory is not usually viewed as part of (or even strongly connected to) graph theory. There's more overlap with modern combinatorial/geometric group theory, especially the aspects that sage.knots implements. On the technical question of where sage.knots should go in the modularization, I have no informed opinion. — Reply to this email directly, view it on GitHub <#38118 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ6CP73LUQUGU4M7SZS7TDZFSVGTAVCNFSM6AAAAABIR6FH4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBVHAYDONRXGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

mkoeppe · 2024-06-03T18:22:21Z

I agree with most of what @culler wrote, except to caution that the word "user" means many different things to different people. And the transition from (1) being merely a user of software to (2) sharing a script to (3) publishing a package in the Python "ecosystem" is soft and gradual, and supporting this transition when Sage is part of the equation is one of the goals of the modularization project.

kwankyu · 2024-06-04T05:47:06Z

First let us note that some distribution packages are already in sage, and some other distribution packages are yet either in Matthias' repo or in future. All these collectively form the basic products of the modularization project. This PR is about preparation of the sage library for the modularization project, in particular, related with the python module sage.knots.

... I [Matthias] have done most of this work already by myself ...

Yes. Thanks for that. However, I think this is the source of problems about the modularization project. People in the sage community did not engage in the design of the modularization of the sage library. For the project to proceed, I think it is the time when the design (and the design principles) is shared with the community, discussed and reflects the results of the discussion. Both of the users and developers of sage should acknowledge the project and become supporters of the modularized sage. Even accepting and not being hostile to # needs tags scattered around doctests needs support of all the sage community.

As we are on this PR, we may take sage.knots as the target of the discussion.

In the current design, sage.knots is in the distribution (package) sagemath-graphs, according to the diagram in #35095.

This PR introduces too many # needs sage.groups tags in sage.knots, which I think signals bad design. The central question is whether sagemath-graphs is the right distribution for sage.knots.

According to #35095, it contains some of sage.combinat, most of sage.graphs, sage.topology, and sage.knots. The module sage.topology is mostly simplicial complex machinery, which uses graphs. Hence sage.topology is really about algebraic topology.

A developer developing his own software may depend on sagemath-graphs. Then he is the developer of his software and at the same time a user of the distribution package sagemath-graphs. The user expects certain functionality of sagemath-graphs from its name, in this case, graphs; sage.topology and sage.knots are not modules expected to be found in sagemath-graphs.

... So there is a component (knots) that has two hard runtime dependencies, groups and graphs. Well, one of the distributions has to ship it.

Why? sage.knots uses groups and graphs, but does not belong to graphs and groups.

It would be a mistake to defer the decision and keep knots for now in a catch-all, not-yet-modularized distribution until "we know better".

If we want to ship sage.knots in a smaller distribution than sagemath-standard-no-symbolics, we may have sagemath-topology that contains sage.topology and sage.knots and depends on sagemath-graphs and sagemath-groups.

Defining convenient and stable ways to advertise functionality to users is a largely separate question, and this is one where I indeed hope that communities of domain experts -- developers and users -- will start to engage with the modularization project.

The first phase will be over when the distribution packages shipping hard dependencies (the blue boxes in the diagram of
#35095) are in sage. People should engage in the design phase of all distributions named sagemath-mathematical-subject. That is now.

... Most of the technical difficulty is gone: defining meta-packages is just a matter of writing a pyproject.toml file; with "optional dependencies" that advertise features. So a different hard problem comes to the fore: Reaching out to users, shaping and leading communities.

Having people wait until the phase of defining "meta-packages" and just accept your design as done in #35095 seems to be unrealistic strategy to proceed the modularization project, as I think the recent disputed PRs manifest.

mkoeppe · 2024-06-04T06:15:22Z

Having people wait until the phase of defining "meta-packages" and just accept your design as done in #35095 seems to be unrealistic strategy to proceed the modularization project, as I think the recent disputed PRs manifest.

I disagree. These so-called "disputes" have no legitimacy whatsoever. The project will not be governed by these expressions of disrespect and hostility.

mkoeppe · 2024-06-04T06:26:36Z

... So there is a component (knots) that has two hard runtime dependencies, groups and graphs. Well, one of the distributions has to ship it.

Why? sage.knots uses groups and graphs, but does not belong to graphs and groups.

I prefer not to engage with such statements of what belongs and what does not.
We have agency there as the developers.

One important criterion is the overall complexity -- how many distributions we create.

kwankyu · 2024-06-04T06:53:49Z

Having people wait until the phase of defining "meta-packages" and just accept your design as done in #35095 seems to be unrealistic strategy to proceed the modularization project, as I think the recent disputed PRs manifest.

I disagree. These so-called "disputes" have no legitimacy whatsoever. The project will not be governed by these expressions of disrespect and hostility.

Sorry that they remind you of the disrespect and hostility. Of course, it has no place in our community. I just wanted to mention that not a few people have hostile (or at least unfriendly) attitude towards the modularization itself. I think it is important for the community to embrace the modularization project as their own mission.

kwankyu · 2024-06-04T07:02:51Z

One important criterion is the overall complexity -- how many distributions we create.

Reducing # needs tags (and keeping only reasonable ones) decreases the overall complexity. Actually this is my main concern, reviewing this PR.

soehms · 2024-06-04T21:28:09Z

Here is my position on modularization: 1. Modularization is NOT for users. A user who wants to use Sage should install Sage. Period. (And the Sage project should put effort into making it as easy as possible to install Sage.)

I agree with that 99%, especially the part in parentheses 100%. On the other hand, some users may not want to spend that much disk space, so why not have an option for them?

soehms · 2024-06-04T21:31:15Z

If we want to ship sage.knots in a smaller distribution than sagemath-standard-no-symbolics, we may have sagemath-topology that contains sage.topology and sage.knots and depends on sagemath-graphs and sagemath-groups.

From a mathematical point of view, this sounds reasonable to me, even though there are no direct dependencies on sage.topology. Could we add more dependencies to such a distribution package, for example sagemath-modules because of sage.homology...? Unfortunately I can't comment on the technical difficulties. But if they are limited, this would be a solution I prefer.

soehms · 2024-06-24T16:52:29Z

I've made the suggested change for link.py. This changes the statistics to the following:

Is there anything wrong with doing the same for knot.py and knotinfo.py?

mkoeppe · 2024-06-24T17:15:39Z

The downside of using the file-level doctest tag is that

it is no longer documented what parts of the functionality uses sage.groups
the parts that do not need groups are no longer tested when sage.groups is not available

It's a tradeoff.

If your preference is to change them to file-level tags, I can make this change.

mkoeppe · 2024-06-24T19:32:48Z

$ ./sage -fixdoctests --no-test --overwrite --verbose src/sage/knots/{knot.py,knotinfo.py}
sage-fixdoctests: Processing unprocessed files
sage-fixdoctests: No fixes made in 'src/sage/knots/knot.py'
File tags: 
       sage.graphs sage.groups
Doctest blocks by persistent tags: 
    35 (untagged)
Doctest examples by tags: 
    95 (untagged)
     1 long time
sage-fixdoctests: No fixes made in 'src/sage/knots/knotinfo.py'
File tags: 
       sage.graphs sage.groups
Doctest blocks by persistent tags: 
   111 (untagged)
     2 database_knotinfo
     1 snappy
     1 not tested
     1 sage.symbolic
Doctest examples by tags: 
   317 (untagged)
    30 database_knotinfo
    14 sage.symbolic
    10 not tested
     9 snappy

soehms · 2024-06-28T18:02:37Z

The downside of using the file-level doctest tag is that

it is no longer documented what parts of the functionality uses sage.groups

the parts that do not need groups are no longer tested when sage.groups is not available

It's a tradeoff.

If your preference is to change them to file-level tags, I can make this change.

I think your second point won't be a problem as long as there are CI runs testing the entire Sage library.

As for your first point, we lose the work you've already done for this PR. On the other hand, we'll need more work in the future to keep this information valid.

So the question is, how useful is the information we lose?

According to your statistics, there are many more untagged doctests than those tagged with sage.groups. This looks as if we single out side-cases. However, I doubt there are many non-trivial applications of these modules that don't need sage.groups or sage.modules. So I prefer the file-level tags.

mkoeppe · 2024-06-28T18:05:09Z

This sounds reasonable to me, thanks.

kwankyu · 2024-07-16T01:41:24Z

diff --git a/src/sage/knots/knotinfo.py b/src/sage/knots/knotinfo.py
index 6bc9c3a09cf..7942a7092a2 100644
--- a/src/sage/knots/knotinfo.py
+++ b/src/sage/knots/knotinfo.py
@@ -1521,7 +1521,7 @@ class KnotInfoBase(Enum):
 
             sage: K3_1  = KnotInfo.K3_1
             sage: K3_1j = K3_1.jones_polynomial()                                       # needs sage.symbolic
-            sage: L2a1_1j = Ljt     # see note above
+            sage: L2a1_1j = Ljt     # see note above                                    # needs sage.symbolic
             sage: R = L2a1_1j.parent()
             sage: Oj = R(1)
             sage: t = R('t')

from

File "src/sage/knots/knotinfo.py", line 1524, in sage.knots.knotinfo.KnotInfoBase.jones_polynomial
Warning: Variable 'Ljt' referenced here was set only in doctest marked '# needs sage.graphs sage.groups sage.symbolic'
    L2a1_1j = Ljt     # see note above

kwankyu · 2024-07-16T01:45:06Z

In

$ sage --fixdoctests --verbose src/sage/knots/knot.py 
Running "sage -t -p src/sage/knots/knot.py"
sage-fixdoctests: Processing unprocessed files
sage-fixdoctests: No fixes made in 'src/sage/knots/knot.py'
File tags: 
       sage.graphs sage.groups
Doctest blocks by persistent tags: 
    35 (untagged)
Doctest examples by tags: 
    95 (untagged)
     1 long time

Doctest lines (parallel to Doctest blocks) would make more sense than Doctest examples. No?

kwankyu · 2024-07-16T01:46:21Z

Otherwise lgtm. Thanks for switching to file-level tags.

kwankyu

Thanks. LGTM.

mkoeppe · 2024-07-17T08:38:12Z

Thank you!

mkoeppe · 2024-07-17T08:38:48Z

Doctest lines (parallel to Doctest blocks) would make more sense than Doctest examples. No?

"Examples" is what the doctest code calls them.

kwankyu · 2024-07-17T08:54:40Z

All right. But I think "example" is ambiguous as a unit of measure (outside of the code)...

mkoeppe added c: algebraic topology t: refactoring labels May 31, 2024

mkoeppe self-assigned this May 31, 2024

github-actions bot added the v: large label May 31, 2024

mkoeppe added the s: needs review label May 31, 2024

kwankyu reviewed May 31, 2024

View reviewed changes

src/sage/knots/link.py Outdated Show resolved Hide resolved

mkoeppe force-pushed the sage_knots_mod branch from 9a45175 to ecbd34d Compare June 1, 2024 23:30

vbraun force-pushed the develop branch from eba5e19 to e5f42fa Compare June 3, 2024 22:15

Matthias Koeppe added 7 commits June 22, 2024 16:14

Add feature 'sage.libs.homfly'

38b5616

src/sage/knots/link.py: Fix indent of doctest

0a33320

src/sage/knots/knotinfo.py: Fix # needs

d6ccaef

src/sage/knots/link.py: Fix # needs, use block tag

f33e981

sage-fixdoctests --verbose: Show statistics on doctest tags

c87a39b

src/sage/knots: Add file-level doctest tag '# needs sage.graphs'

784d0aa

src/sage/knots/link.py: Use file-level # needs sage.groups

50c83dc

mkoeppe force-pushed the sage_knots_mod branch from e281ead to 50c83dc Compare June 22, 2024 23:14

src/sage/knots/{knot,knotinfo}.py: Use file-level # needs sage.groups

acf9c5a

kwankyu and others added 2 commits July 16, 2024 10:46

Merge branch 'develop' into sage_knots_mod

4013dfe

src/sage/knots/knotinfo.py: Use block # needs

b196b22

kwankyu approved these changes Jul 17, 2024

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Jul 17, 2024

vbraun merged commit bd141ad into sagemath:develop Jul 24, 2024
21 checks passed

github-actions bot removed the s: positive review label Jul 24, 2024

mkoeppe added this to the sage-10.5 milestone Jul 25, 2024

mkoeppe deleted the sage_knots_mod branch July 25, 2024 09:31

soehms mentioned this pull request Aug 6, 2024

K5_2 signature change soehms/database_knotinfo#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`sage.knots`: Modularization fixes (imports), `# needs` #38118

`sage.knots`: Modularization fixes (imports), `# needs` #38118

mkoeppe commented May 31, 2024 •

edited

Loading

github-actions bot commented May 31, 2024 •

edited

Loading

kwankyu commented May 31, 2024 •

edited

Loading

mkoeppe commented May 31, 2024

kwankyu commented May 31, 2024

kwankyu commented May 31, 2024

mkoeppe commented May 31, 2024

kwankyu commented May 31, 2024 •

edited

Loading

mkoeppe commented May 31, 2024

kwankyu commented May 31, 2024

kwankyu commented May 31, 2024

soehms commented Jun 3, 2024

mkoeppe commented Jun 3, 2024

NathanDunfield commented Jun 3, 2024

culler commented Jun 3, 2024 via email

mkoeppe commented Jun 3, 2024

kwankyu commented Jun 4, 2024 •

edited

Loading

mkoeppe commented Jun 4, 2024

mkoeppe commented Jun 4, 2024

kwankyu commented Jun 4, 2024

kwankyu commented Jun 4, 2024 •

edited

Loading

soehms commented Jun 4, 2024

soehms commented Jun 4, 2024

soehms commented Jun 24, 2024

mkoeppe commented Jun 24, 2024

mkoeppe commented Jun 24, 2024

soehms commented Jun 28, 2024

mkoeppe commented Jun 28, 2024

kwankyu commented Jul 16, 2024

kwankyu commented Jul 16, 2024

kwankyu commented Jul 16, 2024 •

edited

Loading

kwankyu left a comment

mkoeppe commented Jul 17, 2024

mkoeppe commented Jul 17, 2024

kwankyu commented Jul 17, 2024 •

edited

Loading

sage.knots: Modularization fixes (imports), # needs #38118

sage.knots: Modularization fixes (imports), # needs #38118

Conversation

mkoeppe commented May 31, 2024 • edited Loading

📝 Checklist

⌛ Dependencies

github-actions bot commented May 31, 2024 • edited Loading

kwankyu commented May 31, 2024 • edited Loading

mkoeppe commented May 31, 2024

kwankyu commented May 31, 2024

kwankyu commented May 31, 2024

mkoeppe commented May 31, 2024

kwankyu commented May 31, 2024 • edited Loading

mkoeppe commented May 31, 2024

kwankyu commented May 31, 2024

kwankyu commented May 31, 2024

soehms commented Jun 3, 2024

mkoeppe commented Jun 3, 2024

NathanDunfield commented Jun 3, 2024

culler commented Jun 3, 2024 via email

mkoeppe commented Jun 3, 2024

kwankyu commented Jun 4, 2024 • edited Loading

mkoeppe commented Jun 4, 2024

mkoeppe commented Jun 4, 2024

kwankyu commented Jun 4, 2024

kwankyu commented Jun 4, 2024 • edited Loading

soehms commented Jun 4, 2024

soehms commented Jun 4, 2024

soehms commented Jun 24, 2024

mkoeppe commented Jun 24, 2024

mkoeppe commented Jun 24, 2024

soehms commented Jun 28, 2024

mkoeppe commented Jun 28, 2024

kwankyu commented Jul 16, 2024

kwankyu commented Jul 16, 2024

kwankyu commented Jul 16, 2024 • edited Loading

kwankyu left a comment

Choose a reason for hiding this comment

mkoeppe commented Jul 17, 2024

mkoeppe commented Jul 17, 2024

kwankyu commented Jul 17, 2024 • edited Loading

`sage.knots`: Modularization fixes (imports), `# needs` #38118

`sage.knots`: Modularization fixes (imports), `# needs` #38118

mkoeppe commented May 31, 2024 •

edited

Loading

github-actions bot commented May 31, 2024 •

edited

Loading

kwankyu commented May 31, 2024 •

edited

Loading

kwankyu commented May 31, 2024 •

edited

Loading

kwankyu commented Jun 4, 2024 •

edited

Loading

kwankyu commented Jun 4, 2024 •

edited

Loading

kwankyu commented Jul 16, 2024 •

edited

Loading

kwankyu commented Jul 17, 2024 •

edited

Loading