#6588 closed enhancement (fixed)
Categories for root systems and many misc improvements
Reported by:  Nicolas M. Thiéry  Owned by:  Mike Hansen 

Priority:  major  Milestone:  sage5.0 
Component:  combinatorics  Keywords:  root systems, categories 
Cc:  Sage Combinat CC user, mshimo@…  Merged in:  sage5.0.beta10 
Authors:  Nicolas M. Thiéry  Reviewers:  Anne Schilling, Mark Shimozono, Florent Hivert 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  #10817  Stopgaps: 
Description (last modified by )
New Features:
 Extended weight lattice/space for affine types  coxeter_matrix and coxeter_diagram (for crystallographic types)  Embeddings of the root lattice/space in root lattice realizations  Embeddings of the weight lattice/space in weight lattice realizations  Partial conversion from the root space to the root lattice  Iterator for any Coxeter group, and weak order ideals thereof  Facade option for weak_poset's and bruhat_poset's of finite coxeter groups  weak_poset's of finite coxeter groups are lattices; added link weak_lattice > weak_poset (courtesy of Christian Stump)  Enumerated set for grassmannian elements, and is_grasmmannian test  Added method register_as_conversion to morphisms
Refactoring:
 RootLatticeRealization and WeightLatticeRealization are now categories  Use abstract_methods where appropriate  Cleanup of an_element  to_coroot_lattice_morphism: renamed to to_coroot_lattice_space, and moved from RootLatticeRealization to RootSpace (it does not make sense for, e.g., the weight lattice).  Moved the implementation of associated_coroot in root_lattice_realization to root_space where it belongs
Doc, tests and bug fixes:
 100% doctests on sage.combinat.root_system, except for weyl_group and weyl_characters  100% doctests on CoxeterGroups  Add most modules in sage.combinat.root_system to the reference manual and improved the quickref  Misc doc fixes in the above modules  Add minimal documentation to AffineWeylGroups  Added systematic tests for associated_coroot  Fixed further missing features revealed by this test:  reducible Cartan types were not seen as finite/simply laced/crystallographic as appropriate  symmetrizer needed to be generalized to reducible cartan types (including D2)  the relabelling for reducible cartan types was broken for affine types  More tests for scalar products  Added systematic consistency check between the simple_root and simple_roots methods  same thing for fundamental_weight and fundamental_weights  Fixed a SL vs GL glitch when obtaining simple roots for reducible cartan types revealed by the above  Fixed imports (extraneous ones, and a missing one reported by M. Shimozono and others)
Attachments (3)
Change History (51)
comment:1 Changed 11 years ago by
Authors:  → Nicolas M. Thiéry 

Cc:  mshimo@… added 
Dependencies:  → #10963, #10817 
Description:  modified (diff) 
Report Upstream:  → N/A 
comment:2 Changed 11 years ago by
Description:  modified (diff) 

Summary:  Root systems: categorification → Categories for root systems 
comment:3 Changed 11 years ago by
Description:  modified (diff) 

comment:4 followup: 6 Changed 11 years ago by
Status:  new → needs_review 

comment:5 Changed 11 years ago by
Beside, I commuted it up the SageCombinat queue. In principle, there should not be other dependencies than the listed ones.
comment:6 Changed 11 years ago by
Replying to nthiery:
The updated patch adds many missing doctests, and improves coxeter diagrams. It's probably close to completion, up to fixing some potentially failing doctests.
There are indeed a couple minor ones:
http://sage.math.washington.edu/home/nthiery/trac_6588categoriesroot_systemsnt.patchtestlog
Note: that's with the following patches applied on 5.0.beta5:
trac_12476lattice_join_matrix_speedupfh.patch trac_9469categorymembership_without_argumentsnt.patch trac_10603union_enumset_elconstr_fixfh.patch trac_12528_free_moduleoptimizent.patch trac_10817generalized_associahedracs.patch trac_10817generalized_associahedrareviewnt.patch trac_12078see_alsofh.patch trac_9128intersphinx_python_databasefh.patch trac_9128sphinx_links_allfh.patch trac_12527fraction_fieldis_exact_optimizationnt.patch trac_12510nonzero_equal_consistencyfh.patch trac_12536linear_extensionsas.patch trac_12518enumerated_set_from_iteratorvd.patch trac_11880.patch trac_11880graph_classesreviewnt.patch trac_7980multiplerealizationsnt.patch trac_7980multiplerealizationsreviewnt.patch trac_6588categoriesroot_systemsnt.patch
Off for skiing :)
comment:7 Changed 11 years ago by
I fixed the doctests failures due to this patch. Most of them where due to the fact that affine weyl groups are iterable now, which gives a nicer an_element on any free module thereupon.
The other doctests failures showing up in the log are related to #12518 (or to the ISGCI patch, but that's because I did not upload the appropriate database on my test server).
The review can start!
comment:8 Changed 11 years ago by
Dependencies:  #10963, #10817 → #10817 

