Opened 5 years ago
Closed 5 years ago
#19196 closed enhancement (fixed)
LatticePoset: add is_dismantlable()
Reported by:  jmantysalo  Owned by:  

Priority:  major  Milestone:  sage6.10 
Component:  combinatorics  Keywords:  
Cc:  kdilks, chapoton  Merged in:  
Authors:  Jori Mäntysalo, Frédéric Chapoton  Reviewers:  Frédéric Chapoton, Jori Mäntysalo 
Report Upstream:  N/A  Work issues:  
Branch:  6ac2e21 (Commits)  Commit:  6ac2e219b653b380468205fff51a43a6c168dda4 
Dependencies:  Stopgaps: 
Change History (16)
comment:1 Changed 5 years ago by
 Branch set to u/jmantysalo/latticeposet__add_is_dismantlable__
comment:2 Changed 5 years ago by
 Commit set to f9bc731924fd81f33095b3d18cc9920ef9e55dec
 Status changed from new to needs_review
comment:3 Changed 5 years ago by
 Cc ncohen added
 Status changed from needs_review to needs_work
Everything seems to be already done in graphs... I CC Nathann because I just noticed at #19118 the function
g.subgraph(vertices=g.cores(k=2)[1])
So propably there is some very easy way to make this.
comment:4 Changed 5 years ago by
 Cc ncohen removed
 Status changed from needs_work to needs_review
Forget. Sorry for the noice.
(Even if .cores()
does kind of similar thing, it is not actually useful here. So back to original algorithm and back to needs_review.)
comment:5 Changed 5 years ago by
 Status changed from needs_review to needs_work
Hmm... If a lattice is not dismantlable, then it contains a "crown". Should certificate
return it?
comment:6 Changed 5 years ago by
 Commit changed from f9bc731924fd81f33095b3d18cc9920ef9e55dec to b9e73715c8c22f283f068c9675b924faab180ee4
Branch pushed to git repo; I updated commit sha1. New commits:
b9e7371  Correction of a bug, added negative certificate.

comment:7 Changed 5 years ago by
 Cc aschilling added
 Status changed from needs_work to needs_review
Anne selected as a random victim for possible review.
comment:9 Changed 5 years ago by
 Status changed from needs_review to needs_work
Needs merge, but I'll wait #19123 to be accepted (or rejected) first.
comment:10 Changed 5 years ago by
 Commit changed from b9e73715c8c22f283f068c9675b924faab180ee4 to 00d07b68259a8bbb46d27d7acfced014159648d0
Branch pushed to git repo; I updated commit sha1. New commits:
00d07b6  Merged with latest beta

comment:11 Changed 5 years ago by
 Commit changed from 00d07b68259a8bbb46d27d7acfced014159648d0 to badb4f77b66756a69b3dda76b11ccb548745dbc5
Branch pushed to git repo; I updated commit sha1. New commits:
badb4f7  Indentation in docstring.

comment:12 Changed 5 years ago by
 Status changed from needs_work to needs_review
As #19123 is stalled, I merged this. Maybe I got at least some code into next release.
comment:13 Changed 5 years ago by
 Cc chapoton added; aschilling removed
 Milestone changed from sagewishlist to sage6.10
Frédéric? Someone? This is quite easy one.
Would be nice if at 20160107 I could say that this will be at Sage version 6.10. I will mention dismantlable lattices then in a little talk.
comment:14 followup: ↓ 15 Changed 5 years ago by
 Branch changed from u/jmantysalo/latticeposet__add_is_dismantlable__ to public/19196
 Commit changed from badb4f77b66756a69b3dda76b11ccb548745dbc5 to 6ac2e219b653b380468205fff51a43a6c168dda4
ok, good to go. I have put the code into pep8 standard.
If you approve my small changes, you can set to positive review.
By the way, please avoid the final underscores in your branch names, as it makes rather hard to copy/paste. Better to keep the branch names short and simple, I would say.
New commits:
8d92005  Merge branch 'u/jmantysalo/latticeposet__add_is_dismantlable__' into 6.10.b5

6ac2e21  trac #19196 pep8 code standard

comment:15 in reply to: ↑ 14 Changed 5 years ago by
 Reviewers set to Frédéric Chapoton, Jori Mäntysalo
 Status changed from needs_review to positive_review
Replying to chapoton:
ok, good to go. I have put the code into pep8 standard.
If you approve my small changes, you can set to positive review.
I did, and also put your name to reviewer field.
By the way, please avoid the final underscores in your branch names, as it makes rather hard to copy/paste. Better to keep the branch names short and simple, I would say.
That this was generated name from git trac
, I guess. I will try to use better names from now on. And maybe some day I will remember to use PEP 8 standard...
Thanks  for this, and for many other reviews.
comment:16 Changed 5 years ago by
 Branch changed from public/19196 to 6ac2e219b653b380468205fff51a43a6c168dda4
 Resolution set to fixed
 Status changed from positive_review to closed
Here it is. This seems to be quite fast, but I am sure that
H.is_connected()
can be changed to some digraph function checking ifupper
is reachable fromlower
. Optimization can be made later, so I mark this as needs_review.New commits:
Added function is_dismantlable().