Update Coxeter Group documentation

[thecaligarmo]

Update the documentation in combinat/root_system/plot with visual examples of the examples found in the current documentation.

Additionally, add categories/complex_reflection_or_generalized_coxeter_groups to the documentation as it seems to be missing altogether.

CC: @nthiery @tscrim @jm58660 @fchapoton

Component: documentation

Keywords: sagedays86

Author: Aram Dermenjian

Branch: c083d5c

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/22853

[thecaligarmo]

Branch: u/aram.dermenjian/update_coxeter_group_documentation

[thecaligarmo]

New commits:

f93fd14	Add documentation to root system plots
ead92ff	Add file into doc and fix minor error
693bbfe	remove bad looking example

[thecaligarmo]

Reviewer: nthiery,tscrim

[thecaligarmo]

Commit: 693bbfe

[thecaligarmo]

Changed reviewer from nthiery,tscrim to Nicolas M. Thiéry,Travis Scrimshaw

[thecaligarmo]

Changed author from aram.dermenjian to Aram Dermenjian

[thecaligarmo]

Changed reviewer from Nicolas M. Thiéry,Travis Scrimshaw to none

[tscrim]

comment:7

Definitely something that should be done. However, perhaps we should change the width to 300 px as this is the most common size for plots (there is 1 that is 400 px and some that are 200 px)?

[thecaligarmo]

comment:8

I specifically chose 500 because the 300px images were difficult to see. I tried 400 as well before settling on 500.

An idea, which I couldn't find, I wanted to see if there's a way to limit the height by cropping instead of stretching. It looks like sphinx_plot always returns a square image, so when adding

:height: 200 px

or something we get a shrunk image which still has pointless whitespace. I couldn't find anything in the documentation about cropping, but if you do know of a way to crop, it'd be nice as some of the images definitely take up a lot of vertical room and would be nice to shrink them.

Alternatively, I can reduce the size to 300px as is more standard with the knowledge that some images may be difficult to see. (Note the main difficulty in seeing the images was the words/symbols on the images, so I can potentially reduce the size as well if we don't care about being able to read labels on the images)

[tscrim]

comment:9

What is the dpi (or resolution) on your laptop? It might be hard to read because that is very high.

Unfortunately I don't know enough about the .. PLOT:: to answer your questions. I do know we do not want the Sage doc size to grow too much, so I feel that we should make them 300 px. However, the html has its own spacing rules, so the whitespace may come from that too.

[nthiery]

comment:10

Just two comments for now:

• Aram presented the updated documentation, and it made me jump up and down. So much better.
• I can't wait until sphinx would actually use vector graphics rather than bitmaps. This would resolve all those resolution tweaking issues.

[sagetrac-git]

Changed commit from 693bbfe to 72baca1

[sagetrac-git]

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

72baca1

Alter widths to be the standard 300

[thecaligarmo]

comment:12

My dpi is fairly high, so I've gone ahead and reduced all the graphs to 300px. Can you look them over and see if they actually look good in the smaller size?

@nthiery - sphinx seems to be lacking quite a lot when it comes to images. Hopefully they'll get to updating it in the near future so we can make plot look even more awesome.

---

New commits:

72baca1

Alter widths to be the standard 300

[tscrim]

comment:13

This looks good. Thank you for your work on this.

[tscrim]

Reviewer: Travis Scrimshaw

[vbraun]

comment:14

[dochtml] Error building the documentation.
[dochtml] Traceback (most recent call last):
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main
[dochtml]     "__main__", fname, loader, pkg_name)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/runpy.py", line 72, in _run_code
[dochtml]     exec code in run_globals
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
[dochtml]     main()
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1640, in main
[dochtml]     builder()
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 316, in _wrapper
[dochtml]     getattr(get_builder(document), 'inventory')(*args, **kwds)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 510, in _wrapper
[dochtml]     build_many(build_ref_doc, L)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 266, in build_many
[dochtml]     results.append(target(arg))
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 70, in build_ref_doc
[dochtml]     getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 720, in _wrapper
[dochtml]     getattr(DocBuilder, build_type)(self, *args, **kwds)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 104, in f
[dochtml]     runsphinx()
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 215, in runsphinx
[dochtml]     raise exception
[dochtml] OSError: [combinat ] /home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/combinat/root_system/plot.py:docstring of sage.combinat.root_system.plot.PlotOptions.color:10: WARNING: Exception occurred in plotting plot-15
[dochtml] 
Makefile:988: recipe for target 'doc-html-mathjax' failed
make[1]: *** [doc-html-mathjax] Error 1
make[1]: Leaving directory '/home/buildbot/slave/sage_git/build/build/make'

[tscrim]

comment:15

I made some trivial reviewer changes that need a quick check.

However, I am (still) unable to reproduce the docbuild failure Volker is getting.

---

New commits:

46fde7d	Merge branch 'u/aram.dermenjian/update_coxeter_group_documentation' of git://trac.sagemath.org/sage into u/tscrim/add_root_system_plots-22853
0f087b2	Some trivial reviewer changes.

[tscrim]

Changed commit from 72baca1 to 0f087b2

[thecaligarmo]

