Refactor axioms

[nthiery]

This ticket brings:

• a catalog of axioms axioms.XXX in the global namespace

• axioms as objects rather than plain strings. This had been wished
  for during the review Axioms and more functorial constructions #10963, and some of the code is reminiscent of
  Volker's draft at floor(), ceil(), trunc(), round() for AA #15501. The main advantage for now is to enable
  the definition of axioms with custom printing, thus getting rid of the
  crapy monolithic customization section.

  The bijection axiom <-> "axiom name" remains mandatory to support
  the syntax for implementing axioms in a category.

• improved heuristic for printing categories with axioms. E.g.:

  Category of finite enumerated fields  ->  Category of finite fields
  Category of finite enumerated permutation groups  ->  Category of finite permutation groups
  

  improved printing of AssociativeXXX:

  Category of additive commutative additive magmas ->
  Category of additive-commutative additive magmas
  

  This was discussed and requested on sage-devel for better
  readability when there are many axioms printed. E.g. in

  sage: Semirings().super_categories()[0]
  Category of associative additive-commutative additive-associative additive-unital distributive magmas and additive magmas
  

  The above are the reasons why the commits touch many files)

• some simplifications in the code (e.g. no more _repr_object_names_static)

The code is essentially backward compatible, at the price of having
axioms.Associative == "Associative", with compatible hash values.
Using axioms as strings is deprecated in code. In particular, the
ticket updates the Sage library in a few places to use the preferred
idiom P in C.Axiom rather than "Axiom" in P.category().axioms().

Remaining questions:

• Inside the code, do we want to call the catalog of axioms
  all_axioms or axioms? The latter is more ambiguous,
  but consistent with the name exported in global name space.

• To fetch an existing axiom from a variable containing either the
  axiom or its name, the current idiom is all_axioms(name); that's
  somewhat consistent with the P(xx) idiom to create elements of a
  parent. Would we want some other idiom? all_axioms[name]?

• Do we want to implement a cmp method for axioms?

  • Comparing by string: this could avoid the repeated use of
    sorted(...,key=str) in our documentation
  • Comparing by index/definition time? This would be plausibly more
    meaningful (axioms shown in some natural order). And simplify
    a very tiny bit the one place where this sorting order is used in
    the code.

• Do we want to Cythonize axiom.py? Now? Later?

• Do we want to import all_axioms from category_with_axiom rather than
  axiom? This would avoid a double import in all the files specifying
  _base_category_and_axiom

• Do we want "additive magmas" to be renamed to "additive-magmas" for consistency?

• Do we want to fix the inconsistency below by adding additive- in
  the first output?

  sage: CommutativeAdditiveGroups()
  Category of commutative additive groups
  sage: AdditiveMagmas().AdditiveCommutative()
  Category of additive-commutative additive magmas
  

• To recover the string associated to an axiom in the code, is it fine
  to use str(axiom), or do we want something more explicit and
  possibly distinct from printing like axiom.name()?

• Any better name for axiom.index()? It's only used internally,
  and in a single place; we could also just use axiom._index.

Work for followup tickets:

• Improve the guessing to avoid having to specify
  _base_category_class_and_axiom in
  FiniteDimensionalLieAlgebrasWithBasis and friends.

• Improve the pretty printing to get:

  Category of associative additive-commutative additive-associative additive-unital distributive magmas and additive magmas
  ->
  Category of distributive semigroups and additive-commutative additive monoids
  

CC: @tscrim @simon-king-jena @vbraun @jpflori

Component: categories

Author: Nicolas M. Thiéry

Branch/Commit: u/nthiery/refactor_axioms @ f4ce06a

Issue created by migration from https://trac.sagemath.org/ticket/22965

[nthiery]

Branch: u/nthiery/refactor_axioms

[tscrim]

comment:4

I know it's not ready for review, but something I've been bit by in the past is only defining an __eq__. Make sure you define a __ne__ as return not (self == other).

---

New commits:

4b21055

22965: refactorisation of axioms (code part)

[tscrim]

Commit: 4b21055

[sagetrac-git]

Changed commit from 4b21055 to 47d3b7c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

af59aff	22965: Trivial doctest updates ('XXX' -> XXX, nicer category names)
47d3b7c	22965: updates in the categories to use axioms rather than strings

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c4e0fb5

22965: idiom update: use P in C.Axiom rather than "Axiom" in P.category().axioms()

[sagetrac-git]

Changed commit from 47d3b7c to c4e0fb5

[sagetrac-git]

Changed commit from c4e0fb5 to efedd94

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

efedd94

22965: documentation updates

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

0f2464d

22965: refactorisation of axioms (code part: hunks missing from previous commit)

[sagetrac-git]

Changed commit from efedd94 to 0f2464d

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

75ea783

22965: Trivial doctest updates ('XXX' -> XXX, nicer category names)

[sagetrac-git]

Changed commit from 0f2464d to 75ea783

[nbruin]

comment:12

