Opened 6 months ago

Last modified 4 weeks ago

#32242 needs_review enhancement

Use GNU Info to parse singular's Info file

Reported by: mjo Owned by:
Priority: major Milestone: sage-9.6
Component: packages: standard Keywords:
Cc: Merged in:
Authors: Michael Orlitzky Reviewers:
Report Upstream: N/A Work issues:
Branch: u/mjo/ticket/32242 (Commits, GitHub, GitLab) Commit: 137a9182a02d4aa329a78c2e936b48431c1e9017
Dependencies: #32302 Stopgaps:

Status badges

Description (last modified by mjo)

Our Singular interface currently contains a hand-written parser for Singular's "info" file. In this ticket, we add an optional GNU Info package to parse this file instead.

Relative to running "info", the hand-written parser has several drawbacks:

  • The extra code is a maintenance burden. We should not be wasting our time (poorly) reimplementing standard tools.
  • The custom parser is buggy. For example, it is supposed to raise a KeyError when documentation for a non-existent function is requested. However, the parser does not keep track of what section it's in, so, for example, get_docstring("Preface") will happily return the contents of the preface even though "Preface" is not a Singular function.
  • The first time documentation is requested, the entire info file (see above) is loaded into a dictionary. With the Singular info file currently weighing in at around 11MiB, this is wasting a good chunk of memory.
  • The singular_console() itself tries to launch GNU Info to display its interactive help, and cannot use our hand-written parser.

The one downside to requiring "info" is that it would add a new runtime dependency to sagelib. To avoid that, we make GNU Info optional, and suggest to the user that he install it when he tries to read Singular's documentation.

GNU Info (or its superset, Texinfo) are widespread, portable, and easy to install on all of the systems we support, so in most cases this should be a "free" improvement.

Change History (32)

comment:1 Changed 6 months ago by mjo

  • Authors set to Michael Orlitzky
  • Branch set to u/mjo/ticket/32242
  • Commit set to fc7fd02b833b2f76755f1e50a28dd7dd566962e5
  • Status changed from new to needs_review

New commits:

285d8acTrac #XXXXX: new package for the standalone GNU Info reader.
21586aaTrac #XXXXX: drop SINGULAR_EXECUTABLE from src/bin/sage-env.
fc7fd02Trac #XXXXX: use GNU Info to obtain Singular docstrings.

comment:2 Changed 6 months ago by git

  • Commit changed from fc7fd02b833b2f76755f1e50a28dd7dd566962e5 to c4f693803fdc7deb904bb85c7352f6327f338a66

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4768540Trac #32242: new package for the standalone GNU Info reader.
47eb391Trac #32242: drop SINGULAR_EXECUTABLE from src/bin/sage-env.
c4f6938Trac #32242: use GNU Info to obtain Singular docstrings.

comment:3 Changed 6 months ago by mkoeppe

I don't know... shouldn't there be a feature request / PR to Singular to make the docstrings available via API?

comment:4 Changed 6 months ago by mjo

Singular renders its own interactive help using "info" or one of its alternatives: https://www.singular.uni-kl.de/Manual/latest/sing_16.htm

I don't think they can give us an API without making "info" a dependency of Singular itself.

comment:5 follow-up: Changed 6 months ago by mkoeppe

OK I see.

I still don't get why it's necessary or helpful to replace our parser with calling the info program.

Wouldn't it be much easier to just obtain the help file location from Singular itself?

comment:6 follow-up: Changed 6 months ago by mkoeppe

I would also think it's pretty problematic from the viewpoint of modularization. We may soon have a pip-installable package for the Sage functionality that uses Singular. The wheel could then vendor libsingular. But vendoring the info program? That would be rather strange.

comment:7 in reply to: ↑ 5 ; follow-up: Changed 6 months ago by mjo

Replying to mkoeppe:

OK I see.

I still don't get why it's necessary or helpful to replace our parser with calling the info program.

The interface gets a lot smaller and simpler.

Wouldn't it be much easier to just obtain the help file location from Singular itself?

Singular does know where it lives, but how do you propose we get it? Ask them to add a special command-line option to support our outlandish use case?

comment:8 in reply to: ↑ 6 Changed 6 months ago by mjo

Replying to mkoeppe:

I would also think it's pretty problematic from the viewpoint of modularization. We may soon have a pip-installable package for the Sage functionality that uses Singular. The wheel could then vendor libsingular. But vendoring the info program? That would be rather strange.

Leave it as part of the main Sage package, assumed to be there by the Singular wheel? In almost all cases, the system Info will be used anyway. But I don't really know enough about what you're trying to do to make a good suggestion.

