Context Navigation

← Previous Ticket
Next Ticket →

#14655 closed defect (fixed)

Dynkin diagrams does not copy like digraph

Reported by:	tscrim	Owned by:	sage-combinat
Priority:	major	Milestone:	sage-5.11
Component:	combinatorics	Keywords:	dynkin diagram copy
Cc:	sage-combinat, nthiery, mshimo, aschilling	Merged in:	sage-5.11.beta1
Authors:	Travis Scrimshaw	Reviewers:	Mark Shimozono
Report Upstream:	N/A	Work issues:
Branch:		Commit:
Dependencies:		Stopgaps:

Status badges

Description

Initializing:

sage: d = DynkinDiagram(['B',3,1])
sage: g = DiGraph()
sage: for x in d.vertices(): g.add_vertex(x)
sage: for y in d.edges(): g.add_edge(y)
sage: cd = copy(d)
sage: cg = copy(g)
sage: cd is d
False
sage: cd == d and cg == g
True

Now the correct behavior using DiGraph:

sage: g.vertices()
[0, 1, 2, 3]
sage: cg.vertices()
[0, 1, 2, 3]
sage: cg.add_vertex(4)
sage: g.vertices()
[0, 1, 2, 3]
sage: cg.vertices()
[0, 1, 2, 3, 4]

The error with DynkinDiagram:

sage: d.vertices()
[0, 1, 2, 3]
sage: cd.vertices()
[0, 1, 2, 3]
sage: cd.add_vertex(4)       
sage: cd.vertices()
[0, 1, 2, 3, 4]
sage: d.vertices()
[0, 1, 2, 3, 4]

This causes problems with automorphism_group(edge_labels=True) noticed by Mark Shimozono.

Attachments (1)

trac_14655-dynkin_copy-ts.patch (4.3 KB) - added by tscrim 8 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by tscrim

Cc aschilling added

comment:2 Changed 8 years ago by tscrim

Authors set to Travis Scrimshaw
Status changed from new to needs_review

Fixed by making DynkinDiagram_class.__init__() handle the input from GenericGraph.__copy__() and do some type-checking.

comment:3 Changed 8 years ago by mshimo

Reviewers set to mshimo
Status changed from needs_review to needs_work

Thanks, Travis, for fixing this. A few small things to deal with:

Please fix the docstring for :func:DynkinDiagram: the real issue for the Cartan matrix convention is transposing or not; in everyone's conventions the arrow points from a *longer* root to a *shorter* one.

I ran the following tests and got some errors.

sage: d = DynkinDiagram(['A',3,1])
sage: TestSuite(d).run()
Failure in _test_not_implemented_methods:
Traceback (click to the left of this block for traceback)
...
The following tests failed: _test_not_implemented_methods

I'm not sure I understand why, but :class:DynkinDiagram_class inherits from :class:CartanType_abstract and therefore is obligated to supply its abstract methods. It appears not to fulfill its promise for :meth:is_finite, :meth:is_affine, and :meth:is_irreducible.

Otherwise it looks great!

comment:4 Changed 8 years ago by tscrim

Reviewers changed from mshimo to Mark Shimozono
Status changed from needs_work to needs_review

I believe it is so that you can easily pass Dynkin diagrams as Cartan types (without using the coercion system). Anyways, new version of the patch which implements the additional methods and makes the docstring fix.