Description:  modified (diff) 
Patch updated after a bug report by Mark; see end of patch description
comment:9 Changed 11 years ago by
Upon popular demand, extended weight lattice/spaces are now implemented.
comment:10 followup: 11 Changed 11 years ago by
Reviewers:  → Anne Schilling, Mark Shimozono 

Hi Nicolas,
Impressive patch! Thanks for working on this. Here are some first comments:
 In /sage/categories/affine_weyl_groups.py there is a new import
from sage.categories.infinite_enumerated_sets import InfiniteEnumeratedSets? which is not used. Please either remove this line or add InfiniteEnumeratedSets? in the class (which is probably preferable).
 In /sage/categories/affine_weyl_groups.py please add a
TestSuite(s).run()
doctest.
 I am not sure this is related to the patch, but there are some
strange methods in /sage/categories/coxeter_groups.py without doctests:
@abstract_method(optional = True) def has_right_descent(self, i):
""" Returns whether i is a right descent of self.
# EXAMPLES:: # # sage:
"""
def has_left_descent(self, i):
""" Returns whether
i
is a left descent of self.
This default implementation uses that a left descent of
w
is a right descent ofw^{1}
. """ return (~self).has_right_descent(i)
Should has_left_descent also be an abstract_method? Or is that implicit through has_right_descent?
 Why is the cateogry RootLatticeRealization? in
/sage/combinat/root_system/root_lattice_realization.py here and not in categories (if it is a category as specified in the docstring)?
The same question holds for WeightLatticeRealizations?(Category_over_base_ring) in /sage/combinat/root_system/weight_lattice_realization.py.
 When using the extended weight lattice, the list of fundamental weights
does not include \delta
. On the other hand it is possible to input
\delta
into the method fundamental_weight. This seems a little inconsistent.
sage: Q = RootSystem(['A', 3, 1]).weight_lattice(extended = True); Q Extended weight lattice of the Root system of type ['A', 3, 1] sage: Q.fundamental_weights() Finite family {0: Lambda[0], 1: Lambda[1], 2: Lambda[2], 3: Lambda[3]} sage: Q.fundamental_weight('delta') delta
Also, I posted a first reviewer's patch on sagecombinat with mostly just trivial changes. Please fold it if you are satisfied.
Thanks!
Anne
comment:11 followup: 12 Changed 11 years ago by
Thanks much Anne for your detailed review!
Replying to aschilling:
 In /sage/categories/affine_weyl_groups.py there is a new import
from sage.categories.infinite_enumerated_sets import InfiniteEnumeratedSets? which is not used. Please either remove this line or add InfiniteEnumeratedSets? in the class (which is probably preferable).
Good point. Done!
 In /sage/categories/affine_weyl_groups.py please add a
TestSuite(s).run()
doctest.
Done.
 I am not sure this is related to the patch, but there are some
strange methods in /sage/categories/coxeter_groups.py without doctests:
CoxeterGroups? is now 100% doctested.
Should has_left_descent also be an abstract_method? Or is that implicit through has_right_descent?
There is a default implementation for this method, so it's not abstract. Indeed, it would be nice to track such dependencies in the documentation; but we don't have the infrastructure for that.
 Why is the cateogry RootLatticeRealization? in
