New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding Method for testing avoidance in posets #14099
Comments
comment:1
Could you be interested in running this method on the transitive closure of your poset ? Nathann |
comment:3
apply: poset_functionality_14099_er.patch |
comment:4
Is the transitive closure + subgraph_search fast enough to make it worthwhile? |
comment:5
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 |
comment:6
apply: poset_functionality_14099_er.patch |
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:
Using induced=False results in the poset Poset({0 : [1, 2], 2 : [3]}) which is (3+1)-free containing (3+1):
|
comment:8
you definitely need to use
Nathann |
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.
|
comment:10
On the other hand, for larger posets using the graph search does much better:
|
Changed keywords from posets to posets, days45 |
Changed author from chrisjamesberg, rowland, ahmorales to Eric rowland, Alejandro Morales |
Changed reviewer from saliola to Chris Berg |
comment:13
I just created ticket #14122 because of this ticket. With that ticket, the following code
can be replaced by
Nathann |
comment:14
Attachment: poset_functionality_14099_er.patch.gz |
Changed author from Eric rowland, Alejandro Morales to Eric Rowland, Alejandro Morales |
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 |
comment:16
What is the advantage of merging them into one? The disadvantage is a decrease in clarity. |
comment:17
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 :
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 |
comment:18
#14122 has been merged into beta1. Nathann |
comment:20
instruction for the bot: Apply poset_functionality_14099-er.patch |
comment:33
|
comment:34
Okayyyyyyyyyyyyyyyyyyyyyyyy I see ...... |
comment:36
No way. |
comment:37
I am currently writing an additional patch for this bibliographical reference. I will upload it shortly. Nathann |
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 I don't know why you set the ticket to Nathann |
This comment has been minimized.
This comment has been minimized.
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. |
comment:42
The patches need proper commit messages (use |
Attachment: poset_functionality_14099-er.patch.gz |
comment:43
The reviewer patch needs a proper commit message, use |
comment:44
Attachment: trac_14099-rev.patch.gz |
Merged: sage-5.11.rc0 |
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:
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
The text was updated successfully, but these errors were encountered: