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

LatticePoset: certificate for is_pseudocomplemented() #21505

Closed
jm58660 mannequin opened this issue Sep 16, 2016 · 17 comments
Closed

LatticePoset: certificate for is_pseudocomplemented() #21505

jm58660 mannequin opened this issue Sep 16, 2016 · 17 comments

Comments

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Sep 16, 2016

Add an option to get an element without the pseudocomplement. (Compare to .is_complemented(certificate=True).

Component: combinatorics

Author: Jori Mäntysalo

Branch/Commit: 69e0b1e

Reviewer: Travis Scrimshaw

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

@jm58660 jm58660 mannequin added this to the sage-7.4 milestone Sep 16, 2016
@jm58660 jm58660 mannequin added c: combinatorics labels Sep 16, 2016
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 16, 2016

Branch: u/jmantysalo/pseudocomp-certificate

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 16, 2016

comment:2

Should it be "an element without the pseudocomplement" or "an element without a pseudocomplement", as an element can have at most one pseudocomplemet?


New commits:

4785ba6Add certificate-option to is_pseudocomplemented().

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 16, 2016

Commit: 4785ba6

@jm58660 jm58660 mannequin added the s: needs review label Sep 16, 2016
@tscrim
Copy link
Collaborator

tscrim commented Sep 16, 2016

comment:3

I would say "a pseudocomplement" both in case one does not exist and so it makes it seem less like there is one pseudocomplement for all lattices (which you could get around by saying "the pseudocomplement of the lattice").

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 16, 2016

Changed commit from 4785ba6 to 1ed0863

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 16, 2016

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

1ed0863'the' or 'a'.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 16, 2016

comment:5

Replying to @tscrim:

I would say "a pseudocomplement" - -

OK. Thanks.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 22, 2016

comment:6

Travis, can you also review this? Should be trivial.

(Not that important, but kind of nice to have unified format of is_* -functions when possbile.)

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2016

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2016

comment:7

Is there a reason why you have one as 3 lines if-return and the other as a 1 line return foo if bar else True? I think it would be good to be consistent. Once addressed, you can set a positive review on my behalf.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2016

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

69e0b1eReformatting return-if-else.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2016

Changed commit from 1ed0863 to 69e0b1e

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 22, 2016

comment:9

Replying to @tscrim:

Is there a reason why you have one as 3 lines if-return and the other as a 1 line return foo if bar else True? I think it would be good to be consistent. Once addressed, you can set a positive review on my behalf.

Done that. Thanks.

@vbraun
Copy link
Member

vbraun commented Sep 24, 2016

comment:10
sage -t --long src/sage/combinat/posets/lattices.py
**********************************************************************
File "src/sage/combinat/posets/lattices.py", line 1272, in sage.combinat.posets.lattices.FiniteLatticePoset.is_pseudocomplemented
Failed example:
    L.is_pseudocomplemented(certificate=True)
Exception raised:
    Traceback (most recent call last):
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.posets.lattices.FiniteLatticePoset.is_pseudocomplemented[4]>", line 1, in <module>
        L.is_pseudocomplemented(certificate=True)
    TypeError: is_pseudocomplemented() got an unexpected keyword argument 'certificate'
**********************************************************************
1 item had failures:
   1 of   7 in sage.combinat.posets.lattices.FiniteLatticePoset.is_pseudocomplemented
    [347 tests, 1 failure, 1.10 s]

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Sep 25, 2016

comment:11

Replying to @vbraun:

        L.is_pseudocomplemented(certificate=True)
    TypeError: is_pseudocomplemented() got an unexpected keyword argument 'certificate'

??? I can not get that error message. It seems that the code has been tested before compiling.

@tscrim
Copy link
Collaborator

tscrim commented Sep 25, 2016

comment:12

I agree with Jori's assessment.

@vbraun
Copy link
Member

vbraun commented Oct 6, 2016

Changed branch from u/jmantysalo/pseudocomp-certificate to 69e0b1e

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