Changed branch from u/tscrim/add_root_system_plots-22853 to u/aram.dermenjian/add_root_system_plots-22853

[thecaligarmo]

comment:34

I've gone ahead and just removed that plot. I tried cleaning my doc and making again, and as usual, everything worked well. I guess we'll need to run it through the patchbots to see if it works this time around.

---

New commits:

9a5a9ed

Remove 3d plot with color options

[thecaligarmo]

Changed commit from dfbc5bb to 9a5a9ed

[tscrim]

comment:36

I didn't have too much time to investigate this, but this is probably the best way forward.

IMO, it is better to send it back to the buildbots and see if Volker comes back again saying it fails since we need a specific patchbot to look at it.

[vbraun]

comment:37

[dochtml] Error building the documentation.
[dochtml] Traceback (most recent call last):
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main
[dochtml]     "__main__", fname, loader, pkg_name)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/runpy.py", line 72, in _run_code
[dochtml]     exec code in run_globals
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
[dochtml]     main()
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1642, in main
[dochtml]     builder()
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 316, in _wrapper
[dochtml]     getattr(get_builder(document), 'inventory')(*args, **kwds)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 510, in _wrapper
[dochtml]     build_many(build_ref_doc, L)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 266, in build_many
[dochtml]     results.append(target(arg))
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 70, in build_ref_doc
[dochtml]     getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 720, in _wrapper
[dochtml]     getattr(DocBuilder, build_type)(self, *args, **kwds)
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 104, in f
[dochtml]     runsphinx()
[dochtml]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 215, in runsphinx
[dochtml]     raise exception
[dochtml] OSError: [combinat ] /home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/combinat/root_system/plot.py:docstring of sage.combinat.root_system.plot.PlotOptions.color:21: WARNING: Exception occurred in plotting plot-15
[dochtml] 
make[1]: *** [doc-html-mathjax] Error 1

[vbraun]

comment:38

I'd appreciate it if you don't send me half-assed tickets on the vague hope that they work, I do have a lot of tickets to juggle and can't spend that much time on individual issues.

[tscrim]

comment:39

We would love to send you something that works, but the problem nobody other than you and one patchbot has been able to reproduce that failure. I tried asking for more information and your help (comment:17, comment:27) so we didn't have to do this back and forth. The error message that you keep posting does not contain any useful information. This seemed like the best lead from the patchbot report (comment:30), which actually had some more useful information and only seemed to apply to the now removed plot.

Really what is going on is we are trying to find our way in the dark. Is there some way you can give us access to the machine where it does fail so we can experiment? If you're unwilling to help and unless somebody else has this failure, then we have to set it to positive review and give it to you to test it.

[vbraun]

comment:40

The error is still the TypeError: reduce() of empty sequence with no initial value.

• Add an initial value to the reduction
• Add a unit test that your 3d graphics object contains at least some of the objects you think it does

Jan Groenewald can get you access to the buildbots

[tscrim]

comment:41

Thank you. I will be e-mail Jan about the buildbot access and get to this in a few days (I will be traveling tomorrow from the US to South Korea). Which buildbot(s) does the failure occur on? Feel free to send me an e-mail directly about it.

[tscrim]

Changed branch from u/aram.dermenjian/add_root_system_plots-22853 to u/tscrim/add_root_system_plots-22853

[tscrim]

Changed commit from 9a5a9ed to c083d5c

[tscrim]

comment:42

The problem is this test/plot:

    sage: L = RootSystem(["A",3,1]).ambient_space()
    sage: L.plot(reflection_hyperplanes=False, bounding_box=85/100) # long time
    Graphics3d Object

.. PLOT::
    :width: 300 px

    L = RootSystem(["A",3,1]).ambient_space()
    sphinx_plot(L.plot(reflection_hyperplanes=False, bounding_box=85/100))

I also now know why it fails and can reproduce it in a Sage session:

sage: L = RootSystem(["A",3,1]).ambient_space()
sage: p = L.plot(reflection_hyperplanes=False, bounding_box=int(85)/int(100))
sage: p.tachyon()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: reduce() of empty sequence with no initial value

There is no Sage preprocessor run, so the bounding box gets evaluated to 0, which in turn, creates an empty Graphics3dGroup, which causes the reduce to fail:

sage: g = p.all[9]   # I think this will be the same for all, but not sure
sage: g.all
[]

This is now #23200. This is not a dependency as the correct picture should be plotted, which means the bounding box should be a rational number, meaning we never get the empty Graphics3dGroup (and generates the correct picture). I verified this last commit fixes the problem on the buildbot where it didn't work before.

That does not quite explain why we could not reproduce this failure locally, but it does explain everything else. I'm not going to bother with this last detail.

---

New commits:

5c7a2ba	Merge commit 'dfbc5bb' into u/tscrim/add_root_system_plots-22853
c083d5c	Explicitly call Rational in plot.

[thecaligarmo]

comment:43

Tested on my machine and everything seems to work correctly. Here's to hoping this time we have it.

[vbraun]

Changed branch from u/tscrim/add_root_system_plots-22853 to c083d5c

[nthiery]

Changed commit from c083d5c to none

[nthiery]

comment:45

Yeah!
Thanks so much everyone :-)