Assign each GAP object a *unique* isomorphism from/to OSCAR

[lgoettgens]

This has been brought up by @fingolfin in #2207 (comment).

Currently, there are cases, where computing an isomorphism from an OSCAR object to GAP and then one from its codomain (a GAP object) to OSCAR gives a different object than we started from. For example:

julia> FO = GF(2,2)
Finite field of degree 2 over F_2

julia> FG = codomain(Oscar.iso_oscar_gap(FO))
GAP: GF(2^2)

julia> FO2 = codomain(Oscar.iso_gap_oscar(FG))
Finite field of degree 2 over F_2

julia> FO2 == FO
false

Whenever a Oscar.iso_gap_oscar is created, it is cached as an attribute of the domain (a GAP object).
This PR would add the additional behaviour, that when a GAP object is created as the codomain of Oscar.iso_oscar_gap, this isomorphism (to be correct its inverse) is put into the cache of that GAP object. For the above example, this will lead to true in the last line.

Please see this PR as an opportunity to write your opinion and discuss. If you are in favor of this change, I will add some corresponding tests as well.

Pinging @fingolfin and @ThomasBreuer for their opinion.

[fieker]

how and when are objects deleted?

[lgoettgens]

how and when are objects deleted?

Good question. To be honest, I have no idea. But I guess, the same problem already exists for Oscar.iso_gap_oscar, and thus hopefully has a solution?

[lgoettgens]

Please note, that for some other types of inputs, the wished for behaviour already exists. For example:

julia> FO = cyclotomic_field(5)[1]
Cyclotomic field of order 5

julia> FG = codomain(Oscar.iso_oscar_gap(FO))
GAP: CF(5)

julia> FO2 = codomain(Oscar.iso_gap_oscar(FG))
Cyclotomic field of order 5

julia> FO2 == FO
true

This should create exactly the same (maybe problematic, I don't know) references as Oscar.iso_oscar_gap(oscar_obj) after the proposed change. Or is there something I am missing?

[fieker]

On Fri, Apr 28, 2023 at 08:12:25AM -0700, Lars Göttgens wrote: Please note, that for some other types of inputs, the wished for behaviour already exists. For example: ```julia julia> FO = cyclotomic_field(5)[1] Cyclotomic field of order 5 julia> FG = codomain(Oscar.iso_oscar_gap(FO)) GAP: CF(5) julia> FO2 = codomain(Oscar.iso_gap_oscar(FG)) Cyclotomic field of order 5 julia> FO2 == FO true ``` This should create exactly the same (maybe problematic, I don't know) references as `Oscar.iso_oscar_gap(oscar_obj)` after the proposed change. Or is there something I am missing?

I don't know - and please, accept this as well as a question for @ThomasBreuer and @fingolfin Maybe it's done using WeakDicts - or maybe not...

…

-- Reply to this email directly or view it on GitHub: #2330 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[ThomasBreuer]

The following happens with the current Oscar/GAP.

julia> iso1 = Oscar.iso_oscar_gap(GF(3));

julia> iso2 = Oscar.iso_oscar_gap(GF(3, 1));

julia> domain(iso1) === domain(iso2)
false

julia> codomain(iso1) === codomain(iso2)
true

The function iso_oscar_gap is not injective, w.r.t. the identity of objects in GAP.
In this example, the point is that iso_oscar_gap calls GAP.Globals.GF(3), and that the result is cached on the GAP side.
(In fact, the situation is more complicated, the field is not always cached. Since this is not documented, we cannot rely on the behaviour w.r.t. caching.)

Thus we do not have a unique Oscar object that belongs to a given GAP object.

[ThomasBreuer]

Two more remarks:

• Since the question is about caching objects in other objects, eqiality w.r.t. == (as in one of the examples above) is not enough, we really have to guarantee equality w.r.t. === if we want to promise a particular behaviour.
• Concerning the deletion of objects:
  f = iso_gap_oscar(D) stores f in the GAP object D via a GAP attribute, and D is stored as domain(f) in (the header of) f; as soon as no other "living" objects point to D or f, then the two objects can be garbage collected.
  Similarly, f = iso_oscar_gap(D) stores f in D via the @attr mechanism, and D is stored as domain(f).

[fingolfin]

As Thomas already pointed out, there is no problem with object retention here, as we are not using a global cache: just a pointer in object A to object B and vice versa. And since both objects are managed by the Julia GC, it will detect if no other objects references them, and "delete" them (i.e. the circular reference is fine).

Thomas also points out that for some objects, due to the way things are on the GAP side, we still won't be quite in the situation that everything is unique, as e.g. GAP code is often careless when it comes to finite fields, and instead of passing around a finite field, it just passes around the size of that field, and assumes that describes the field uniquely (spoiler: it doesn't in general, and that has lead to us uncovering and fixing a bunch of bugs in GAP). Anyway, that means GAP code will call GF(3) a lot, and get equal but non-identical objects this way... and of course only one of them will have a pointer to the "corresponding Oscar object". But their elements will all be "equal" and freely interchangeable, so they should also be "mappable" via our isomorphisms...

Anyway, I don't think that's an actual problem or concern for this PR. First off, it is meant for "heavier" objects such as Lie algebras where we generally don't have this issue at all. Secondly, even in cases where this issue exists, well, we are not worse off with the change than we are right now...

[ThomasBreuer]

Following the remark by @fingolfin:
After changing the proposed code to

 @attr Map function iso_oscar_gap(R)
  iso = _iso_oscar_gap(R)
  S = codomain(iso)
  GAP.Globals.SetIsoGapOscar(S, inv(iso))
  @assert codomain(GAP.Globals.IsoGapOscar(S)) === R  # could differ if IsoGapOscar was set
  return iso
end

I get this:

julia> F1 = GF(3)
Galois field with characteristic 3

julia> Oscar.iso_oscar_gap(F1);

julia> F2 = GF(3, 1)
Finite field of degree 1 over F_3

julia> Oscar.iso_oscar_gap(F2);
ERROR: AssertionError: codomain(GAP.Globals.IsoGapOscar(S)) === R
[...]

The iso_oscar_gap code calls GF(3) on the GAP side, and the result gets cached in GAP, thus the second iso_oscar_gap call just fetches it from the cache, and now the GAP object stores already the Oscar object F1 as its IsoGapOscar value.

Thus we can set the inverse of the newly constructed map in the codomain
but we cannot guarantee that the two stored maps are inverses of each other in the sense that their domains/codomains fit.
(I will extend the documentation of iso_gap_oscar and iso_oscar_gap accordingly.)

[lgoettgens]

Let's see if this breaks anything more than before.

[lgoettgens]

The assertion fails at multiple points in the test-suite.

[ThomasBreuer]

In addition to my previous comment:
Once Oscar.iso_oscar_gap(R) has been computed, we can check whether its codomain stores already a Oscar.iso_gap_oscar map, and if not then we can construct and store such a map. Since this is cheap and has no bad side-effects (in the sense that Oscar.iso_oscar_gap and Oscar.iso_gap_oscar anyhow to not promise specific objects as the codomains of their return values), I think it is a good idea to do this. However, this does in general not have the intended effect of this pull request.

[lgoettgens]

I think the problem this PR tries to address is unsolvable as there is no 1-to-1 correspondence of OSCAR types and GAP types. If there is no objection from @ThomasBreuer or @fingolfin, I would close this in the near future as a failed attempt.

[fingolfin]

Fine by me