Opened 3 years ago

Closed 2 years ago

#22139 closed enhancement (fixed)

Poset: Add certificate to is_[meet|join]_semilattice

Reported by: jmantysalo Owned by:
Priority: major Milestone: sage-8.0
Component: combinatorics Keywords:
Cc: tscrim Merged in:
Authors: Jori Mäntysalo Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 5364368 (Commits) Commit: 53643683480fb5d46e0d29db21631b04f299646b
Dependencies: Stopgaps:

Description

This patch will add a certificate-option to functions checking finite poset is a semilattice.

I also suggest removing deprecated meet_matrix() and join_matrix() for posets (they are not at lattices).

Change History (10)

comment:1 Changed 3 years ago by jmantysalo

  • Branch set to u/jmantysalo/semilattice-cert

comment:2 follow-up: Changed 3 years ago by jmantysalo

  • Cc tscrim added
  • Commit set to db04bf55c6cb339ecc8f1db8ca87d748b827b0ec
  • Status changed from new to needs_info

This is kind of needs-review, but if this is OK, I must made the "dual changes" meet <-> join.

There was also a slight problem. Code like

try:
    make_some_matrix()
except ValueError:
    return False
return True

might -- at least in theory -- return False when there is some other error at Matrix creation (out of memory?).


New commits:

db04bf5Certificate for is_meet_semilattice.

comment:3 in reply to: ↑ 2 Changed 2 years ago by jmantysalo

Replying to jmantysalo:

This is kind of needs-review, but if this is OK, I must made the "dual changes" meet <-> join.

Travis, want still another "add certificate" -ticket?

comment:4 Changed 2 years ago by git

  • Commit changed from db04bf55c6cb339ecc8f1db8ca87d748b827b0ec to 2a09fc71043a0c71a7351e3d9de493a216136c87

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

2a09fc7Merge branch 'develop' into t/22139/semilattice-cert

comment:5 Changed 2 years ago by jmantysalo

  • Milestone changed from sage-7.6 to sage-8.0

comment:6 follow-up: Changed 2 years ago by tscrim

I think this should be okay. Just implement the dual version, and we will be good to go.

comment:7 Changed 2 years ago by git

  • Commit changed from 2a09fc71043a0c71a7351e3d9de493a216136c87 to 53643683480fb5d46e0d29db21631b04f299646b

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

5364368Add certificate to is_join_semilattice.

comment:8 in reply to: ↑ 6 Changed 2 years ago by jmantysalo

  • Status changed from needs_info to needs_review

Replying to tscrim:

I think this should be okay. Just implement the dual version, and we will be good to go.

Done that.

comment:9 Changed 2 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM. Thanks.

comment:10 Changed 2 years ago by vbraun

  • Branch changed from u/jmantysalo/semilattice-cert to 53643683480fb5d46e0d29db21631b04f299646b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.