Mystery of "GAP documented global functions"

[E. Madison Bray]

In the sage.libs.gap package, in the assigned_names.py module, there
is a function called just list_functions() [1] which is documented to
"Return the GAP documented global functions".

This is used in the all_documented_functions.py [2] module to create
Python wrappers for "all documented GAP functions, they can be thought
of as the official API of GAP".

This turns out to create a bit of a problem: The GAP function
IsDocumentedWord, which this code uses to determine if a function is
"documented", works by searching for the function's name in all
documentation known to GAP. By default this is just the standard GAP
reference docs. However, if you have any GAP packages installed, it
will *also* search the docs for those packages.

If you have lots of packages installed that means this is *extremely*
slow, even if we're just searching for terms that we only care about
being in the main GAP docs.

Though perhaps it is a faulty assumption that this is "the official
API of GAP" in the first place. Or at the very least, perhaps we
don't really need to care about whether or not the function is
"documented", and that just returning all global functions is good
enough for a first pass.

But I'm not sure. Volker wrote this code originally so he would know
best. But what do we want to present as the "API" provided by Sage's
GAP interface?






[1] https://gitlab.com/sagemath/sage/blob/master/src/sage/libs/gap/assigned_names.py#L118
[2] https://gitlab.com/sagemath/sage/blob/master/src/sage/libs/gap/all_documented_functions.py

[Volker Braun]

This is basically the tab completion list in the Sage/libgap interface. As you found out, creating it on the fly is way too slow. Hence we cache a reasonable list of names. 

[E. Madison Bray]

There is also an existing hard-coded list of function names that are
actually what's used for dir() and tab-completion, completely separate
from this module. AFAICT this all_documented_functions module isn't
really used anywhere in Sage, and doesn't turn up much in the
documentation. As Alex Konovalov pointed out to me, the "example" in
all_documented_functions of doing `from
sage.libs.gaps.all_documented_functions import *` is probably not a
good idea in most cases anyways. For example:

sage: DihedralGroup(8)
Dihedral group of order 16 as a permutation group
sage: type(_)
<class 'sage.groups.perm_gps.permgroup_named.DihedralGroup_with_category'>
sage: from sage.libs.gap.all_documented_functions import *
sage: DihedralGroup(8)
<pc group of size 8 with 3 generators>
sage: type(_)
<type 'sage.libs.gap.element.GapElement'>

Oops, this clobbers multiple existing built-ins in Sage. Alex also
suggested that using IsDocumentedWord like this was perhaps
ill-considered in the first place.

Given that it isn't actually used anywhere in Sage, and is not
particularly documented, and mostly seems to cause problems, it would
be best just to remove this? Does anyone use it?

> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

[Dima Pasechnik]

On Fri, Dec 14, 2018 at 10:19 AM E. Madison Bray <erik....@gmail.com> wrote:
>
> There is also an existing hard-coded list of function names that are
> actually what's used for dir() and tab-completion, completely separate
> from this module. AFAICT this all_documented_functions module isn't
> really used anywhere in Sage, and doesn't turn up much in the
> documentation. As Alex Konovalov pointed out to me, the "example" in
> all_documented_functions of doing `from
> sage.libs.gaps.all_documented_functions import *` is probably not a
> good idea in most cases anyways. For example:
>
> sage: DihedralGroup(8)
> Dihedral group of order 16 as a permutation group
> sage: type(_)
> <class 'sage.groups.perm_gps.permgroup_named.DihedralGroup_with_category'>
> sage: from sage.libs.gap.all_documented_functions import *
> sage: DihedralGroup(8)
> <pc group of size 8 with 3 generators>
> sage: type(_)
> <type 'sage.libs.gap.element.GapElement'>
>
> Oops, this clobbers multiple existing built-ins in Sage. Alex also
> suggested that using IsDocumentedWord like this was perhaps
> ill-considered in the first place.
>
> Given that it isn't actually used anywhere in Sage, and is not
> particularly documented, and mostly seems to cause problems, it would
> be best just to remove this? Does anyone use it?

