Skip to content
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

Add is_induced_subposet #15875

Closed
sagetrac-csar mannequin opened this issue Feb 27, 2014 · 88 comments
Closed

Add is_induced_subposet #15875

sagetrac-csar mannequin opened this issue Feb 27, 2014 · 88 comments

Comments

@sagetrac-csar
Copy link
Mannequin

sagetrac-csar mannequin commented Feb 27, 2014

Add is_induced_subposet() method to Posets to check if a subposet is an (induced) subposet of another.

CC: @kevindilks @nathanncohen @tscrim

Component: combinatorics

Keywords: posets

Author: Jori Mäntysalo

Branch/Commit: c7db822

Reviewer: Travis Scrimshaw, Nathann Cohen

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

@sagetrac-csar sagetrac-csar mannequin added this to the sage-6.2 milestone Feb 27, 2014
@sagetrac-csar
Copy link
Mannequin Author

sagetrac-csar mannequin commented Feb 27, 2014

Branch: u/csar/ticket/15875

@sagetrac-csar
Copy link
Mannequin Author

sagetrac-csar mannequin commented Mar 3, 2014

comment:2

I'm not positive this works. I think I've stumbled across an example where you have equality of the results of hasse_diagram(), but not of _hasse_diagram.

@sagetrac-csar
Copy link
Mannequin Author

sagetrac-csar mannequin commented Mar 3, 2014

Commit: 7ecc981

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 2, 2014

comment:5

You don't have to say Poset(DiGraph({1:[2,3],2:[4],3:[4]})), just Poset({1:[2,3],2:[4],3:[4]}).

See also #16892, it relates to this one. I think that you can look the code of it for you.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Jul 28, 2015

comment:6

How are other "containing"-functions made? For a poset A to be a subposet of B it should be that for every cover relation u, v in A there is u' < v' in B, where u == u' (and v == v'). How to define ==, for example if u is int(42)and u' is Integer(42)?

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 9, 2015

comment:8

At least

g1 = DiGraph({13:[14]})
g2 = DiGraph({13r:[14r]})
g1.is_subgraph(g2), g2.is_subgraph(g1)

outputs (True, True). Is there examples of sub* function with different behaviour? If not, then also is_subposet() should be like is_subgraph(). CC to ncohen for this question.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 9, 2015

comment:9

Since a long time, graphs have been converting all 'integer' labels into 'int'. That was long before I came. I expect that the reason is that you "pay for labels" when you deal with graphs, and that was probably a way to avoid that.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 9, 2015

comment:10

Replying to @nathanncohen:

Since a long time, graphs have been converting all 'integer' labels into 'int'. That was long before I came. I expect that the reason is that you "pay for labels" when you deal with graphs, and that was probably a way to avoid that.

This was an answer to different question... (on sage-devel). To clarify, here is another example of is_sub*-function:

r1=2.0-1.0
r2=3.0-1.0
i1=1
i2=2
{i1, i2}.issubset({r1, r2})

This also outputs True. Hence it seems that Poset({r1:[r2]}).is_subposet(Poset({i1:[i2]})) should also return True. But I would like someone to confirm this.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 9, 2015

comment:11

That would be because Integer(5) == int(5) returns True, then. Would you expect them to be handled any differently by posets?

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 10, 2015

comment:12

Replying to @nathanncohen:

That would be because Integer(5) == int(5) returns True, then. Would you expect them to be handled any differently by posets?

Well, no. So, in principle P.is_subposet(Q) can be implemented like set([frozenset(x) for x in P.relations()]).issubset(set([frozenset(x) for x in Q.relations()])). Or course there is better ways to do it in reality.

Is there any graph function already to wrap for this? Something about reachability in digraphs?

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 10, 2015

comment:13

Is there any graph function already to wrap for this? Something about reachability in digraphs?

Hmmmm... I don't think so. If there was, you could probably expect the Poset (Directed Acyclic Graph) case to be much faster to solve, so it's really Poset code in the end.

Nathann

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 10, 2015

comment:14

Would implementing P.is_subposet(Q) as P.hasse_diagram().is_subgraph(Q.hasse_diagram().transitive_closure()) be too slow? I am not sure if I can make faster version by manually calling digraph functions in a loop. But of course it is much faster to get False if some element of P is not an element of Q at all by first checking for that. But will transitive_closure()) eat memory?

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 10, 2015

comment:15

Replying to @jm58660:

Would implementing P.is_subposet(Q) as P.hasse_diagram().is_subgraph(Q.hasse_diagram().transitive_closure()) be too slow?

"depends". It means a copy of Q's graph even if P is only one vertex. In other cases it will be optimal.

I am not sure if I can make faster version by manually calling digraph functions in a loop. But of course it is much faster to get False if some element of P is not an element of Q at all by first checking for that. But will transitive_closure()) eat memory?

It will eat some memory indeed, but it will be freed then. Ideally, we would need a dense representation for Q's transitive closure and a sparse representation of P. We can iterate over all edges of P, and check every time that it is an edge of Q's transitive closure.

