#6588 closed enhancement (fixed)
Categories for root systems and many misc improvements
Reported by:  nthiery  Owned by:  mhansen 

Priority:  major  Milestone:  sage5.0 
Component:  combinatorics  Keywords:  root systems, categories 
Cc:  sagecombinat, 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 6 years ago by
 Cc mshimo@… added
 Dependencies set to #10963, #10817
 Description modified (diff)
 Report Upstream set to N/A
comment:2 Changed 6 years ago by
 Description modified (diff)
 Summary changed from Root systems: categorification to Categories for root systems
comment:3 Changed 6 years ago by
 Description modified (diff)
comment:4 followup: ↓ 6 Changed 6 years ago by
 Status changed from new to needs_review
comment:5 Changed 6 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 in reply to: ↑ 4 Changed 6 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 6 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 6 years ago by
 Dependencies changed from #10963, #10817 to #10817
 Description modified (diff)
Patch updated after a bug report by Mark; see end of patch description
comment:9 Changed 6 years ago by
Upon popular demand, extended weight lattice/spaces are now implemented.
comment:10 followup: ↓ 11 Changed 6 years ago by
 Reviewers set to 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 in reply to: ↑ 10 ; followup: ↓ 12 Changed 6 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 in reply to: ↑ 11 ; followup: ↓ 16 Changed 6 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 6 years ago by
 Reviewers changed from Anne Schilling, Mark Shimozono to Anne Schilling, Mark Shimozono, PatchBot
 Status changed from needs_review to 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 in reply to: ↑ 13 Changed 6 years ago by
 Status changed from needs_work to 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 6 years ago by
 Description modified (diff)
comment:16 in reply to: ↑ 12 ; followup: ↓ 17 Changed 6 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 in reply to: ↑ 16 ; followup: ↓ 18 Changed 6 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 in reply to: ↑ 17 ; followup: ↓ 21 Changed 6 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
comment:19 Changed 6 years ago by
 Description modified (diff)
comment:20 Changed 6 years ago by
 Description modified (diff)
 Summary changed from Categories for root systems to Categories for root systems and many misc improvements
comment:21 in reply to: ↑ 18 ; followup: ↓ 22 Changed 6 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 in reply to: ↑ 21 ; followup: ↓ 23 Changed 6 years ago by
 Status changed from needs_review to 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 in reply to: ↑ 22 Changed 6 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 6 years ago by
 Reviewers changed from Anne Schilling, Mark Shimozono, PatchBot to Anne Schilling, Mark Shimozono
Let's not anthropomorphize the PatchBot? :)
comment:25 in reply to: ↑ 24 ; followup: ↓ 28 Changed 6 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 6 years ago by
 Status changed from positive_review to needs_work
comment:27 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to positive_review
comment:28 in reply to: ↑ 25 Changed 6 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 6 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 6 years ago by
 Description modified (diff)
comment:31 Changed 6 years ago by
 Status changed from positive_review to needs_work
comment:32 Changed 6 years ago by
 Status changed from needs_work to needs_review
I guess the last patch still needs review?
comment:33 followup: ↓ 34 Changed 6 years ago by
 Reviewers changed from Anne Schilling, Mark Shimozono to Anne Schilling, Mark Shimozono, Florent Hivert
 Status changed from needs_review to 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 in reply to: ↑ 33 ; followup: ↓ 35 Changed 6 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 in reply to: ↑ 34 ; followup: ↓ 36 Changed 6 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 in reply to: ↑ 35 ; followup: ↓ 37 Changed 6 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 in reply to: ↑ 36 ; followup: ↓ 44 Changed 6 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 6 years ago by
 Status changed from positive_review to 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 6 years ago by
 Status changed from needs_work to 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 6 years ago by
 Status changed from needs_review to needs_work
 Work issues set to 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 6 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 6 years ago by
comment:42 Changed 6 years ago by
 Status changed from needs_work to positive_review
comment:43 Changed 6 years ago by
 Work issues documentation deleted
comment:44 in reply to: ↑ 37 Changed 6 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 6 years ago by
Apply this one (identical to the other, but renames and reindents two files; thus unreadable)
comment:45 Changed 6 years ago by
Rebased the last patch to sage5.0.beta9 (there was some trivial fuzz).
comment:46 Changed 6 years ago by
 Merged in set to sage5.0.beta10
 Resolution set to fixed
 Status changed from positive_review to closed
comment:47 Changed 6 years ago by
Yippee! Thanks all for your help getting this in!
comment:48 Changed 5 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.