Adding Method for testing avoidance in posets

[d561a5fa-51dc-41bc-86f5-988f7f02b59b]

This is a test for finite posets to check if they do not have induced posets isomorphic to the union of two disjoint chains. Example 3+1 free, 2+2 free.

Apply:

• attachment: poset_functionality_14099-er.patch
• attachment: trac_14099-rev.patch

Depends on #14536

CC: @sagetrac-chrisjamesberg @sagetrac-ahmorales @saliola @nathanncohen @nthiery @hivert

Component: combinatorics

Keywords: posets, days45, days49

Author: Eric Rowland, Alejandro Morales

Reviewer: Chris Berg

Merged: sage-5.11.rc0

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

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:1

Could you be interested in running this method on the transitive closure of your poset ?

http://www.sagemath.org/doc/reference/sage/graphs/generic_graph.html#sage.graphs.generic_graph.GenericGraph.subgraph_search

Nathann

[d561a5fa-51dc-41bc-86f5-988f7f02b59b]

comment:3

apply: poset_functionality_14099_er.patch

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:4

Is the transitive closure + subgraph_search fast enough to make it worthwhile?

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:5

Is the transitive closure + subgraph_search fast enough to make it worthwhile?

Well... It is sligthly smarter than what you do (it would not enumerate all chains of length 'n' beginning with an element v, if this element v is comparable with an element of the chain of length 'm' you already picked) and it is implemented in Cython.

I'd say that it's definitely worth a try.

Nathann

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:6

apply: poset_functionality_14099_er.patch

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:7

I don't think subgraph_search answers the same question....

Using induced=True results in the poset Poset({0 : [], 1 : [2], 2 : [3]}) which is 3+1 being (3+1)-free:

# g is the transitive closure
sage: g = DiGraph({0 : [], 1 : [2, 3], 2 : [3]})
sage: g.subgraph_search(DiGraph({0 : [1], 1 : [2]}) + DiGraph({0 : []}), induced=True)

Using induced=False results in the poset Poset({0 : [1, 2], 2 : [3]}) which is (3+1)-free containing (3+1):

# g is the transitive closure
sage: g = DiGraph({0 : [1, 2, 3], 2 : [3]})
sage: g.subgraph_search(DiGraph({0 : [1], 1 : [2]}) + DiGraph({0 : []}), induced=False)
Subgraph of (): Digraph on 4 vertices

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:8

you definitely need to use induced = True, but not on the digraphs. As I said you need to work on their transitive closure :

def test(g,n,m):
    n = 3
    m = 1
    pattern = digraphs.Circuit(n+1); pattern.delete_vertex(n)
    pattern += digraphs.Circuit(m+1); pattern.delete_vertex(n+m-1)
    c = g.transitive_closure().subgraph_search(pattern.transitive_closure(), induced = True)
    if c:
      print "Pattern found with chains", c.connected_components()
    else:
      print "No pattern in this graph"

g = DiGraph({0 : [], 1 : [2, 3], 2 : [3]})
test(g,3,1)

Nathann

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:9

Yes, you are right; it seems to work. It doesn't seem to be any faster for posets on 7 elements, but perhaps it's worth having both options available.

sage: def test1(poset, m, n):
....:     for mchain in poset.chains().elements_of_depth_iterator(m):
....:         for nchain in poset.chains().elements_of_depth_iterator(n):
....:             if all(poset.compare_elements(x, y) is None for x in mchain for y in nchain):
....:                 return False
....:     return True
....: 
sage: %time len([p for p in Posets(7) if test1(p, 3, 1)])
CPU times: user 22.46 s, sys: 0.03 s, total: 22.49 s
Wall time: 22.49 s
639

sage: def test2(poset, m, n):
....:     relations = poset.relations()
....:     g = DiGraph([rel for rel in relations if rel[0] != rel[1]])
....:     g.add_vertices([rel[0] for rel in relations if rel[0] == rel[1]])
....:     mchain = digraphs.Circuit(m+1)
....:     mchain.delete_vertex(m)
....:     nchain = digraphs.Circuit(n+1)
....:     nchain.delete_vertex(n)
....:     return g.transitive_closure().subgraph_search((mchain + nchain).transitive_closure(), induced = True) is None
....: 
sage: %time len([p for p in Posets(7) if test2(p, 3, 1)])
CPU times: user 23.47 s, sys: 0.01 s, total: 23.48 s
Wall time: 23.48 s
639

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:10

On the other hand, for larger posets using the graph search does much better:

sage: posets = [Posets.RandomPoset(30, .5) for x in range(10)]
sage: %time len([p for p in posets if test1(p, 3, 1)])
CPU times: user 20.69 s, sys: 0.00 s, total: 20.69 s
Wall time: 20.69 s
6
sage: %time len([p for p in posets if test2(p, 3, 1)])
CPU times: user 0.19 s, sys: 0.00 s, total: 0.19 s
Wall time: 0.19 s
6

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

Changed keywords from posets to posets, days45

[d561a5fa-51dc-41bc-86f5-988f7f02b59b]

Changed author from chrisjamesberg, rowland, ahmorales to Eric rowland, Alejandro Morales

[d561a5fa-51dc-41bc-86f5-988f7f02b59b]

Changed reviewer from saliola to Chris Berg

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:13

I just created ticket #14122 because of this ticket.

With that ticket, the following code

mchain = digraphs.Circuit(m+1)
mchain.delete_vertex(m)
nchain = digraphs.Circuit(n+1)
nchain.delete_vertex(n)
return g.transitive_closure().subgraph_search((mchain + nchain).transitive_closure(), induced = True) is None

can be replaced by

