Limit the number of global objects

[fingolfin]

After discussion with @fieker, we agreed that Oscar should try to minimize the number of global (non-function) objects it defines.

We should write up some rules for that and document this in the style guide. Two possible rules of thumb:

1. global objects should be really, really important, and very frequently used in multiple applications
2. global objects should be "as unique as possible"

Of course these are (necessarily) soft rules, but they give a rough idea, and if in doubt, we'll have to discuss it -- but if those are in the style guides, then at least people have a chance to act about them.

I'll try to illustrate further with concrete (counter) examples:

The current globals QQ and ZZ satisfy both of these: they are used virtually everywhere; and they are singletons, so it doesn't get more unique than that.

In contrast, we think that e.g. QQi, ZZi, QQBar should not be there (at least should not be exported to the user name space by default:

• they are not used that much
• they are not very "unique" (e.g. QQi is not even a NumberField, and differs completely from quadratic_field(-1)[1]; and QQBar is just "a" algebraic closer of the rationals, for very specific applications)

Thoughts, comments, ideas?

[thofma]

I don't see the harm in QQBar and I can't think of other examples than ZZ and QQ fitting these criteria. So should the limit just be 2? Or we remove ZZ and QQ altogether to have a consistent system.

[tthsqe12]

Um, the list of global objects is quite a bit longer than this...
Just look at everything that has "ID" in its name in AA/src/generic/GenericTypes.jl

[fieker]

I'd limit it to at most QQ and ZZ. I don't see QQBar universally used, so I'd rather not have QQBar globally predefined. - neither QQi, ZZi, Qab, .... I think they are all niche types - most users will never use them.
The PolyID stuff are, as far as I can see the Dicts for cache=true - they are not exported (or should not be).
Maybe, to help move forward we need to have an .Oscar_init file where one can predifine more global objects?

[fingolfin]

Regarding caches and FooBarID: as long as those are not exported, I don't have a problem with them (though perhaps we should use a convention like having the names of such "internal" variables be prefixed with an underscore).

I don't think and "oscar init file" is a workable solution, or at least it's not so easy to see how this should be done: for one thing, we'd only want this to be in effect when using Oscar is done from Main (so that package which use Oscar are not affected by this). And I am concerned how this would interact with precompilation?

[thofma]

I don't understand the Oscar init file. If it is per user, it should go into $JULIAHOME/startup.jl. Otherwise it is just somewhere in Oscar.

[fieker]

On Thu, Jun 09, 2022 at 06:19:26AM -0700, Tommy Hofmann wrote: I don't understand the Oscar init file. If it is per user, it should go into `$JULIAHOME/startup.jl`. Otherwise it is just somewhere in Oscar.

It is hard to add QQ = rationals(); in startup.jl as oscar is not loaded at this point. However, in Oscar we could, at the end of loading everything also evaluate a user file - if present.

…

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

[thofma]

I see. But I have some reservations about Oscar loading random files. I am also not sure what loading means. Load into which namespace?

To move forward we can

1. just remove QQi, ZZi QQBar,
2. write down in the developer documentation that we these are the only two exceptions.

[ulthiel]

As someone who may have initiated the discussion: I'm happy with ZZ and QQ only (but these should exist for convenience). I just thought that when it was decided there's QQBar, there could be QQAb as well. I was surprised by QQi and feared QQsqrt2 may come next.

[fieker]

On Thu, Jun 09, 2022 at 06:47:28AM -0700, Tommy Hofmann wrote: I see. But I have some reservations about Oscar loading random files. I am also not sure what loading means. Load into which namespace?

A separate diskussion for when we are bored with life

To move forward we can 1. just remove `QQi`, `ZZi` `QQBar`, 2. write down in the developer documentation that we these are the only two exceptions.

Agreed

…

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

[lgoettgens]

To move forward we can

1. just remove QQi, ZZi QQBar,
2. write down in the developer documentation that we these are the only two exceptions.

QQBar will be no longer exported, once #2445 is merged. QQi and ZZi seem to no longer exist.

[lgoettgens]

Um, the list of global objects is quite a bit longer than this...

Yes, indeed. As of the current master (eb8ea10), the list is as follows:

julia> filter(name -> !(typeof(getfield(Oscar, name)) <: Union{Function, DataType, UnionAll}), names(Oscar)) |> (x -> show(IOContext(stdout, :limit => false), "text/plain", x))
29-element Vector{Symbol}:
 :ANTIC
 :AbstractAlgebra
 :CalciumQQBar
 :FieldElement
 :FlintQQ
 :FlintQQi
 :FlintZZ
 :FlintZZi
 :GAP
 :Generic
 :Graphs
 :Hecke
 :IntExt
 :Native
 :Nemo
 :OSCAR
 :Oscar
 :Polymake
 :QQ
 :QQBar
 :RDF
 :RationalUnion
 :RingElement
 :Singular
 :VarName
 :ZZ
 :error_dim_negative
 :inf
 :oscar

I would group them as follows:

(Sub)-modules of Oscar packages:

ANTIC
AbstractAlgebra
GAP
Generic
Graphs
Hecke
Native
Nemo
OSCAR
Oscar
oscar
Polymake
Singular

I think the cornerstones should be exported, but what about stuff like Generic and Native?

Often used rings/fields

CalciumQQBar
FlintQQ
FlintQQi
FlintZZ
FlintZZi
QQ
QQBar
ZZ

Summarizing the previous discussion, only QQ and ZZ should be exported.

Type unions

FieldElement
IntExt
RationalUnion
RingElement
VarName

In my opinion either export all or none (and then the same for the ones missing here like IntegerUnion).

Misc

error_dim_negative
inf
RDF

I have no idea what they are used for, so no idea.

[fingolfin]

I am taking care of oscar and OSCAR in PR #2726, and of error_dim_negative in Nemocas/AbstractAlgebra.jl#1410 and TODO.

IMHO we also should not (re-)export at least these:

• RDF
• IntExt
• VarName
• Native

[lgoettgens]

Updated list as of 91d2938

julia> filter(name -> !(typeof(getfield(Oscar, name)) <: Union{Function, DataType, UnionAll}), names(Oscar)) |> (x -> show(IOContext(stdout, :limit => false), "text/plain", x))
27-element Vector{Symbol}:
 :ANTIC
 :AbstractAlgebra
 :BasisLieHighestWeight
 :FieldElement
 :FlintQQ
 :FlintQQi
 :FlintZZ
 :FlintZZi
 :GAP
 :Generic
 :Hecke
 :IntExt
 :MPolyDecRingOrQuo
 :NCRingElement
 :Native
 :Nemo
 :Oscar
 :Polymake
 :QQ
 :RDF
 :RationalUnion
 :RingElement
 :Singular
 :Strassen
 :VarName
 :ZZ
 :inf

[fingolfin]

removing also all unions and modules, we get this:

julia> filter(name -> !(typeof(getfield(Oscar, name)) <: Union{Function, DataType, UnionAll, Union, Module}), names(Oscar)) |> (x -> show(IOContext(stdout, :limit => false), "text/plain", x))
8-element Vector{Symbol}:
 :FlintQQ
 :FlintQQi
 :FlintZZ
 :FlintZZi
 :QQ
 :RDF
 :ZZ
 :inf

[fieker]

I'd also remove Flint*

[fieker]

Lars @lgoettgens is looking at removing the stuff

[lgoettgens]

Triage decided to remove all uses of the four Flint* globals in Hecke (thofma/Hecke.jl#1674) and Oscar (#4261), then wait for some time, and then remove them (Nemocas/Nemo.jl#1935).