Lazy species

[mantepse]

We provide an implementation of combinatorial species based on the lazy series framework and #38446.

dependencies: #38974

[github-actions]

Documentation preview for this PR (built with commit ebddbd4; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[tscrim]

A few small things.

Thank you!

I do really think putting the specific species for the univariate case as methods onto the parent class is much better: no repetition of documentation, it directly gives you the corresponding class (which makes it easier to use ?), and it’s a fairly natural way to handle the simple redirection. Yet if you still remain unconvinced, we can leave it the way it currently is.

I'm confused now - I think I followed your idea of separating univariate and multivariate species (#38544 (comment)) and creating proper methods for each. Apparently I misunderstood you.

No, you understood me correctly, I just changed my mind with what might be the best practice. (I"m not 100% convinced yet either though.) Sorry for the confusion.

So what I am thinking is your original idea is best with one small modification (from this comment):

def __init__(self,...):
    if self._arity == 1:
        from types import MethodType
        self.Sets = MethodType(SetSpecies, LazySpeciesUnivariate)
        self.Graphs = MethodType(GraphSpecies, LazySpeciesUnivariate)
        self.SetPartitions = MethodType(SetPartitionSpecies, LazySpeciesUnivariate)
        self.Cycles = MethodType(CycleSpecies, LazySpeciesUnivariate)

This way it isn't a lambda function, but has the full (class-level) docstring. The downside is that they won't be included in the doc. The other variation is keeping both classes, but having the univariate as

class LazySpeciesUnivariate(LazySpecies):
    """
    Univariate lazy species.
    """

from types import MethodType
LazySpeciesUnivariate.Sets = MethodType(SetSpecies, LazySpeciesUnivariate)
LazySpeciesUnivariate.Graphs = MethodType(GraphSpecies, LazySpeciesUnivariate)
LazySpeciesUnivariate.SetPartitions = MethodType(SetPartitionSpecies, LazySpeciesUnivariate)
LazySpeciesUnivariate.Cycles = MethodType(CycleSpecies, LazySpeciesUnivariate)

(Python doesn't seem to have a good mechanism in Python for converting classes into methods like how I am doing.)

[mantepse]

Indeed, it seems that AbstractSetPartition._repr_ behaves badly:

sage -t --warn-long 5.0 --random-seed=89871645352471344146790937796251140637 src/sage/rings/lazy_species.py
**********************************************************************
Error: Failed example:: Got: [{{'a', x}, {1}},
 {{'a'}, {1}, {x}},
 {{1, 'a', x}},
 {{1, 'a'}, {x}},
 {{x, 1}, {'a'}}]
                                                                                                                                                                                                                                                                                                                                                                                          
    sorted(G.structures(["a", 1, x]), key=str)
Expected:
    [{{'a', x}, {1}},
     {{'a'}, {1}, {x}},
     {{1, 'a', x}},
     {{1, 'a'}, {x}},
     {{1, x}, {'a'}}]
Got:
    [{{'a', x}, {1}},
     {{'a'}, {1}, {x}},
     {{1, 'a', x}},
     {{1, 'a'}, {x}},
     {{x, 1}, {'a'}}]
**********************************************************************
1 item had failures:
   1 of   5 in sage.rings.lazy_species.LazySpeciesUnivariate.SetPartitions
    [290 tests, 1 failure, 15.54s wall]

Oh yes. The code is

    def _repr_(self):
...
        try:
            s = [sorted(x) for x in self]
        except TypeError:
            s = [sorted(x, key=str) for x in self]
        return '{' + ', '.join('{' + repr(x)[1:-1] + '}' for x in s) + '}'

and it's a certain M. Rubey who is to blame. #38974. Embarassing.

[mantepse]

    def _repr_(self):
...
        try:
            s = [sorted(x) for x in self]
        except TypeError:
            s = [sorted(x, key=str) for x in self]
        return '{' + ', '.join('{' + repr(x)[1:-1] + '}' for x in s) + '}'

I think the problem is that, for the set partition $\{\{1, x\}, \{"a"\}\}$, the order of 1 and x is random, because they are coerced into the symbolic ring.

[mantepse]

So what I am thinking is your original idea is best with one small modification [...]

I tried the second variant, but it actually doesn't work the way I did it. Apparently, MethodType passes the class rather than the instance of the class.

However, it seems to me that the current approach (i.e., separate classes for univariate and multivariate species and providing proper methods defined in the class) makes sense. I think that we really want to see the methods (Sets, Graphs, ...) in the documentation, and it seems that the pattern of not having a class docstring for SetSpecies and using the SetSpecies.__init__ docstring just for running the testsuite is easy to understand and extend.

[mantepse]

Ping?

[tscrim]

So what I am thinking is your original idea is best with one small modification [...]

I tried the second variant, but it actually doesn't work the way I did it. Apparently, MethodType passes the class rather than the instance of the class.

Ah, right, it would pass the class rather than the instance. I think I messed up my initial experiment. However, this does work:

sage: class Foo():
....:     def __init__(self, B):
....:         print(B)
sage: class Bar():
....:     def __init__(self, v):
....:         self.v = v
....:         from types import MethodType
....:         self.F = MethodType(Foo, self)
....:     def __repr__(self):
....:         return "Bar: %s"  % self.v
sage: B = Bar(5)
sage: B.F()
Bar: 5
<__main__.Foo object at 0x723f4645e660>

However, it seems to me that the current approach (i.e., separate classes for univariate and multivariate species and providing proper methods defined in the class) makes sense. I think that we really want to see the methods (Sets, Graphs, ...) in the documentation, and it seems that the pattern of not having a class docstring for SetSpecies and using the SetSpecies.__init__ docstring just for running the testsuite is easy to understand and extend.

Yes, that is the downside with my MethodTypes approach. Anyways, let's go with the current version then.

[mantepse]

Does there remain anything that I should do or do you just want to have a closer look?

The next (substantial) improvements are #40129, #40163 and #40186. Arithmetic product is easy and in the pipeline. After that, we have truly superseeded the old species framework.

[tscrim]

One doc thing and one general question (that I probably should have asked much earlier, sorry).

Should we call these Lazy(Combinatorial)Species and not just (Combinatorial)Species? Having Lazy in front suggests there is or will be a finite precision/non-lazy version, but I don't think this will happen.

Once addressed, it will be a positive review.

[tscrim]

We probably should put this with the L's rather than at the bottom. Also, would there be some other natural place to link this doc? It's hard to find just being in the big combinat module list.

[mantepse]

you mean, move it between k_tableau and lr_tableau?

[tscrim]

Yes, that’s correct.

[mantepse]

Should I move cfinite_sequence, too, or in a different PR?

[tscrim]

While I would normally say it’s better to move it in a different PR, we can just get it done here

[mantepse]

One doc thing and one general question (that I probably should have asked much earlier, sorry).

No problem! I'm amazed and it really makes me happy that you do thorough reviews!

Should we call these Lazy(Combinatorial)Species and not just (Combinatorial)Species? Having Lazy in front suggests there is or will be a finite precision/non-lazy version, but I don't think this will happen.

I put the Lazy in front because of PolynomialSpecies, which is not a very good name, and because of uniformity and discoverability. The latter, because I would think that to many formal power series guys species won't occur as a possibly better tool, even when it is.

I agree that a finite precision version won't happen.

We could have CombinatorialSpecies for what's now PolynomialSpecies and LazyCombinatorialSpecies instead of LazySpecies. (By coincidence, LazyC + tab is currently unused, whereas we have LazySy + tab and LazySp + tab. I'd support that, but I won't advocate for it.

The reason for not advocating: AtomicSpecies and MolecularSpecies were useful to me frequently (mostly to generate all of a given grade), whereas PolynomialSpecies are only a unwanted necessity. PolynomialSpecies occur when extracting a term of a LazySpecies and to construct LazySpecies by passing a coefficient function.

[tscrim]

I don’t think the lazy in from makes it very discoverable compared to the API we have for similar such objects (mainly completion aka formal_series_ring). It would only be if someone is already working with the other lazy series that they would find it, but they wouldn’t know what it is and ignore it. I think CombinatorialSpecies would be the ideal name with my current understanding. PolynomialSpecies will just remain as it is, which I feel fits well enough with the power series vs polynomials differences. Putting it another way, I don’t think it should be defined by its laziness (in contrast to FPS with finite precision where we have different implementations).

[mantepse]

I think CombinatorialSpecies would be the ideal name with my current understanding. PolynomialSpecies will just remain as it is, which I feel fits well enough with the power series vs polynomials differences. Putting it another way, I don’t think it should be defined by its laziness (in contrast to FPS with finite precision where we have different implementations).

Just a brief update: I am not sure. Essentially, that does mean that we should make LazyPowerSeriesRing an implementation of PowerSeriesRing.

Another thing I just noticed is that CombinatorialSpecies(min=None) is currently in the global namespace: it designates a currently undefined species, with valuation min. Not sure how to do the deprecation here.

I guess we could first leave things as they are (perhaps rename LazySpecies to LazyCombinatorialSpecies) and after the deprecation period drop Lazy.

[tscrim]

I think CombinatorialSpecies would be the ideal name with my current understanding. PolynomialSpecies will just remain as it is, which I feel fits well enough with the power series vs polynomials differences. Putting it another way, I don’t think it should be defined by its laziness (in contrast to FPS with finite precision where we have different implementations).

Just a brief update: I am not sure. Essentially, that does mean that we should make LazyPowerSeriesRing an implementation of PowerSeriesRing.

In the sense of a common abstract class and creation (with the default precision being passed as oo), then I would not be opposed. However, because there is both a finite precision and lazy version, it makes some sense to have them be separate. That being said, I do think it would be better to have more common API and base classes, but that should be done separately.

Another thing I just noticed is that CombinatorialSpecies(min=None) is currently in the global namespace: it designates a currently undefined species, with valuation min. Not sure how to do the deprecation here.

I guess we could first leave things as they are (perhaps rename LazySpecies to LazyCombinatorialSpecies) and after the deprecation period drop Lazy.

Hmm….that’s really annoying. We can’t really just do a drop-in replacement as it has different API, right? We could do a 2 step deprecation: the CombinatorialSpecies and then Lazy[Combinatorial]Species after removing Lazy from it. Yet that’s a bit of work. There might not be a good option here. :/

[mantepse]

I would like to move on here. Meanwhile, I have also implemented the arithmetic product. I agreed to give an online course on enumeration with sagemath at 128.5, but I guess it is too late to have this even in develop.

[tscrim]

Okay, we can merge this now. I would like to deprecate LazyCombinatorialSpecies once we have the full replacement for the old version ready, but we can address that at that time.

[mantepse]

Thank you!