Opened 3 years ago

Closed 8 months ago

#26959 closed enhancement (worksforme)

libgap attribute access

Reported by: SimonKing Owned by: embray
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: interfaces Keywords: libgap attribute access
Cc: dimpase Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

If foo is defined in libgap, then libgap.foo should access it. Currently it doesn't:

sage: libgap.fail
Traceback (most recent call last):
...
AttributeError: No such attribute: fail.
sage: libgap.eval('G := DihedralGroup(8)')
<pc group of size 8 with 3 generators>
sage: libgap.G
Traceback (most recent call last):
...
AttributeError: No such attribute: G.
sage: libgap.eval('G')
<pc group of size 8 with 3 generators>
sage: libgap.eval('fail')
fail

I guess libgap.fail would be more pythonic than libgap.eval('fail').

Note that only part of the above works in the pexpect interface:

sage: gap.eval('G := DihedralGroup(8);')
'<pc group of size 8 with 3 generators>'
sage: gap.G   # is some gap function, not what was defined above
G
sage: gap.fail # works as I would expect
fail

Change History (10)

comment:1 Changed 3 years ago by SimonKing

  • Keywords libgap attribute access added

comment:2 Changed 3 years ago by embray

  • Owner changed from (none) to embray

I agree. We had a long e-mail thread sort of discussing this, and it led to frustratingly few useful conclusions.

The problem is that right now there is a hard-coded list of what global variables in GAP can be accessed via attribute access on the libgap instance. I think this is partly intentional: The idea is that GAP has a ton of global variables (there's really just one big global namespace in GAP), so if you allow attribute access via libgap for any GAP global, it means libgap does not necessarily have a predictable API because it can depend on the version of GAP, what packages are loaded, etc.

I'm not totally sure this matters though. It should still be understood that the libgap object is a relatively low-level interface to GAP, and it does not necessarily have a "standard API" w.r.t. what GAP variables it provides access to. If nothing else, code using libgap that depends on functions (e.g. from GAP packages) that might not be defined should do a proper hasattr(libgap, "<funcname>") before attempting to use them, or at least be able to handle the resulting AttributeError if it's missing.

comment:3 Changed 3 years ago by dimpase

A further complication is that what's working in GAP depends upon the set of GAP packages installed and/or loaded.

comment:4 follow-ups: Changed 3 years ago by nbruin

In the ECL wrapper I decided against this. If libgap.foo is just a shorthand for libgap.eval("foo"), then using libgap.foo is not going to be very efficient, because every time it will need to set up a wrapper object, which needs to hook into gap's garbage collector somewhere to ensure that the object doesn't get deleted before the wrapper disappears. Much more efficient:

FOO=lib.eval("foo")
<do all kinds of things with FOO>

If you view these instructions like the "import" statements required in python, it won't seem so onerous.

It would be possible to create a namespace object that does have the above semantics, but caches the wrapper (it would basically be a defaultdict). This could then be used for permanent objects, such as functions (perhaps have some technology that also tries function_factory when appropriate?). I would say that the namespace of the libgap interface itself is not the best spot for this, though.

comment:5 in reply to: ↑ 4 Changed 3 years ago by SimonKing

Replying to nbruin:

In the ECL wrapper I decided against this. If libgap.foo is just a shorthand for libgap.eval("foo"), then using libgap.foo is not going to be very efficient,

Yes, that is what I was afraid of. And actually in the p_group_cohomology version that I am testing right now, I do what you suggest: I define Failure = libgap.eval('fail') in one module and import Failure from there.

So, I wouldn't mind to close this ticket as "won't fix".

comment:6 Changed 3 years ago by embray

There's still something to be done here. See also the thread from https://groups.google.com/d/msg/sage-devel/iPTfFXUk8XU/UX3qr42xAQAJ

It's just not 100% clear to me, as a non-GAP user, what the best way forward is. I asked Alex Konovalov (specifically w.r.t. how to determine what functions in GAP are "standard") and he wasn't sure either, or didn't understand the question.

comment:7 in reply to: ↑ 4 Changed 3 years ago by embray

Replying to nbruin:

In the ECL wrapper I decided against this. If libgap.foo is just a shorthand for libgap.eval("foo"), then using libgap.foo is not going to be very efficient, because every time it will need to set up a wrapper object, which needs to hook into gap's garbage collector somewhere to ensure that the object doesn't get deleted before the wrapper disappears.

I don't follow you here. In the GAP interface there is no substantive difference between these cases: It still has to create a GapElement object to hold the returned object. You can see this yourself by looking at the sources in sage.libs.gap.libgap. There isn't really any substantive difference between libgap.eval("foo") and libgap.foo.

The only thing about the attribute getter is that it has some code for slightly fast-tracking functions, but even it is unnecessary (it effectively is just skipping checking the object's TNUM) and brittle (it assumes that everything in our hard-coded list of "common GAP functions" is still actually a function, when really GAP allows names to be redefined quite easily a la Python).

For functions that are used frequently there is also GAP.function_factory which is just the same as Gap.__getattr__ but it really really assumes that the name is bound to a function (when it might not be), and then it just caches the wrapper element. As you wrote, it might not be a bad idea if that caching happened automatically, especially for common functions that are not likely to be rebound. But it's not clear how to programatically determine what functions are "common functions that are not likely to be rebound".

comment:8 Changed 3 years ago by embray

  • Milestone changed from sage-8.6 to sage-8.7

Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.

comment:9 Changed 3 years ago by embray

  • Milestone changed from sage-8.7 to sage-pending

Removing most of the rest of my open tickets out of the 8.7 milestone, which should be closed.

comment:10 Changed 8 months ago by embray

  • Milestone changed from sage-pending to sage-duplicate/invalid/wontfix
  • Resolution set to worksforme
  • Status changed from new to closed

This is fixed since a while ago by #27911. It is further improved on in #31404, since gappy does not use self.eval(attr) to do this, but simply uses the libgap API to check if GAP has a global named attr and raises an AttributeError if not, so this should alleviate nbruin's concerns mentioned here.

Note: See TracTickets for help on using tickets.