The most costly part of it will probably be the vertex labelling.

Nathann

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 11, 2015

Changed branch from u/csar/ticket/15875 to u/jmantysalo/is_subposet

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 11, 2015

comment:17

Done this as an oneline-wrapper. I think that faster versions are easier to do when we already have a docstring and maybe see how this will be used.

This is intentionally left out from index of functions, as I am still waiting for #18534 to get accepted (or rejected).


New commits:

29ab8f2Added a function is_subposet().

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 11, 2015

Changed commit from 7ecc981 to 29ab8f2

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 11, 2015

Author: Jori Mäntysalo

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 11, 2015

Changed keywords from none to posets

@jm58660 jm58660 mannequin added s: needs review and removed p: minor / 4 labels Aug 11, 2015
@jm58660 jm58660 mannequin removed this from the sage-6.4 milestone Aug 11, 2015
@jm58660 jm58660 mannequin added the s: positive review label Sep 28, 2015
@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 1, 2015

comment:60

Does not work:

P = Poset({1:[2]})
Q1 = Poset({1:[]})
Q2 = LatticePoset({1:[]})
Q1.is_induced_subposet(P), Q2.is_induced_subposet(P)

outputs (True, False).

@jm58660 jm58660 mannequin added s: needs work and removed s: positive review labels Oct 1, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

28a3cc3Merge branch 't/15875/is_subposet' into ind_subposet. Correction of

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Changed commit from 9119980 to 28a3cc3

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 27, 2015

comment:62

Resolved a conflict, made to work with lattices also. Should be OK now.

@jm58660

This comment has been minimized.

@jm58660 jm58660 mannequin added s: needs review and removed s: needs work labels Oct 27, 2015
@jm58660 jm58660 mannequin modified the milestones: sage-6.9, sage-6.10 Oct 27, 2015
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 27, 2015

comment:63

Hello Jori,

I do not understand the point of the line if not hasattr(other, 'hasse_diagram'):. Given that you call subposet and .hasse_diagram on other anyway your code will raise an exception whenever those methods are not available.

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

fb6299eRemoved a test for having attribute _hasse_diagram.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Changed commit from 28a3cc3 to fb6299e

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 27, 2015

comment:65

Replying to @nathanncohen:

I do not understand the point of the line if not hasattr(other, 'hasse_diagram'):. Given that you call subposet and .hasse_diagram on other anyway your code will raise an exception whenever those methods are not available.

As you wish, changed that.

Now P.is_induced_subposet('junk') will say AttributeError: 'str' object has no attribute '_is_facade'. A slightly confusing to a user, but should not be that hard to track the error.

(Quite many Sage functions give strange errors from wrong input... At least this won't be first one.)

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 27, 2015

comment:66

Hello Jori,

As you seem to care about this message, I pused a commit at public/15875 that will raise a slightly more meaningful error message when the input is bad. Add it if you like it, ignore it if you do not.

The probability of users giving this function bad input would be much lower if the function you add had an 'INPUT' section explaining what exactly it expects.

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Changed commit from fb6299e to 4228920

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

4228920Better error reporting.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 27, 2015

comment:68

Replying to @nathanncohen:

As you seem to care about this message, I pused a commit at public/15875 that will raise a slightly more meaningful error message when the input is bad. Add it if you like it, ignore it if you do not.

Thanks. Added that.

The probability of users giving this function bad input would be much lower if the function you add had an 'INPUT' section explaining what exactly it expects.

Hmm... Maybe I should go through functions and add INPUT sections.

This patch contains a non-relating line that corrects an indentation bug in dilworth_decomposition(). OK to add here?

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 27, 2015

comment:69

Yo,

Hmm... Maybe I should go through functions and add INPUT sections.

Could you add one at least in this function that you add here?

This patch contains a non-relating line that corrects an indentation bug in dilworth_decomposition(). OK to add here?

No prob.

About the commit: [please do not change anything as it is not important], but notice that instead of adding my commit on your branch (with cherry-pick, for instance) you apparently applied the changes manually (or did something else that I cannot guess), which has as a side-effect that you became the committer+author of my changes.

Please don't change anything as it is not important, I am just giving you a side-effect of the procedure you followed to use my commit.

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Changed commit from 4228920 to c7db822

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

c7db822Added an INPUT section to docstring.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 27, 2015

comment:71

Replying to @nathanncohen:

Hmm... Maybe I should go through functions and add INPUT sections.

Could you add one at least in this function that you add here?

Done.

About the commit: [please do not change anything as it is not important], but notice that instead of adding my commit on your branch (with cherry-pick, for instance) you apparently applied the changes manually (or did something else that I cannot guess), which has as a side-effect that you became the committer+author of my changes.

True. I'll try to remember that in the future.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 27, 2015

comment:72

Thanks,

Nathann

@vbraun
Copy link
Member

vbraun commented Oct 28, 2015

Changed branch from u/jmantysalo/is_subposet to c7db822

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants