# Type and function renaming in Oscar II

Continuing the work started in [the old "type renaming" HackMD](https://hackmd.io/0hrBHIUrRZG23xiZZ0NCEA).

Goal: Deal with renaming questions in <https://github.com/oscar-system/Oscar.jl/issues/3054> and elsewhere.

## General rules

ring and ring elem type names are aligned: - `FooRing` and `FooRingElem`
- `FooField` and `FooFieldElem`
- ...

### Rules for "abstract" versus "absolute" versus "Abs"

- `Abs..` in type names can indicate "abstract" or "absolute". Perhaps it should be only one?
- there are many abstract types without the Abs, what is the plan?
- abstract type `ProjSpc`, but abstract type `AbsAffineAlgebraicSet`
- abstract type `ModuleFP`, abstract type `AbstractFreeModElem`
- abstract type `OscarMap`
- `AbstractLat`
- *concrete* types `AbstractVariety` etc.

- in some cases there also aliases, for backwards compatibility, let's get rid of them for 1.0, maybe? e.g. these

@alias AbsLat AbstractLat
@alias AbsSpace AbstractSpace
@alias AbsSpaceMor AbstractSpaceMor

> [time=Wed, Jan 10, 2024 12:09 PM] After some discussion we feel the double meaning of `Abs` is not that bad, nor the user of `Abstract` for concrete types. But what really would be helpful is if more (ideally all) types had a little docstring giving a hint as to what it is about

Lars suggestion:
- only use `Abstract` in a mathematical sense, never in a technical sense to indicate abstract types (unfortunately it contradicts Julia naming, but we already do this elsewhere, e.g. `is_finite`)

Claus points out that we in general don't need to indicate that types are abstract because it's trivial to find out as a programmer whether a type is concrete or not

results of discussion:
1. we (?) can live with the current state of Abs vs Abstract (vs Absolute) ... at least nobody dared to say otherwise (Claus and Max are scary)
- if this is agreed outcome, let's edit dev docs (which currently say `Abs` always means `Absolute` which is patently wrong right now)
2. we want docstrings on types
- someone should work on this (but this is post 1.0)
- Julia nightly has a function `Docs.undocumented_names` which can be used to find types w/o docs
3. there is no rule that forces one to add Abs or Abstract as prefix for abstract types (use it if sensible, don't use it if it makes little sense (e.g. `Group` or `Ring` are abstract types ... ))

## `ModuleFP` vs. `AbstractAlgebra.FPModule`

- `AbstractAlgebra.FPModule` is dense
- it is written for euclidean rings
-
- `Oscar.ModuleFP` is sparse
- constructed e.g. via `free_module`
- documentation only mentions it over polynomial rings and related rings
- but e.g. `FreeMod(QQ, 2, :v)` works
- new names:
- `AbstractAlgebra.FPModule` -> `DenseFPModule`
- `Oscar.ModuleFP` -> `SparseFPModule`
- needed in the future: definition of interfaces (i.e. what does one have to implement minimally to get certain features? e.g. `lift`, `kernel`)
- and then ideally have conformance test suites for these ...

| old/current name | new name | export | comments |
|-----------------------|----------|-------------|---|
| - | new parent type `FPModule` ? |
| FPModule (abstract type!) | EuclideanFPModule | |
| FPModuleElem | EuclideanFPModuleElem | | at some point we had the rule that if we have dense and sparse variants, we omit the `Dense`
| FPModuleHomomorphism | EuclideanFPModuleHom
| FreeModule | free_module | | this is a function in AA |
| AbstractAlgebra.Generic.FreeModule | EuclideanFreeModule |
| AbstractAlgebra.Generic.FreeModuleElem | EuclideanFreeModuleElem |
| ModuleFP (abstract type) | FPModule| |
| ModuleFPElem | FPModuleElem
| ModuleFP_dec | FPDecModule | | as in MPolyRing vs. MPolyDecRing
| Map_dec |
| ModuleFPElem_dec | FPDecModuleElem
| ModuleFPHom | FPModuleHom
| ModuleFPHomDummy | FPModuleHomDummy oder so `_FPModuleHomDummy` ??? | | auxillary type for the map type system, ignore for now
| FreeMod | FreeModule
| FreeModElem | FreeModuleElem
| FreeModuleHom | FreeModuleHom
| FreeMod_dec | FreeDecModule
| FreeModElem_dec | FreeDecModuleElem
| FreeModuleHom_dec | FreeDecModuleHom
| SubQuoHom | SubquoModuleHom ???? | |
| SubQuoHom_dec | SubquoDecModuleHom ??? |
| SubModuleOfFreeModule | ??| | has no `elem_type` -- its elements are of type `FreeModElem` (similar to how elements of ideals have the ring as parent, not the ideal)
| --- |
| ---
| AA.ResidueRing (abstract type) | (remove it? not needed) |
| AA.Generic.ResidueRing | EuclideanRingResidueRing | | only really supports euclidean rings
| AA.Generic.ResidueField | EuclideanRingResidueField | | only really supports euclidean rings

> ** review `SubModuleOfFreeModule` maybe it should not be a subtype of `Module` at all as it does not satisfy the interface (its "elements", e.g. its generators, do not have it as parent / parent type), and be clearly marked as an internal helper (we already don't export it) **

We agreed that we want "long expressive" names over abbreviations for readable self-explanatory code. But in some cases, it may be worthwhile to keep the abbreviation for pragmatic reasons... but we should offer the long names. Here are some:
- [ ] gen -> generator
- [ ] gens -> generators
- [ ] conj -> conjugate
- [ ] conj! -> conjugate!
- [ ] minpoly -> minimal_polynomial (alias exists in Oscar, move to AA?)
- [ ] charpoly -> characteristic_polynomial (alias exists in Oscar, move to AA?)

# functions which check and potentially return values

## General notes
- Some functions end with `_with_data`, the vast majority end with a more descriptive name. Both are fine, as the second output of some function depends on whether the first output is `true` or `false`.

## Missing, inconsistent, or faulty documentation
("missing documentation" could also mean "should not be exported")
- `is_G_hom`
- `is_absolutely_irreducible`
- `is_admissible_ordering`
- `is_alternating_form` (compare with `is_alternating`)
- `is_anti_isometry` (compare with `is_anti_isometric_with_anti_isometry`)
- `is_binomial` (should monomials be allowed as binomials?)
- `is_chain_complex`
- `is_characteristic`
- `is_cochain_complex`
- `is_complex`
- `is_decomposable`
- `is_dense`
- `is_domain_type`
- `is_eichler`
- `is_equal_locally`
- `is_equivalent_with_isometry`
- `is_etale`
- `is_exact_type`
- `is_finite_order`
- `is_free_with_basis`
- `is_geometrically_reduced`
- `is_hermitian_form`
- `is_hermitian_matrix`
- `is_hessenberg`
- `is_hnf`
- `is_indefinite`
- `is_isometry`
- `is_kernel_polynomial`
- `is_local_square`
- `is_negative_root_with_index`
- `is_positive_entry`
- `is_positive_root_with_index`
- `is_primitive_over`
- `is_quadratic_form` (compare with `is_quadratic`)
- `is_root_with_index`
- `is_simple_root_with_index`
- `is_subset_locally`
- `is_symmetric_form`
- `is_two_sided`
- `is_unicode_allowed`

## requires ending `_with_foo`
- `is_GLZ_conjugate`
- `is_cellular`
- `is_coboundary`
- `is_compatible`
- `is_congruent`
- `is_conjugate_subgroup` (returns something, not sure what)
- `is_locally_free`
- `is_principal_fac_elem` (rename to `is_principal_with_fac_elem`)
- `is_projective`
- `is_subgroup`

##### Module construction
The following are (up to my (Lars) knowledge) only used for Lie modules (and one for modules). They all return a boolean and the data they were created with, so e.g. for `W = exterior_power(V, 3)` one would have `is_exterior_power(W) == (true, V, 3)`. I have no idea how to consistently name them apart from `with_data`. Any other ideas?
- `is_direct_sum`
- `is_dual`
- `is_exterior_power`
- `is_symmetric_power`
- `is_tensor_power`
- `is_tensor_product`

## miscellaneous
- either rename
-- `is_left`→`is_left_coset` & `is_right`→`is_right_coset`
or rename
-- `is_left_ideal`→`is_left` & `is_right_ideal`→`is_right`
- rename `is_semi_regular`→`is_semiregular` (compare to many other `is_semi*` functions)
- rename `isinfinite`→`is_inf`?
- rename `is_finiteorder`→`is_finite_order`?
- further concretise input type in docu of `is_independent`

---

- `has_preimage` and `haspreimage` should be merged and renamed to `has_preimage_with_preimage`
- careful: check if some methods maybe only return a bool...?)
- some methods have a `check` argument, what's up with that?

#### number of ...

Many different ways to express "number of" in function names are currently in use.
Proposal: for all of them, have a long (discoverable) version of the form `number_of_FOO`.
In addition, for some also have short aliases (basically: when convenient / when we already have a short base name) of the form `nFOO`. (general rule of thumb: shorthands w/o underscore, < 15 chars)

- `n_cones` -> rename to `number_of_cones` + add @alias `ncones`
- `n_connected_components` -> rename to `number_of_connected_components`
- `n_maximal_cells` -> rename to `number_of_maximal_cells` + add @alias `nmaxcells`
- `n_maximal_cones` -> rename to `number_of_maximal_cones` + add @alias `nmaxcones`
- `n_maximal_polyhedra` -> rename to `number_of_maximal_polyhedra` + add @alias `nmaxpolyhedra`
- `ncols` -> rename to `number_of_columns`, keep short as @alias
- `ndigits` -> `const number_of_digits = Base.ndigits`
- `nedges` + `ne` -> rename to `number_of_edges`, keep `nedges` as @alias (+ maybe `GraphsBase.ne` once that exists)
- `nfacets` -> rename to `number_of_*` + keep short as @alias
- `ngens` -> rename to `number_of_generators`, keep short as @alias
- `npatches` -> rename to `number_of_*` + keep short as @alias
- `npoints` -> rename to `number_of_*` + keep short as @alias
- `npolyhedra` -> rename to `number_of_*` + keep short as @alias
- `nrays` -> rename to `number_of_rays`, keep short as @alias
- `nrels` -> rename to `number_of_relations`, keep short as @alias
- `nrows` -> rename to `number_of_rows`, keep short as @alias
- `num_partitions` -> rename to `number_of_partitions` + add @alias `npartitions`
- `num_positive_roots` -> rename to `number_of_positive_roots` + add @alias `nposroots`
- `num_roots` -> rename to `number_of_roots` + add @alias `nroots`
- `num_simple_roots` -> rename to `number_of_simple_roots` + add @alias `nsimpleroots`
- `num_standard_tableaux` -> rename to `number_of_standard_tableaux`
- `number_atlas_groups` -> rename to `number_of_atlas_groups`
- `number_conjugacy_classes` -> rename to `number_of_*`
- `number_moved_points` -> rename to `number_of_*`
- `number_of_lattices` -> add @alias `nlattices`
- `number_perfect_groups` -> rename to `number_of_*`
- `number_primitive_groups` -> rename to `number_of_*`
- `number_small_groups` -> rename to `number_of_*`
- `number_transitive_groups` -> rename to `number_of_*`
- `nvars` -> add `number_of_variables`, keep short as @alias
- `nvertices` + `nv` -> rename to `number_of_vertices`, keep `nvertices` as @alias (+ maybe `GraphsBase.nv` once that exists)

All of this gets implemented in [AbstractAlgebra.jl#1553](https://github.com/Nemocas/AbstractAlgebra.jl/pull/1553), [Nemo.jl#1624](https://github.com/Nemocas/Nemo.jl/pull/1624), [Hecke.jl#1364](https://github.com/thofma/Hecke.jl/pull/1364), [Oscar.jl#3272](https://github.com/oscar-system/Oscar.jl/pull/3272).

### Random Changes (accumulated from above)

| done | old | new | PR | more info |
| -------- | -------- | -------- | -------- | -------- |
| [x] | `is_isomorphic_with_alternating_group` | `is_isomorphic_to_alternating_group` | [Oscar.jl#3170](https://github.com/oscar-system/Oscar.jl/pull/3170) |also adjust phrasing in docstring|
| [x] | `is_isomorphic_with_symmetric_group` | `is_isomorphic_to_symmetric_group` | [Oscar.jl#3170](https://github.com/oscar-system/Oscar.jl/pull/3170) | also adjust phrasing in docstring |
| [x] | `are_algebraically_independent` | `is_algebraically_independent` | | ("a vector is alg indep if its elements are alg indep") |
| [ ] | `psylow_subgroup` | `sylow_subgroup` | | [Hecke.jl#1015](https://github.com/thofma/Hecke.jl/issues/1015)), but also make sure the retun values match (the subgroup and the embedding |
| [ ] | | | | |

## Type names for maps/homomorphism

some tubtypes of `Map`:
* AbsLocalizedRingHom
* AbstractAlgebra.Generic.MapCache
* AbstractAlgebra.Generic.MapWithRetraction
* AbstractAlgebra.Generic.MapWithSection
* AbstractAlgebra.Generic.ModuleHomomorphism
* AbstractAlgebra.Generic.ModuleIsomorphism
* AbstractSpaceMor
* AbstractSpaceRes
* GAPGroupHomomorphism
* GrpAbFinGenMap
* Hecke.AbToNfMultGrp
* Hecke.AbsOrdToAlgAssMor
* Hecke.DiscLogLocallyFreeClassGroup
* Hecke.MapClassGrp
* Isogeny
* Oscar.RingFlattening
* ...

Can we come up with some suggestions that people can look at to get an idea what to call their maps?

There are quite different patterns here, depending on the scenario. Collect some scenarios with suggestions how to name types:
- is a morphism or "just" a map?
- interpretation maps
- domain and codomain same type or not?
-
- ...

Some random thoughts
- Decide `Hom` or `Homomorphism` or `Mor` or ...
- Use `Map` if it not necessarily structure preserving ?
-

## AbstractAlgebra

- `divides(a, b)`
- some dispatches only return bool -> `is_divisible_by` instead
- others return `(bool, c)` where for `bool=true` we have `a = b*c`. Lars: should this be something like `is_divisible_by_with_quotient`??? (ref for the current state: https://github.com/Nemocas/Nemo.jl/issues/862#issuecomment-669289362)

- `Fac` -> ??

## Nemo

Everything from the table (all but `Fac`) has been taken care of in https://github.com/Nemocas/Nemo.jl/pull/1638.

| old/current name | new name | export | comments |
|-----------------------|----------|-------------|---|
| PadicField | (keep) | | `?PadicField` gives documentation for `FlintPadicField`
| FlintPadicField | (remove) | |
| padic | PadicFieldElem | |
| QadicField | (keep) | | `?QadicField` gives documentation for `FlintQadicField`
| FlintQadicField | (remove) | |
| qadic | QadicFieldElem | |
| arb | ArbFieldElem | | maybe can stay as it is more an "intern" type (use `RealFieldElem` instead); maybe we can just un-export it and the other arb/acb types
| arb_poly | ArbPolyRingElem
| arb_mat | ArbMatrix
| acb | AcbFieldElem
| acb_poly | AcbPolyRingElem
| acb_mat | AcbMatrix
| -----
| ~~AnticNumberField~~ | | | ~~stays (not a good name but we have no better)~~ see https://hackmd.io/ONVPdlfARgWzkfkNkcJkgA
| ~~nf_elem~~ | ~~AnticNumberFieldElem~~| | ~~should match `AnticNumberField`~~ see https://hackmd.io/ONVPdlfARgWzkfkNkcJkgA
| CalciumField | | | stays (not a good name but we have no better)
| ca | CalciumFieldElem | | should match `CalciumField`
| Fac | Factorization???
| Loc | LocalizedEuclideanRing |
| LocElem | LocalizedEuclideanRingElem |
| lll_ctx | LLLContext
| qqbar | QQBarFieldElem | |
| CalciumQQBarField | QQBarField| | [see issue](https://github.com/Nemocas/Nemo.jl/issues/1394)
| ---
| FlintQQiField | QQiField
| fmpqi | QQiFieldElem | do not satisfy number field interface, should not be exported
| FlintZZiRing | ZZiRing
| fmpzi | ZZiRingElem | do not satisfy number field interface, should not be exported
| fmpzUnitRange | ZZRingElemUnitRange

## Hecke

[Table of Hecke types to be renamed](https://hackmd.io/ONVPdlfARgWzkfkNkcJkgA)

- rename `subalgebra` to `sub` ?

## Oscar

## random notes

things noticed while discussing
- there should be `spec` as alias/alternative to `Spec`
- `is_invertible` versus `is_isomorphism`
- for some morphisms we have one, for some the other
- perhaps we should have both at least were it makes sense (i.e. were invertible morphisms are isomorphism)
- there is `is_invertible_with_inverse` but no `is_isomorphism_with_inverse` ...
- consistency: `quo` and `sub` methods
- sometimes return just the quotient/sub object;
- some also return a morphism;
- some have a `task` argument
- should we have `sub` and also `sub_with_embedding` / `sub_with_map` / `sub_with_data` ? etc.
- ditto for `quo` versus `quo_with_map` / `quo_with_data`
- Concerning `quo`:
- There are 71 `methods(quo)`. Most of them return two arguments, the quotient and the projection.
- Most of the others (16 methods, all for the first argument either a `FreeMod` or a `SubquoModule`) also return *by default* these two results, but an optional keyword argument `task` allows one to change the output in order to omit the quotient or the projection.
- The `quo(M::ModuleFP{T}, V::Vector{<:ModuleFPElem{T}}, task::Symbol)` methods are documented (with examples!) but just throw an error.
- The `quo(tbl::Oscar.GAPGroupCharacterTable, nclasses::Vector{Int64})` method also returns two results, but the second is not a map but the array of integers (the class fusion) that describes the projection; note that the quotient table does not know about a corresponding group.
- Thus is there anything to do for `quo`?
- Concerning `sub`:
- There are 69 `methods(sub)`, of which 34 return the substructure together with an embedding.
- 10 methods, all for the first argument either a `FreeMod` or a `SubquoModule`, also return *by default* these two results, but an optional keyword argument `task` allows one to change the output in order to omit the sustructure or the embedding.
- 19 methods deal with *matrices* and return submatrices. Only one of them has a docstring.
- 4 methods for Lie algebras return just the substructure.
- The `sub(M::ModuleFP{T}, V::Vector{<:ModuleFPElem{T}}, task::Symbol)` methods are documented (with examples!) but just throw an error.
- In principle, it is o.k. to have different `sub` behaviour for matrices and algebraic structures. In this sense, only the methods for Lie algebras are not consistent.
- OSCAR 1.0 issue says the following and we don't know what it refers to (maybe that `Inv` is bad and should be `Invar`, e.g. `InvRing -> InvarRing`):
> Inv and Invar in type names
- there are some function that gives all "sub" for certain types:
- subfields
- subgroups
- submodules
- should these be unified to `subs` ??!?
- perhaps not...