If you're going to override "eq" and "hash" you should probably define the thing to be CachedRepresentation. Is there a good reason to be CachedRepresentation?

(CachedRepresentation has a very nontrivial cost. You should only use it if you need its features)

[nthiery]

comment:14

Replying to @nbruin:

If you're going to override "eq" and "hash" you should probably define the thing to be CachedRepresentation. Is there a good reason to be CachedRepresentation?

(CachedRepresentation has a very nontrivial cost. You should only use it if you need its features)

That's a good question. The __eq__ and __hash__ are really here just for backward compatibility (supporting the idiom "Foo" in C.axioms()). I used UniqueRepresentation because that's really what I mean :-) But you are right, it could be more proper to use CachedRepresentation.

In terms of speed, I believe the only important thing is fast equality and hash (which calls for Cythonizing them). Axioms will rarely be recreated.

Cheers,
Nicolas

[nthiery]

comment:16

Replying to @tscrim:

I know it's not ready for review, but something I've been bit by in the past is only defining an __eq__. Make sure you define a __ne__ as return not (self == other).

Yeah, I pondered about this. Accepting "Associative" == axioms.Associative is really just here as a workaround for backward compatibility; namely to support idiom "Associative" in C.axioms() that people apparently have been using. I wondered whether I really wanted to enable more than just the strict minimum.

[tscrim]

comment:17

If you want to keep the __eq__, then you really need to define a __ne__ to keep things consistent in case someone wants to test inequality. A slightly contrived example, but say someone has a constructor that accepts an axiom as an arg and they want to do something special when the axiom is not axioms.Commutative.

Also, __init__, __repr__, and index of Axiom do not have doctests.

[tscrim]

comment:18

I'm not really commenting on whether you should define an __eq__ or not.

[tscrim]

comment:19

Also, why does Axiom inherit from SageObject? What features are you using?

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

126bb50

22965: fixed missing doctests

[sagetrac-git]

Changed commit from 75ea783 to 126bb50

[nthiery]

comment:21

Replying to @tscrim:

Also, why does Axiom inherit from SageObject? What features are you using?

I believe none at this stage. I just put that by default, mostly for consistency with Category.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

f4ce06a

22965: added Axiom.__ne__ as negation of Axiom.__eq__

[sagetrac-git]

Changed commit from 126bb50 to f4ce06a

[nthiery]

comment:24

Hi; any comment on the questions raised in the description?

[tscrim]

comment:25

• Inside the code, do we want to call the catalog of axioms
  all_axioms or axioms? The latter is more ambiguous,
  but consistent with the name exported in global name space.

I say axioms both for consistency and naturality.

• To fetch an existing axiom from a variable containing either the
  axiom or its name, the current idiom is all_axioms(name); that's
  somewhat consistent with the P(xx) idiom to create elements of a
  parent. Would we want some other idiom? all_axioms[name]?

I would go for the latter since it is a container object and all_axioms[name] is how Python handles other container objects, e.g., dict.

• Do we want to implement a cmp method for axioms?

No, but rich comparisons are good.

• Comparing by string: this could avoid the repeated use of
  sorted(...,key=str) in our documentation
• Comparing by index/definition time? This would be plausibly more
  meaningful (axioms shown in some natural order). And simplify
  a very tiny bit the one place where this sorting order is used in
  the code.

Definitely Index/definition time as this will allow us to control the printing in a more straightforward way.

• Do we want to Cythonize axiom.py? Now? Later?

I am inclined to say Cythonize it to keep the categories as fast as possible. I feel like even if highly optimized, this switch will slow down category creation because the string operations are lightweight and optimized at the machine level. However, I haven't done any timings.

Also, it would be good if you could get rid of the SageObject inheritance from Axiom so we could cdef it, and I don't see axioms fundamentally behaving like other instances of SageObject.

• Do we want to import all_axioms from category_with_axiom rather than
  axiom? This would avoid a double import in all the files specifying
  _base_category_and_axiom

• Do we want "additive magmas" to be renamed to "additive-magmas" for consistency?

I don't think the hyphen there is completely grammatically correct, at least it is strange to me. So I'm -1.

• Do we want to fix the inconsistency below by adding additive- in
  the first output?

  sage: CommutativeAdditiveGroups()
  Category of commutative additive groups
  sage: AdditiveMagmas().AdditiveCommutative()
  Category of additive-commutative additive magmas
  

I would like the latter to be

Category of commutative additive magmas

as this is more concise and easier to parse.

• To recover the string associated to an axiom in the code, is it fine
  to use str(axiom), or do we want something more explicit and
  possibly distinct from printing like axiom.name()?

str(axiom) is the most natural for me, but it doesn't hurt to have an alias axiom.name().

• Any better name for axiom.index()? It's only used internally,
  and in a single place; we could also just use axiom._index.

I would get rid of it all together and (mildly) break encapsulation here by using axiom._index. It is both faster, only done in one place, and is an implementation detail.

[koffie]

comment:27

Looks like there are a lot of things in the comments to still be addressed and merge conflicts that need to be resolved.