twochains = digraphs.Tournament(n) + digraphs.Tournament(m)
return g.transitive_closure().subgraph_search(twochains, induced = True) is None

Nathann

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:14

Attachment: poset_functionality_14099_er.patch.gz

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

Changed author from Eric rowland, Alejandro Morales to Eric Rowland, Alejandro Morales

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:15

As #14122 just got reviewed, could you rebase your patch on top of it and use the Tournament cnstructor ? :-)

By the way, could your merge your two functions into only one ? It would accept a list as a parameter and run the "real code" if the list has length one, and call itself on each pair of the list when it has a larger length.

Nathann

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:16

What is the advantage of merging them into one? The disadvantage is a decrease in clarity.

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:17

What is the advantage of merging them into one? The disadvantage is a decrease in clarity.

One advantage is that you only have to write documentation and doctests for only one function instead of two, and documentation and doctests are actually the largest part of this patch. Then 50% of the actual code is only there to deal with the input and not actual computations. If you want to improve clarity, you can also dramatically decrease the tests you do at the beginning. Something like that :

def i_take_lists_of_pairs_or_just_a_pair_as_input(u,v=None):
    if v is None:
        return all(i_take_lists_of_pairs_or_just_a_pair_as_input(*x) for x in u)
    u,v = Integer(u), Integer(v)
    if u<0 or v<0:
        raise ValueError("Both elements of a pair must be positive integers") 
    # No I can do some actual computations
    print u,v
    return True

But honestly I think that only one method (taking a pair as input) should exist in Sage. To me the rest is "personal code".

Nathann

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:18

14122 has been merged into beta1.

Nathann

[fchapoton]

comment:20

instruction for the bot:

Apply poset_functionality_14099-er.patch

[fchapoton]

Description changed:

--- 
+++ 
@@ -1 +1,4 @@
-This is a test for finite posets to check if the do not have induced posets each of which isomorphic to two disjoint chains. Example 3+1 free, 2+2 free.
+This is a test for finite posets to check if they do not have induced posets  isomorphic to the union of two disjoint chains. Example 3+1 free, 2+2 free.
+
+Apply: 
+* [attachment: poset_functionality_14099-er.patch](https://github.com/sagemath/sage-prod/files/10657108/poset_functionality_14099-er.patch.gz)

[fchapoton]

comment:21

I also think that only the method taking a pair of numbers should go into sage.

You can now use the TransitiveTournament method, it works for n=1.

[fchapoton]

Dependencies: #14536

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:23

This is now rebased on #14536.

Awaiting review.

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

Changed keywords from posets, days45 to posets, days45, days49

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:25

Helloooooooooooooooooooooo !!

• Could you add your new method to the index of methods at the top of the file ? This way we have a nice list of everything that the posets can do http://www.sagemath.org/doc/reference/combinat/sage/combinat/posets/posets.html

• Could you rewrite your reference so that Sphinx understands it ? You have an example there :

  sage: Graph.average_distance??

• Why do you build the digraph from the relations, instead of using the Hasse Diagram ? I thought that this is how it was done in an earlier version of the patch O_o

Nathann

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:26

I have no idea what Sphinx is.

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:27

It is the piece of code that generates the html documentation of Sage.

sage -b && sage -docbuild reference/combinat html

Nathann

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:30

O_o

Nathann

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:31

Could you please rewrite the reference to [Stanley, Enumerative Combinatorics Volume 1, 2nd edition] in the usual Sphinx format, or explain why you think that it is a bad idea ?

Nathann

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:32

Anything else?

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:33

O_O

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:34

Okayyyyyyyyyyyyyyyyyyyyyyyy I see ......

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:36

No way.

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:37

I am currently writing an additional patch for this bibliographical reference.

I will upload it shortly.

Nathann

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:39

Here is a patch that rewrites the reference to Stanley's book as we usually do in Sphinx, and as it is done in Graph.average_distance?? and other functions.

I don't know why you set the ticket to positive_review twice even though the guy who was doing the review (=me) never said that he agreed with it. I don't get why Chris set it to positive review while there was a comment which had not been fixed either. Anyway, I think that this small attachment: trac_14099-rev.patch patch has to be added to this ticket too, unless you can explain why you prefer it to be left out.

Nathann

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

Description changed:

--- 
+++ 
@@ -2,3 +2,4 @@

 Apply: 
 * [attachment: poset_functionality_14099-er.patch](https://github.com/sagemath/sage-prod/files/10657108/poset_functionality_14099-er.patch.gz)
+* [attachment: trac_14099-rev.patch](https://github.com/sagemath/sage-prod/files/10657109/trac_14099-rev.patch.gz)

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

comment:40

I set the ticket to positive review, once, after I asked if there was anything else and you said "Okayyyyyyyyyyyyyyyyyyyyyyyy I see ......" which I took to mean that everything was okay.

Chris set it to positive review after I addressed your latest comment, which was not specific at all. I improved the typesetting of the variables. Apparently this was not what you had in mind, but you hadn't said that.

You need to be more clear in your communication with people who are learning how to contribute. At this point I am tired of making point modifications every time you think something should be changed, so please just finish the patch up yourself as you want it.

[jdemeyer]

comment:42

The patches need proper commit messages (use hg qrefresh -e for that).

[a7405f10-fcc8-4a59-b616-0c8c9dab053b]

Attachment: poset_functionality_14099-er.patch.gz

[jdemeyer]

comment:43

The reviewer patch needs a proper commit message, use hg qrefresh -e for this.

[6bdad4c1-1e26-4f2f-a442-a01a2292c181]

comment:44

Attachment: trac_14099-rev.patch.gz

[jdemeyer]

Merged: sage-5.11.rc0