/sage/combinat/root_system/root_lattice_realization.py here and not in categories (if it is a category as specified in the docstring)?
The same question holds for WeightLatticeRealizations?(Category_over_base_ring) in /sage/combinat/root_system/weight_lattice_realization.py.
 When using the extended weight lattice, the list of fundamental weights
does not include
\delta
. On the other hand it is possible to input\delta
into the method fundamental_weight. This seems a little inconsistent.sage: Q = RootSystem(['A', 3, 1]).weight_lattice(extended = True); Q Extended weight lattice of the Root system of type ['A', 3, 1] sage: Q.fundamental_weights() Finite family {0: Lambda[0], 1: Lambda[1], 2: Lambda[2], 3: Lambda[3]} sage: Q.fundamental_weight('delta') delta
More on those after lunch!
Also, I posted a first reviewer's patch on sagecombinat with mostly just trivial changes. Please fold it if you are satisfied.
Folded in! There is a new reviewer's patch on the patch server.
Cheers,
Nicolas
comment:12 followup: 16 Changed 11 years ago by
 Why is the cateogry RootLatticeRealization? in
/sage/combinat/root_system/root_lattice_realization.py here and not in categories (if it is a category as specified in the docstring)?
The same question holds for WeightLatticeRealizations?(Category_over_base_ring) in /sage/combinat/root_system/weight_lattice_realization.py.
Yeah, we had a similar discussion for the crystal categories and the categories for Symmetric Functions and friends. And there is a non trivial borderline to set.
On one hand, we have general categories (like Groups, Algebras, ...) which are likely to get used in several Sage modules. Also, they name a wellknown area of mathematics; so it's natural to import them by default in the interpreter, if not just as an entry point.
On the other hand we have categories that are rather specific to a Sage module, if not just implementation details. So it's good to keep them in this module, in particular to be as close as close as possible from the other sources of that module.
CoxeterGroups? clearly fits the first case (most mathematicians have heard of them; this category is used by WeylGroup? (in sage.combinat.root_system) and by SymmetricGroup? (in sage.groups).
The categories for symmetric functions are basically implementation details and fits in the second case. Anyway, SymmetricFunctions? is already a good entry point.
Crystals are only implemented in sage.combinat.crystals (so far). But this names an area of mathematics. So this fits a bit more the first case.
RootLatticeRealizations? and WeightLatticeRealizations? seem rather specific. So this fits a bit more the second case.
 When using the extended weight lattice, the list of fundamental weights
does not include
\delta
. On the other hand it is possible to input\delta
into the method fundamental_weight. This seems a little inconsistent.
Yeah, this abuse is documented in "fundamental_weight". Basically, we need a function that describes the embedding of the basis elements of the (extended) weight lattice. That's almost what fundamental_weight does, and I did not have a good alternative name. So I abused fundamental_weight to do just a bit more that its natural job. Anyway, we currently only have a single implementation of WeightLatticeRealizations? in the affine case, so it's very localized, and easy to change in the future if we come up with a good name (unless you have one!).
Let me know if you are ok with the above and with my reviewer's patch. If yes, I'll fold the patches, reindent properly root_lattice_realization.py and weight_lattice_realization.py, and add a 's' at the end of those files (I haven't done those yet to keep the diff meaningful). Then, the patch will be good to go.
For the record, all tests pass on Sage 5.0 beta6, with the following patches applied:
trac_10817generalized_associahedracs.patch trac_12645fix_rst_markupsk.patch trac_9128intersphinx_python_databasefh.patch trac_9128sphinx_links_allfh.patch trac_9128MANIFESTfh.patch trac_12527fraction_fieldis_exact_optimizationnt.patch trac_12510nonzero_equal_consistencyfh.patch trac_12536linear_extensionsas.patch element_compare_consistencyfh.patch trac_11880isgciall_in_onenc.patch trac_11880isgcimorereviewnt.patch trac_7980multiplerealizationsnt.patch trac_7980multiplerealizationsreviewnt.patch trac_6588categoriesroot_systemsnt.patch trac_6588categoriesroot_systemsreviewnt.patch
Cheers,
Nicolas
comment:13 followup: 14 Changed 11 years ago by
Reviewers:  Anne Schilling, Mark Shimozono → Anne Schilling, Mark Shimozono, PatchBot 

Status:  needs_review → needs_work 
Fails doctests on Sage 5.0.beta7:
sage t long devel/sagemain/sage/combinat/root_system/weight_space.py ********************************************************************** File "/storage/masiao/sage5.0.beta7/devel/sagemain/sage/combinat/root_system/weight_space.py", line 126: sage: Q.null_root(0)[index] Exception raised: Traceback (most recent call last): File "/storage/masiao/sage5.0.beta7/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/storage/masiao/sage5.0.beta7/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/storage/masiao/sage5.0.beta7/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_1[17]>", line 1, in <module> Q.null_root(Integer(0))[index]###line 126: sage: Q.null_root(0)[index] TypeError: __call__() takes exactly 0 positional arguments (1 given) **********************************************************************
(The patchbot found this, but I also reproduced it by hand)
comment:14 Changed 11 years ago by
Status:  needs_work → needs_review 

Replying to davidloeffler:
Fails doctests on Sage 5.0.beta7:
sage t long devel/sagemain/sage/combinat/root_system/weight_space.py ********************************************************************** File "/storage/masiao/sage5.0.beta7/devel/sagemain/sage/combinat/root_system/weight_space.py", line 126: sage: Q.null_root(0)[index] Exception raised: Traceback (most recent call last): File "/storage/masiao/sage5.0.beta7/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/storage/masiao/sage5.0.beta7/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/storage/masiao/sage5.0.beta7/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_1[17]>", line 1, in <module> Q.null_root(Integer(0))[index]###line 126: sage: Q.null_root(0)[index] TypeError: __call__() takes exactly 0 positional arguments (1 given) **********************************************************************(The patchbot found this, but I also reproduced it by hand)
Sorry, already fixed on the SageCombinat queue where the review is occuring. I'll delete the patch here for the moment so that others don't waste time reviewing an outdated version.
Thanks though for the report!
comment:15 Changed 11 years ago by
Description:  modified (diff) 

comment:16 followup: 17 Changed 11 years ago by
Replying to nthiery:
Thanks much Anne for your detailed review!
You are welcome. I hope you will return the favor for trac_12536linear_extensionsas.patch!
 Why is the cateogry RootLatticeRealization? in
/sage/combinat/root_system/root_lattice_realization.py here and not in categories (if it is a category as specified in the docstring)?
The same question holds for WeightLatticeRealizations?(Category_over_base_ring) in /sage/combinat/root_system/weight_lattice_realization.py.
Yeah, we had a similar discussion for the crystal categories and the categories for Symmetric Functions and friends. And there is a non trivial borderline to set.
On one hand, we have general categories (like Groups, Algebras, ...) which are likely to get used in several Sage modules. Also, they name a wellknown area of mathematics; so it's natural to import them by default in the interpreter, if not just as an entry point.
On the other hand we have categories that are rather specific to a Sage module, if not just implementation details. So it's good to keep them in this module, in particular to be as close as close as possible from the other sources of that module.
CoxeterGroups? clearly fits the first case (most mathematicians have heard of them; this category is used by WeylGroup? (in sage.combinat.root_system) and by SymmetricGroup? (in sage.groups).
The categories for symmetric functions are basically implementation details and fits in the second case. Anyway, SymmetricFunctions? is already a good entry point.
Crystals are only implemented in sage.combinat.crystals (so far). But this names an area of mathematics. So this fits a bit more the first case.
No, there is a lot of crystal code in /sage/categories: crystals, finite_crystals, highest_weight_crystals, ....
 When using the extended weight lattice, the list of fundamental weights
does not include
\delta
. On the other hand it is possible to input\delta
into the method fundamental_weight. This seems a little inconsistent.Yeah, this abuse is documented in "fundamental_weight". Basically, we need a function that describes the embedding of the basis elements of the (extended) weight lattice. That's almost what fundamental_weight does, and I did not have a good alternative name. So I abused fundamental_weight to do just a bit more that its natural job. Anyway, we currently only have a single implementation of WeightLatticeRealizations? in the affine case, so it's very localized, and easy to change in the future if we come up with a good name (unless you have one!).
Why can't fundamental_weights when "extended" is set also include delta
? This would at least fit with the notational abuse of fundamental_weight.
Let me know if you are ok with the above and with my reviewer's patch. If yes, I'll fold the patches, reindent properly root_lattice_realization.py and weight_lattice_realization.py, and add a 's' at the end of those files (I haven't done those yet to keep the diff meaningful). Then, the patch will be good to go.
I also wrote another very small reviewer's patch that you can fold in.
For the record, all tests pass on Sage 5.0 beta6, with the following patches applied:
trac_10817generalized_associahedracs.patch trac_12645fix_rst_markupsk.patch trac_9128intersphinx_python_databasefh.patch trac_9128sphinx_links_allfh.patch trac_9128MANIFESTfh.patch trac_12527fraction_fieldis_exact_optimizationnt.patch trac_12510nonzero_equal_consistencyfh.patch trac_12536linear_extensionsas.patch element_compare_consistencyfh.patch trac_11880isgciall_in_onenc.patch trac_11880isgcimorereviewnt.patch trac_7980multiplerealizationsnt.patch trac_7980multiplerealizationsreviewnt.patch trac_6588categoriesroot_systemsnt.patch trac_6588categoriesroot_systemsreviewnt.patch
That's good!
Cheers,
Anne
comment:17 followup: 18 Changed 11 years ago by
Replying to aschilling:
You are welcome. I hope you will return the favor for trac_12536linear_extensionsas.patch!
I sure will, soon!
Crystals are only implemented in sage.combinat.crystals (so far). But this names an area of mathematics. So this fits a bit more the first case.
No, there is a lot of crystal code in /sage/categories: crystals, finite_crystals, highest_weight_crystals, ....
I meant: the crystals themselves (CrystalOfLetter?, KR, ...) are all in implemented in sage.combinat.crystals.
Why can't fundamental_weights when "extended" is set also include
delta
? This would at least fit with the notational abuse of fundamental_weight.
Because we want to do things like:
sage: x in self.fundamental_weights()
sage: all(L.some_property() for L in self.fundamental_weights())
So adding delta would change the semantic of fundamental_weights. Whereas the current abuse of fundamental_weight can't break any code.
I also wrote another very small reviewer's patch that you can fold in.
I don't see it on the queue; did you push?
Thanks!
Nicolas
comment:18 followup: 21 Changed 11 years ago by
Replying to nthiery:
Replying to aschilling:
You are welcome. I hope you will return the favor for trac_12536linear_extensionsas.patch!
I sure will, soon!
Thanks.
Crystals are only implemented in sage.combinat.crystals (so far). But this names an area of mathematics. So this fits a bit more the first case.
No, there is a lot of crystal code in /sage/categories: crystals, finite_crystals, highest_weight_crystals, ....
I meant: the crystals themselves (CrystalOfLetter?, KR, ...) are all in implemented in sage.combinat.crystals.
But these are not categories, they are classes. In your case, a cateogory is in /sage/combinat/root_systems/
Why can't fundamental_weights when "extended" is set also include
delta
? This would at least fit with the notational abuse of fundamental_weight.Because we want to do things like:
sage: x in self.fundamental_weights()
sage: all(L.some_property() for L in self.fundamental_weights())
So adding delta would change the semantic of fundamental_weights. Whereas the current abuse of fundamental_weight can't break any code.
How about self.fundamental_weights(extended = True) with default extended = False?
I also wrote another very small reviewer's patch that you can fold in.
I don't see it on the queue; did you push?
Sorry, I forgot to push. It should be there now.
Best,
Anne
Changed 11 years ago by
Attachment:  trac_6588categoriesroot_systemsntbeforereindentation.patch added 

This contains the userreadable diff. Do not apply!
comment:19 Changed 11 years ago by
Description:  modified (diff) 

comment:20 Changed 11 years ago by
Description:  modified (diff) 

Summary:  Categories for root systems → Categories for root systems and many misc improvements 
comment:21 followup: 22 Changed 11 years ago by
Replying to aschilling:
But these are not categories, they are classes. In your case, a cateogory is in /sage/combinat/root_systems/
Yes: unlike for crystals, and like for symmetric functions (well for NCSF actually; the categorification of symmetric functions is yet to be done), I want to keep the categories close to the classes. As I said, it's borderline, but it feels better to me this way.
How about self.fundamental_weights(extended = True) with default extended = False?
Well, since it's an abuse, and one that we might want to get rid of, I'd rather not abuse yet another function. Unless you have a natural use case for this notation? (for the extended weight lattice, that's just self.basis()).
Sorry, I forgot to push.
Given how many times I played that gag to you, I am not going to throw the first rock :)
It should be there now.
Yes, thanks! I have folded together all patches and posted them here. I also did the reindentation and renaming *_lattice_realization > *_lattice_realizations. On my side, the patch is good to go.
Cheers,
Nicolas
comment:22 followup: 23 Changed 11 years ago by
Status:  needs_review → positive_review 

How about self.fundamental_weights(extended = True) with default extended = False?
Well, since it's an abuse, and one that we might want to get rid of, I'd rather not abuse yet another function. Unless you have a natural use case for this notation? (for the extended weight lattice, that's just self.basis()).
Ok, since this is in self.basis() I am satisfied.
Yes, thanks! I have folded together all patches and posted them here. I also did the reindentation and renaming *_lattice_realization > *_lattice_realizations. On my side, the patch is good to go.
Ok, thanks! Unless I hear otherwise from Mark, I'll set a positive review.
Cheers,
Anne
comment:23 Changed 11 years ago by
Good morning Anne!
Replying to aschilling:
Ok, thanks! Unless I hear otherwise from Mark, I'll set a positive review.
Yippee!
A bunch of patches went into 5.0.beta8 this morning. Maybe this one will follow :)
I am working on the review of #12536.
Cheers,
Nicolas
comment:24 followup: 25 Changed 11 years ago by
Reviewers:  Anne Schilling, Mark Shimozono, PatchBot → Anne Schilling, Mark Shimozono 

Let's not anthropomorphize the PatchBot? :)
comment:25 followup: 28 Changed 11 years ago by
Hi Nicolas,
When working on a review for #12667, I noticed some documentation problems with this patch in root_lattice_realizations. I wrote a review patch on the sagecombinat queue. If you are happy, please fold, repost here and reset the positive review.
Best,
Anne
comment:26 Changed 11 years ago by
Status:  positive_review → needs_work 

comment:27 Changed 11 years ago by
Description:  modified (diff) 

Status:  needs_work → positive_review 
comment:28 Changed 11 years ago by
Replying to aschilling:
When working on a review for #12667, I noticed some documentation problems with this patch in root_lattice_realizations. I wrote a review patch on the sagecombinat queue. If you are happy, please fold, repost here and reset the positive review.
Good catches. Thanks! Folded.
I allowed myself to further add the weight lattice realizations file to the reference manual, and to replace {\vee} by \vee. I also set back \mathbb{N} to \NN, since \NN will be added very soon to the standard Sage macros.
comment:29 Changed 11 years ago by
While I was at it, I added all the other missing files in root_system.rst (type_*, ...), and fixed the documentation typos revealled by the compilation. Florent reviewed them. New patch posted.
comment:30 Changed 11 years ago by
Description:  modified (diff) 

comment:31 Changed 11 years ago by
Status:  positive_review → needs_work 

comment:32 Changed 11 years ago by
Status:  needs_work → needs_review 

I guess the last patch still needs review?
comment:33 followup: 34 Changed 11 years ago by
Reviewers:  Anne Schilling, Mark Shimozono → Anne Schilling, Mark Shimozono, Florent Hivert 

Status:  needs_review → positive_review 
No, I had uploaded the wrong file. The new one is the one that has been checked by Florent.
Now it's a very final positive review. Sorry Jeroen for the mess; I hope you were not in the process of applying / testing it!
comment:34 followup: 35 Changed 11 years ago by
I am not sure why you replaced \mathcal{N} by \NN since this currently gives a latex error when compiling the documentation:
preparing documents... done
WARNING: display latex u'
NN': latex exited with error:lattice_realizations
[stderr]
[stdout] This is pdfTeXk, Version 3.14159261.40.9 (Web2C 7.5.7)
%&line parsing enabled.
entering extended mode (./math.tex LaTeX2e <2005/12/01> Babel <v3.8l> and hyphenation patterns for english, usenglishmax, dumylang, noh yphenation, germanx20080618, ngermanx20080618, ancientgreek, ibycus, ar abic, basque, bulgarian, catalan, pinyin, coptic, croatian, czech, danish, dutc h, esperanto, estonian, farsi, finnish, french, galician, german, ngerman, mono greek, greek, hungarian, icelandic, indonesian, interlingua, irish, italian, la tin, lithuanian, mongolian, mongolian2a, bokmal, nynorsk, polish, portuguese, r omanian, russian, sanskrit, serbian, slovak, slovenian, spanish, swedish, turki sh, ukenglish, ukrainian, uppersorbian, welsh, loaded. (/usr/local/texlive/2008/texmfdist/tex/latex/base/article.cls Document Class: article 2005/09/16 v1.4f Standard LaTeX document class (/usr/local/texlive/2008/texmfdist/tex/latex/base/size12.clo)) (/usr/local/texlive/2008/texmfdist/tex/latex/base/inputenc.sty (/usr/local/texlive/2008/texmfdist/tex/latex/base/utf8.def (/usr/local/texlive/2008/texmfdist/tex/latex/base/t1enc.dfu) (/usr/local/texlive/2008/texmfdist/tex/latex/base/ot1enc.dfu) (/usr/local/texlive/2008/texmfdist/tex/latex/base/omsenc.dfu))) (/usr/local/texlive/2008/texmfdist/tex/latex/amsmath/amsmath.sty For additional information on amsmath, use the `?' option. (/usr/local/texlive/2008/texmfdist/tex/latex/amsmath/amstext.sty (/usr/local/texlive/2008/texmfdist/tex/latex/amsmath/amsgen.sty)) (/usr/local/texlive/2008/texmfdist/tex/latex/amsmath/amsbsy.sty) (/usr/local/texlive/2008/texmfdist/tex/latex/amsmath/amsopn.sty)) (/usr/local/texlive/2008/texmfdist/tex/latex/amscls/amsthm.sty) (/usr/local/texlive/2008/texmfdist/tex/latex/amsfonts/amssymb.sty (/usr/local/texlive/2008/texmfdist/tex/latex/amsfonts/amsfonts.sty)) (/usr/local/texlive/2008/texmfdist/tex/latex/tools/bm.sty) (./math.aux) (/usr/local/texlive/2008/texmfdist/tex/latex/amsfonts/umsa.fd) (/usr/local/texlive/2008/texmfdist/tex/latex/amsfonts/umsb.fd) ! Undefined control sequence. <recently read> \NN l.29 $\NN
$
[1] (./math.aux) ) (see the transcript file for additional information) Output written on math.dvi (1 page, 152 bytes). Transcript written on math.log.
Is it allowed to leave such a failure?
Thanks,
Anne
comment:35 followup: 36 Changed 11 years ago by
Hi Anne:
Replying to aschilling:
I am not sure why you replaced \mathcal{N} by \NN since this currently gives a latex error when compiling the documentation:
As I said: I also set back \mathbb{N} to \NN, since \NN will be added very soon to the standard Sage macros
There are lots of other failures like this in the documentation; it's best to progressively get rid of them, but here it's just temporary, so that's good enough. Let's not waste time on having to update this later.
Cheers,
Nicolas
comment:36 followup: 37 Changed 11 years ago by
Hi Nicolas,
Well, one of my patches recently did not get merged until all these failures were rectified. So I thought it was compulsory for all documentation to work.
Best,
Anne
comment:37 followup: 44 Changed 11 years ago by
Replying to aschilling:
Well, one of my patches recently did not get merged until all these failures were rectified. So I thought it was compulsory for all documentation to work.
There is at least one other \NN in th sources. Anyway, let's not waste even more time discussing it. Here is the diff with the patch I am about to upload(sorry Jeroen):
+ with the coroots are all in `\NN`, which is not ++ with the coroots are all non negative integers, which is not
I left the positive review; feel free to change.
comment:38 Changed 11 years ago by
Status:  positive_review → needs_work 

This obviously fails on 32bit systems:
sage t "devel/sage/sage/combinat/root_system/dynkin_diagram.py" ********************************************************************** File "/export/home/buildbot/sage5.0.beta9/devel/sage/sage/combinat/root_system/dynkin_diagram.py", line 167: sage: hash(CartanType(['A',3]).dynkin_diagram()) # indirect doctest Expected: 286820813001824631 Got: 2127980169 **********************************************************************
comment:39 Changed 11 years ago by
Status:  needs_work → needs_review 

Thanks Jeroen for the report. This should be fixed with the upcoming patch. The diff is:

sage/combinat/root_system/dynkin_diagram.py
diff git a/sage/combinat/root_system/dynkin_diagram.py b/sage/combinat/root_system/dynkin_diagram.py
a b class DynkinDiagram_class(DiGraph, Carta 165 165 EXAMPLES:: 166 166 167 167 sage: hash(CartanType(['A',3]).dynkin_diagram()) # indirect doctest 168 286820813001824631 169 168 286820813001824631 # 64bit 169 2127980169 # 32bit 170 170 """ 171 171 # Should assert for immutability! 172 172
comment:40 Changed 11 years ago by
Status:  needs_review → needs_work 

Work issues:  → documentation 
There is a problem with the documentation. Doing "make doc" after applying this patch gives:
reading sources... [ 92%] sage/combinat/root_system/weight_lattice_realizations reading sources... [ 95%] sage/combinat/root_system/weight_space reading sources... [ 97%] sage/combinat/root_system/weyl_group reading sources... [100%] sage/structure/parent <autodoc>:0: ERROR: Inconsistent literal block quoting. looking for nowoutdated files... none found pickling environment... done checking consistency... done preparing documents... done writing output... [ 2%] categories writing output... [ 4%] combinat/algebra writing output... [ 6%] combinat/index writing output... [ 8%] combinat/root_systems writing output... [ 10%] index
Unfortunately, there is no indication which file caused the problem.
comment:41 Changed 11 years ago by
Ok, it was in root_lattice_realizations. At this occasion, Anne and myself spotted a couple more doc typos. I am about to upload an updated patch. See :attachment:latest_change.patch for the changes since last version.
Changed 11 years ago by
Attachment:  latest_change.patch added 

comment:42 Changed 11 years ago by
Status:  needs_work → positive_review 

comment:43 Changed 11 years ago by
Work issues:  documentation 

comment:44 Changed 11 years ago by
There is at least one other \NN in th sources. Anyway, let's not waste even more time discussing it. Here is the diff with the patch I am about to upload(sorry Jeroen):
I finished my patch on \NN: this is #12717
Changed 11 years ago by
Attachment:  trac_6588categoriesroot_systemsnt.patch added 

Apply this one (identical to the other, but renames and reindents two files; thus unreadable)
comment:45 Changed 11 years ago by
Rebased the last patch to sage5.0.beta9 (there was some trivial fuzz).
comment:46 Changed 11 years ago by
Merged in:  → sage5.0.beta10 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:48 Changed 11 years ago by
The examples for __cmp__
in sage/combinat/root_system/type_dual.py
are bad. #12793 fixes them, please review.
The updated patch adds many missing doctests, and improves coxeter diagrams. It's probably close to completion, up to fixing some potentially failing doctests.