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

GapObj vs. julia_to_gap #1025

Closed
ThomasBreuer opened this issue Aug 14, 2024 · 1 comment
Closed

GapObj vs. julia_to_gap #1025

ThomasBreuer opened this issue Aug 14, 2024 · 1 comment
Labels
topic: conversion Issue related to conversion

Comments

@ThomasBreuer
Copy link
Member

Originally we installed methods for GapObj in situations where we wanted to convert the given Julia object to GAP.
Meanwhile we use GapObj in Oscar also for fetching the "underlying GAP object" stored in an Oscar object.

(On the one hand, this is better than accessing some field (with non-uniform name) in the object.
On the other hand, one cannot decide from the code whether a call of GapObj will just fetch a known value, or will compute the value in the first call (and then store it), or will create a new object on the GAP side in each call.)

For the "fetching" aspect of GapObj, sometimes we install in fact a GAP.julia_to_gap(x) method instead of a GapObj(x) method, in order to cover automatically also the situation GapObj([x, y, z]; recursive = true).
The delegations between the methods in GAP.jl are not really nice:
Typically, GapObj(x, recursive = false) calls julia_to_gap(x, IdDict(); recursive = false), and if there is no special method for these arguments then this delegates to the generic julia_to_gap(x).

Here is a situation where this setup causes a problem:

using Oscar
g = symmetric_group(2);
T = right_transversal(g, g);
GapObj(T)

If a version of GAP.jl is used that includes #989 then we get a stack overflow error.
(Currently Oscar.jl wants an older version of GAP.jl, thus one cannot run into this problem without modifying Oscar.jl's Project.toml.)
The point is that the object T above is an AbstractVector, and the new GAP.jl provides a special julia_to_gap method for such objects.
An obvious solution is to provide also a GapObj(T) method in Oscar, but it would be better to provide better support in GAP.jl.

(The CI tests do not find the above problem. There is a test of GAP.jl together with the current Oscar.jl, but since Oscar.jl does not admit the dev version of GAP.jl, this test is skipped, and reports success.)

@fingolfin
Copy link
Member

@ThomasBreuer I believe this has been resolved by your PR #1029 ? If you agree, please close; otherwise: what exactly is missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: conversion Issue related to conversion
Projects
None yet
Development

No branches or pull requests

2 participants