Open wbhart opened 3 years ago
Here are some more things. This function is in the abstract section right? Yet the function doesn't work on all implementations. Furthermore, it makes assumptions about decending exponents, which is not correct in general.
EDIT: This is fixed now
Recently (on PR #887) we split the generic code in AbstractAlgebra into implementations for abstract types (in src) and implementations for generic types (in src/generic). However, this was a first order approximation to the task. Some improvements could be made over time. Note that not all these suggestions are sensible and we should discuss whether we actually want them or not.
[ ] Perhaps similar, zero, polynomial, which generate generic types belong in the generic module. The reason I haven't put them in there now is that these are the generic functions which other packages will need to overload (e.g. Nemo), and it is natural that they belong in AbstractAlgebra. They are generic fallbacks, if you like, in case those packages don't implement something specific. To reflect this, the functions themselves could possibly be split into two parts, one in src/generic the other in src. As it is, even though they can only generate generic types, the functions still provide functionality to all types implemented by other packages.
[ ] There are a handful of AbstractAlgebra.blahs still left in Generic files. These could be omitted in some cases by importing the relevant symbols into Generic. In most cases this is not desirable, but there are some places where it would be desirable.
[ ] I have introduced CommonTypes.jl. It contains the definition of Perm and CycleDec which are types needed by matrices. The problem is that functions for matrices cannot refer to Generic types in their signatures (it's fine to refer to them in their bodies), because types must be defined before use. But Generic types don't get defined until after all the files in AbstractAlgebra, as Generic needs to import those functions to overload them. It may be possible to define things in the other order so that this problem goes away.
[ ] I have not attempted to split the test code into abstract and generic parts. Probably cleaning up the test code should be combined with checking that our generic types actually conform to the interfaces they are supposed to, and that unsafe operators are properly tested (especially alias tested), which in many cases they are not.
[ ] I have not dealt with the files that have arisen from previous partial attempts at this project, e.g. in the algorithms directory. These should now be in the src directory.
[ ] There are some functions that I've moved in Generic purely because there is no abstract type (e.g. AbsSeriesRing) that could be used in their signature. There is nothing in their implementation which prevents them being fallbacks for all AbsSeries, etc. This is something that could be fixed.
[ ] In src/RelSeries.jl, set_valuation!, set_precision!, set_length! function as generic fallback functions for all series types, assuming they have val, prec and length fields. I don't know if this is desirable or not. At the end of the day we can make arbitrary assumptions about the types people might implement following an interface. But providing ones like this might just cause more grief when they discover there are functions not in the required interface that they actually need due to the contents of the struct they have implemented. I prefer to give some good examples of creating rings that follow our interfaces, rather than oversimplifying the required interfaces, which basically defeats the purpose of having them.
[ ] Two of the Submodule constructors (sub) had to be left in AbstractAlgebra.jl because they refer to the Generic.Submodule type in their signatures and so must be defined after Generic. Similarly for one of the snf functions in InvariantFactorDecomposition.jl and maybe a couple of other functions.
[ ] The distinction between Generic and Abstract in the case of (finitely presented) Modules and ModuleHomomorphisms is not very clear. There is no clear interface for these things. On the one hand, for finitely presented modules, for example, there are numerous concrete types belonging to the abstract type FPModule. But the implementation depends heavily on a particular representation of the data rather than an interface, which means the abstract code is not particularly general here. There are also hacks like the _matrix function (implemented as a type stable accessor) which are called from supposedly abstract code. The map field of the module types is also accessed directly instead of via an interface. This is surely not intended to be part of an official interface. The nature of finitely presented modules is such that they are sufficiently diverse that one probably doesn't want a terribly strict interface, making them essentially unsuitable for truly generic code. One wants at best some general uniformity, rather than a strict interface.
[ ] Maps are designed to be completely general, but a lot of the Generic code provides functionality for all maps (e.g. generic compositions), which can be somewhat confusing. I've taken the approach that code which implements functionality (no matter how generic its intended use) for concrete Map types is Generic, not Abstract. Only code which is representation independent which is written to a Map interface is considered truly abstract. I think this is the way it should be, but perhaps up for discussion.
[ ] In src/generic/MPoly.jl, some of the evaluation code would work just fine in src/MPoly.jl, but < Julia-1.5 doesn't support adding methods to abstract types and therefore this currently can't be supported. When we finally make Julia-1.5 the minimum requirement, we can move this code over.