comment:9 in reply to: ↑ 7 ; follow-up: Changed 6 months ago by mkoeppe

Replying to mjo:

Singular does know where it lives, but how do you propose we get it?

Use feGetResource in libsingular_resources?

comment:10 in reply to: ↑ 9 Changed 6 months ago by mjo

Replying to mkoeppe:

Replying to mjo:

Singular does know where it lives, but how do you propose we get it?

Use feGetResource in libsingular_resources?

Looks like it will work, thanks. I still think it's kinda dumb to poorly reimplement a ubiquitous 300k tool in math software, but it's good to know that we can find the help file if we really need to.

comment:11 Changed 6 months ago by mkoeppe

info is a well-documented, stable, extremely simple file format, I don't think there's anything wrong with doing basic parsing on it in Python.

comment:12 Changed 6 months ago by mjo

Well, we don't really parse it. It's more like a grep:

sage: from sage.interfaces.singular import get_docstring                        
sage: get_docstring("Preface")

will dutifully pull up the "Preface" node without regard for where in the manual it lives (it's not a function that we wrap and should throw a KeyError instead). The current branch oversimplifies a bit here too, checking only that the parent node is "Functions", but you can pass a series of (sub)nodes to info to ensure that you're navigating to where you think you are.

It looks like we also install info files for maxima and giac. So not only would having "info" installed allow you to run the interactive help in a singular_console(), but it may help somebody read the maxima and giac docs, too.

comment:13 Changed 6 months ago by mkoeppe

I don't really have an objection to adding info as a package. Accessing the interactive help in the standalone giac, maxima, singular shipped with Sage is a nice-to-have feature; but I don't think this is a strong argument to making it a "standard" package.

However, I don't support replacing the in-Python code (no issues have been reported with it -- no matter how primitive the code may be) by calling out to the info program. This adds a complication that I would really hope to avoid.

comment:14 follow-ups: Changed 6 months ago by mjo

Conversely, I'm only willing to tolerate the new dependency if it eliminates these hacks. It may be true that no issues have been reported with the info parser, but how long have we been waiting for someone to write an spkg-configure.m4 for singular? The only reason no one has done it (singular has a pc file, and is a huge, slow-to-build package) is because everyone is afraid to step into a pile of shit when they open singular.py{,x} =)

Replacing the cruft is its own reward if it makes maintaining the singular interface easier. I'm unhappy about the dependency myself, but who does it really hurt? Doing something about SINGULARPATH is necessary before we can use Singular from the system. Using GNU Info to read the info file is an easy and principled way to get us to a point where everyone on Fedora, Debian, Gentoo, Homebrew, Conda, Nix, etc. can just use Singular (and Info) from their package manager. That's a big gain for what constitutes almost the entirety of our users. And who is penalized by having Info be a standard package? Maybe some BSD installs that aren't currently supported platforms? This should be something like bzip2 that no one ever builds.

comment:15 in reply to: ↑ 14 Changed 6 months ago by mkoeppe

Replying to mjo:

who is penalized by having Info be a standard package?

There is no problem with making info a standard package.

But there is a problem in adding an additional non-Python runtime dependency to the Sage library. info cannot be installed by pip.

comment:16 in reply to: ↑ 14 Changed 6 months ago by mkoeppe

Replying to mjo:

how long have we been waiting for someone to write an spkg-configure.m4 for singular?

This task was waiting for the stalled Singular upgrade ticket #25993.

comment:17 Changed 6 months ago by mjo

One final point for GNU Info:

The Singular manual is now 11MB in singular-4.2.1. The existing implementation loads the entire thing (not just the function documentation, as I've shown) into a global variable that lives forever. This permanently wastes roughly the same amount of memory:

sage: from sage.interfaces.singular import generate_docstring_dictionary        
sage: get_memory_usage()                                                        
6467.30859375
sage: generate_docstring_dictionary()                                           
sage: get_memory_usage()                                                        
6479.34765625

And that will get worse as the Singular manual grows.

comment:18 follow-up: Changed 6 months ago by mkoeppe

By the way, this ticket does not solve the original problem at all. If we want to use system Singular on a system that does not have info, then the installed info from the proposed SPKG does not have information regarding the info file location. So you are only shifting the problem of locating the info file from runtime to build time; *and* you are introducing a new non-Python dependency for the sagelib runtime.

comment:19 in reply to: ↑ 18 Changed 6 months ago by mjo

Replying to mkoeppe:

By the way, this ticket does not solve the original problem at all. If we want to use system Singular on a system that does not have info, then the installed info from the proposed SPKG does not have information regarding the info file location. So you are only shifting the problem of locating the info file from runtime to build time; *and* you are introducing a new non-Python dependency for the sagelib runtime.

While it's possible to put the info files anywhere, we can stick a colon on the end of $INFOPATH to have the SPKG try the default list as fallbacks: https://git.savannah.gnu.org/cgit/texinfo.git/tree/info/filesys.h#n70

If we manage to find a system that both doesn't have GNU Info and yet still goes out of its way to install info files in a crazy location... then I guess at that point we could use DEPCHECK to ensure that the Singular SPKG gets built on that system.

comment:20 follow-up: Changed 6 months ago by mkoeppe

Sorry, overall this ticket looks like a solution in search of a problem, and one that will create new problems as a side effect.

The actual problem has a simple solution, comment:9.

comment:21 Changed 6 months ago by mjo

We could always make GNU Info an optional package, and take the same approach to

sage: singular.<function>?

that we take to

sage: singular_console()
> help <function>;

That is, it doesn't work unless you have Info installed. The docstring for the sage object would suggest the optional package if it's not available.

comment:22 in reply to: ↑ 20 Changed 6 months ago by mjo

Replying to mkoeppe:

Sorry, overall this ticket looks like a solution in search of a problem, and one that will create new problems as a side effect.

The actual problem has a simple solution, comment:9.

This is only a solution if you insist on keeping our lame python implementation and all of the problems it comes with. It'll get the job done, but I won't be the one to do it.

comment:23 Changed 6 months ago by git

  • Commit changed from c4f693803fdc7deb904bb85c7352f6327f338a66 to a82847c7617969350dbc409c6821d10abc74817f

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

a82847cTrac #32242: use fallback defaults for $INFOPATH.

comment:24 Changed 6 months ago by mkoeppe

  • Status changed from needs_review to needs_work
  • Work issues set to reduce ticket to adding info as optional package

comment:25 Changed 6 months ago by mjo

  • Dependencies set to #32302
  • Milestone changed from sage-9.4 to sage-9.5

comment:26 Changed 6 months ago by git

  • Commit changed from a82847c7617969350dbc409c6821d10abc74817f to 624a0e8d064ba70d40af701ec65b0cd47d9cbad6

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2acc06eTrac #32302: drop SINGULAR_EXECUTABLE from src/bin/sage-env.
fde32deTrac #32242: new optional package for the standalone GNU Info reader.
7d1e61bTrac #32242: use fallback defaults for $INFOPATH.
56186bfTrac #32242: use GNU Info to obtain Singular docstrings.
624a0e8Trac #32242: add "optional" tags for tests that require GNU Info.

comment:27 Changed 6 months ago by mjo

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues reduce ticket to adding info as optional package deleted

New commits:

2acc06eTrac #32302: drop SINGULAR_EXECUTABLE from src/bin/sage-env.
fde32deTrac #32242: new optional package for the standalone GNU Info reader.
7d1e61bTrac #32242: use fallback defaults for $INFOPATH.
56186bfTrac #32242: use GNU Info to obtain Singular docstrings.
624a0e8Trac #32242: add "optional" tags for tests that require GNU Info.

comment:28 Changed 6 months ago by mjo

It's now completely optional. When "info" is not installed, you'll be prodded to install it:

sage: singular.align?                                                           
Signature:   singular.align(*args, **kwds)
Type:        SingularFunction
String form: align
File:        ~/src/sage.git/local/lib/python3.9/site-packages/sage/interfaces/singular.py
Docstring:  
This function is an automatically generated pexpect wrapper around the
Singular function 'align'.

EXAMPLES:

   sage: groebner = singular.groebner
   sage: P.<x, y> = PolynomialRing(QQ)
   sage: I = P.ideal(x^2-y, y+x)
   sage: groebner(singular(I))
   x+y,
   y^2-y

The relevant Singular documentation will be shown here if you install
GNU Texinfo, the stand-alone GNU Info program, or the SageMath "info"
SPKG.

comment:29 Changed 6 months ago by mkoeppe

I have opened #32323 for the uncontroversial part "Add GNU info as an optional package".

comment:30 Changed 7 weeks ago by mjo

  • Summary changed from Use GNU Info (new package) to parse singular's Info file to Use GNU Info to parse singular's Info file

comment:31 Changed 7 weeks ago by git

  • Commit changed from 624a0e8d064ba70d40af701ec65b0cd47d9cbad6 to 137a9182a02d4aa329a78c2e936b48431c1e9017

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cdfa63cTrac #32242: use GNU Info to obtain Singular docstrings.
137a918Trac #32242: add "optional" tags for tests that require GNU Info.

comment:32 Changed 4 weeks ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6
Note: See TracTickets for help on using tickets.