Opened 7 years ago

Closed 6 years ago

#10065 closed enhancement (duplicate)

removed bug and new methods in Poset

Reported by: stumpc5 Owned by: sage-combinat
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: combinatorics Keywords: poset, days30
Cc: saliola@…, chapoton@… Merged in:
Authors: Reviewers: Franco Saliola
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Poset.dual contained a bug:

P = Poset(([1,2],[[1,2]]))
Q = P.dual()
Q.show()
... AssertionError: `self` should be an acyclic graph

The patch contains the solution for this bug as well as new methods is_lattice is_isomorphic is_selfdual panyushev_complement panyushev_orbits

Attachments (1)

trac_10065_smaller_poset_improvements-cs.patch (6.2 KB) - added by stumpc5 6 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by stumpc5

  • Status changed from new to needs_review

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

The procedure is_isomorphic has two problems :

  • one doctest is failing (see the report of the build bot). This is related to a deprecated use of size instead of cardinality, somewhere. Maybe we should correct this deprecated use first.
  • it fails on Lattices
    P=Posets.PentagonPoset()
    P.is_isomorphic(P)
    ValueError: The input is not a finite poset.
    

Maybe this can be corrected by having a method is_isomorphic for Lattices which is inherited from Posets.

comment:3 Changed 6 years ago by chapoton


One more thing to be corrected :

The doc of '''panyushev_orbits''' is badly formatted. It does not display correctly in the notebook.

comment:4 in reply to: ↑ 2 Changed 6 years ago by stumpc5

  • Cc chapoton@… added

Replying to chapoton:

  • one doctest is failing (see the report of the build bot). This is related to a deprecated use of size instead of cardinality, somewhere. Maybe we should correct this deprecated use first.

I only updated the doctest - the size/cardinality is in generic_graph.py. I am in the process to make changes to the is_isomorphic method in there anyway. I will make this little change in that other patch.

  • it fails on Lattices
    P=Posets.PentagonPoset()
    P.is_isomorphic(P)
    ValueError: The input is not a finite poset.
    

Maybe this can be corrected by having a method is_isomorphic for Lattices which is inherited from Posets.

This was just a minor mistake in the is_isomorphic method for Posets. I updated it.

The doc of panyushev_orbits is badly formatted. It does not display correctly in the notebook.

Done...

comment:5 Changed 6 years ago by stumpc5

I uploaded a changed version of the patch containing a faster way to compute the Panyushev complement. Beside this, I didn't do any changes.

comment:6 Changed 6 years ago by chapoton

This last change makes some tests fail. See the shortlog by the buildbot.

comment:7 Changed 6 years ago by stumpc5

Sorry Fred, I often forget to rebuild before making the test... if I didn't do more mistakes, it should work now.

@Franco: I also had to edit the patches

trac_6641-poset_antichains_backtracker.patch addon_poset_antichains_backtracker-cs.patch

(and uploaded them to the combinat queue) for ticket #6641. Could you probably fold them and add them to your ticket? Thanks!

comment:8 Changed 6 years ago by saliola

  • Milestone set to sage-duplicate/invalid/wontfix

These issues are addressed in #10998.

comment:9 Changed 6 years ago by saliola

  • Keywords days30 added

comment:10 Changed 6 years ago by mariah

  • Status changed from needs_review to positive_review

If this is a sage-duplicate, then I believe this ticket can be closed.

comment:11 Changed 6 years ago by jdemeyer

  • Authors Christian Stump deleted
  • Resolution set to duplicate
  • Reviewers set to Franco Saliola
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.