it's used for tab completion and as well for directly calling libgap functions
(otherwise you need to call libgap's function_factory on them):

sage: libgap.SymmetricGroup(3).Order() # SymmetricGroup is in the list
6
sage: libgap.ImfMatrixGroup(12,3) # ImfMatrixGroup is not there
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-8-86e9cd3d6fbf> in <module>()
----> 1 libgap.ImfMatrixGroup(Integer(12),Integer(3))

/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/misc/lazy_import.pyx
in sage.misc.lazy_import.LazyImport.__getattr__
(build/cythonized/sage/misc/lazy_import.c:3536)()
320 True
321 """
--> 322 return getattr(self.get_object(), attr)
323
324 # We need to wrap all the slot methods, as they are not forwarded

/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/libs/gap/libgap.pyx
in sage.libs.gap.libgap.Gap.__getattr__
(build/cythonized/sage/libs/gap/libgap.c:6229)()
668 g = make_any_gap_element(self, gap_eval(name))
669 else:
--> 670 raise AttributeError(f'No such attribute: {name}.')
671
672 self.__dict__[name] = g

AttributeError: No such attribute: ImfMatrixGroup.

sage: libgap.function_factory('ImfMatrixGroup')(12,3) # but this works
ImfMatrixGroup(12,3)

----
Best,
Dima

[E. Madison Bray]

It isn't though. You and Volker both seem to be confusing two
different things, unless I'm missing something. Grep the sources
yourself: The module sage.libs.gap.all_documented_functions is not
referenced anywhere outside of itself:

$ grep -R all_documented_functions src/sage
src/sage/libs/gap/all_documented_functions.py: sage: from
sage.libs.gap.all_documented_functions import *

What *is* used to provide directory/tab-completion is a different
module called sage.libs.gap.gap_functions, which just contains a
hard-coded list of strings naming various GAP functions. In fact I
recently updated [1] this list because it contained some globals which
are not functions, as well as a few that no longer exist:
https://git.sagemath.org/sage.git/commit/?id=60d7eb69ffa7d402495717148879906639324244

Actually, it would be good at least if the list in this module were
tested to ensure it remains valid. When I updated it I did something
like

for name in common_gap_functions:
try:
func = libgap.eval(name)
except Exception as exc:
print("{} possibly missing? Error: {}".format(name, exc)
continue

if not IsFunction(func):
print("{}: not a function".format(func))

[Dima Pasechnik]

In all_documented_functions.py you see

for _f in _FUNCTIONS:
globals()[_f] = libgap.function_factory(_f)

and this is exactly the "function_factorisation" of everything in
sage.libs.gap.gap_functions

So simply removing all_documented_functions.py will break a hell of a
lot of code.

(although there is of course nothing against merging sage.libs.gap.gap_functions
and sage.libs.gap.all_documented_functions - unless I miss something)

[Simon King]

Hi Eric,

On 2018-12-14, E. Madison Bray <erik....@gmail.com> wrote:
> sage: from sage.libs.gap.all_documented_functions import *
> sage: DihedralGroup(8)
><pc group of size 8 with 3 generators>
> sage: type(_)
><type 'sage.libs.gap.element.GapElement'>
>
> Oops, this clobbers multiple existing built-ins in Sage.

What's the problem? If you import something, the stuff you import will
of course override existing stuff.

However, if I understand correctly, that's not your main point.

Cheers,
Simon

[E. Madison Bray]

... yes, but, module-level code still does not get executed unless the
module is actually imported at some point, which it isn't. I thought
the grep output I showed demonstrated this but go ahead and try it
yourself. Remove both sage.libs.gap.assigned_names and
sage.libs.gap.all_documented_functions. These modules are not used at
all by anything.

> and this is exactly the "function_factorisation" of everything in
> sage.libs.gap.gap_functions
>
> So simply removing all_documented_functions.py will break a hell of a
> lot of code.
>
> (although there is of course nothing against merging sage.libs.gap.gap_functions
> and sage.libs.gap.all_documented_functions - unless I miss something)

I think you're definitely missing something.

[E. Madison Bray]

On Fri, Dec 14, 2018 at 12:40 PM Simon King <simon...@uni-jena.de> wrote:
>
> Hi Eric,
>
> On 2018-12-14, E. Madison Bray <erik....@gmail.com> wrote:
> > sage: from sage.libs.gap.all_documented_functions import *
> > sage: DihedralGroup(8)
> ><pc group of size 8 with 3 generators>
> > sage: type(_)
> ><type 'sage.libs.gap.element.GapElement'>
> >
> > Oops, this clobbers multiple existing built-ins in Sage.
>
> What's the problem? If you import something, the stuff you import will
> of course override existing stuff.

Yeah, and that's obviously bad advice to be giving users. "Here:
Override Sage's a builtins with a bunch of arbitrary built-ins from
GAP; enjoy playing wheel of fortune with what still works!"

> However, if I understand correctly, that's not your main point.

No...

[Dima Pasechnik]

Indeed, my bad, sorry. I was confusing these with libgap-prefixed things.
I'm OK with removing sage.libs.gap.all_documented_functions

It's still a mystery to me how some GAP functions are
"function_factorised" though.

[E. Madison Bray]

I'm not exactly sure what you mean by "function factorised" but regardless I don't think it's a deep mystery. Just look at Gap.__getattr__

[Dima Pasechnik]

On Fri, Dec 14, 2018 at 12:32 PM E. Madison Bray <erik....@gmail.com> wrote:
>
> I'm not exactly sure what you mean by "function factorised" but regardless I don't think it's a deep mystery. Just look at Gap.__getattr__

I see. By the way, could you explain the need for

Length = libgap.function_factory('Length')
FlagsType = libgap.function_factory('FlagsType')
TypeObj = libgap.function_factory('TypeObj')
IS_SUBSET_FLAGS = libgap.function_factory('IS_SUBSET_FLAGS')
GET_OPER_FLAGS = libgap.function_factory('GET_OPER_FLAGS')
OPERATIONS = libgap.get_global('OPERATIONS')
NameFunction = libgap.function_factory('NameFunction')

in src/sage/libs/gap/operations.py ? With Gap.__getattr__ they seem to
be not needed.

This is what confused me into thinking that the only way to get a GAP
function into
libgap is to explicitly wrap it in function_factory().

One has similar explicit calls to function_factory() in
src/sage/libs/gap/assigned_names.py
(the latter can go completely, right?)

Dima

[Dima Pasechnik]

On Fri, Dec 14, 2018 at 12:53 PM Dima Pasechnik <dim...@gmail.com> wrote:
>
> On Fri, Dec 14, 2018 at 12:32 PM E. Madison Bray <erik....@gmail.com> wrote:
> >
> > I'm not exactly sure what you mean by "function factorised" but regardless I don't think it's a deep mystery. Just look at Gap.__getattr__
>
> I see. By the way, could you explain the need for
>
> Length = libgap.function_factory('Length')
> FlagsType = libgap.function_factory('FlagsType')
> TypeObj = libgap.function_factory('TypeObj')
> IS_SUBSET_FLAGS = libgap.function_factory('IS_SUBSET_FLAGS')
> GET_OPER_FLAGS = libgap.function_factory('GET_OPER_FLAGS')
> OPERATIONS = libgap.get_global('OPERATIONS')
> NameFunction = libgap.function_factory('NameFunction')

I mean, one could instead just do

Length = libgap.Length
etc.

[Volker Braun]

Afair the all_documented_functions is documenting the starting point of our list of hardcoded tab-completions. At one point I went through it by hand and tweaked it, though. The module cannot be default imported for the reasons you mention. But I think its useful to have the code around since it is far from obvious what GAP symbols are useful entry points that should be accessible from Sage. GAP only has a single flat namespace and its littered with private stuff...

[E. Madison Bray]

On Fri, Dec 14, 2018 at 2:33 PM Volker Braun <vbrau...@gmail.com> wrote:
>
> Afair the all_documented_functions is documenting the starting point of our list of hardcoded tab-completions. At one point I went through it by hand and tweaked it, though. The module cannot be default imported for the reasons you mention. But I think its useful to have the code around since it is far from obvious what GAP symbols are useful entry points that should be accessible from Sage. GAP only has a single flat namespace and its littered with private stuff...

It's a fair point that that code *might* be useful for occasionally
hard-coding a list of useful GAP symbols. But in that case it would
be code that is not run by importing a module, but rather a function
that is used to generate a list of functions, sort of like (I think)
we do for PARI (but we would just use it at runtime).

However, even if used like that, the present code that lists
"documented functions" returns some 10000 functions, a great deal more
than the ~1400 we have hard-coded in sage.libs.gap.gap_functions.

It should also be used in such a way that it can be kept up-to-date
regularly. As I mentioned elsewhere in this thread, I found that the
existing list was quite outdated and contained some entries that
weren't even functions (such that trying to libgap.__getattr__ them
raised an exception).

So unless someone has a concrete plan for dynamically generating a
"standard" GAP API, the current code still isn't worth keeping around,
at least in its current state.

[Dima Pasechnik]

On Mon, Dec 17, 2018 at 4:07 PM E. Madison Bray <erik....@gmail.com> wrote:
>
> On Fri, Dec 14, 2018 at 2:33 PM Volker Braun <vbrau...@gmail.com> wrote:
> >
> > Afair the all_documented_functions is documenting the starting point of our list of hardcoded tab-completions. At one point I went through it by hand and tweaked it, though. The module cannot be default imported for the reasons you mention. But I think its useful to have the code around since it is far from obvious what GAP symbols are useful entry points that should be accessible from Sage. GAP only has a single flat namespace and its littered with private stuff...
>
> It's a fair point that that code *might* be useful for occasionally
> hard-coding a list of useful GAP symbols. But in that case it would
> be code that is not run by importing a module, but rather a function
> that is used to generate a list of functions, sort of like (I think)
> we do for PARI (but we would just use it at runtime).
>
> However, even if used like that, the present code that lists
> "documented functions" returns some 10000 functions, a great deal more
> than the ~1400 we have hard-coded in sage.libs.gap.gap_functions.
>
> It should also be used in such a way that it can be kept up-to-date
> regularly. As I mentioned elsewhere in this thread, I found that the
> existing list was quite outdated and contained some entries that
> weren't even functions (such that trying to libgap.__getattr__ them
> raised an exception).
>
> So unless someone has a concrete plan for dynamically generating a
> "standard" GAP API, the current code still isn't worth keeping around,
> at least in its current state.

I suppose we at least should rename these files, prefixing them with
`_`, to make
clear it's not something to be used without much thought.

[E. Madison Bray]

On Mon, Dec 17, 2018 at 5:16 PM Dima Pasechnik <dim...@gmail.com> wrote:
>
> On Mon, Dec 17, 2018 at 4:07 PM E. Madison Bray <erik....@gmail.com> wrote:
> >
> > On Fri, Dec 14, 2018 at 2:33 PM Volker Braun <vbrau...@gmail.com> wrote:
> > >
> > > Afair the all_documented_functions is documenting the starting point of our list of hardcoded tab-completions. At one point I went through it by hand and tweaked it, though. The module cannot be default imported for the reasons you mention. But I think its useful to have the code around since it is far from obvious what GAP symbols are useful entry points that should be accessible from Sage. GAP only has a single flat namespace and its littered with private stuff...
> >
> > It's a fair point that that code *might* be useful for occasionally
> > hard-coding a list of useful GAP symbols. But in that case it would
> > be code that is not run by importing a module, but rather a function
> > that is used to generate a list of functions, sort of like (I think)
> > we do for PARI (but we would just use it at runtime).
> >
> > However, even if used like that, the present code that lists
> > "documented functions" returns some 10000 functions, a great deal more
> > than the ~1400 we have hard-coded in sage.libs.gap.gap_functions.
> >
> > It should also be used in such a way that it can be kept up-to-date
> > regularly. As I mentioned elsewhere in this thread, I found that the
> > existing list was quite outdated and contained some entries that
> > weren't even functions (such that trying to libgap.__getattr__ them
> > raised an exception).
> >
> > So unless someone has a concrete plan for dynamically generating a
> > "standard" GAP API, the current code still isn't worth keeping around,
> > at least in its current state.
>
> I suppose we at least should rename these files, prefixing them with
> `_`, to make
> clear it's not something to be used without much thought.

I think instead I will mark them deprecated, since it's not something
that is currently useful even given some thought. I'll work more
later with the GAP developers to see what they think about providing a
default list of "common" functions and how would be the best way to go
about that. Alex seemed skeptical about just using IsDocumentedWord
as-is.

I might just borrow the relevant code from the GAP Jupyter kernel for
tab-completion...