Opened 4 years ago

Closed 9 months ago

#27637 closed enhancement (fixed)

Equivariant Ehrhart Theory

Reported by: gh-sophiasage Owned by:
Priority: major Milestone: sage-9.6
Component: geometry Keywords: #Days98
Cc: Jean-Philippe Labbé Merged in:
Authors: Sophia Elia Reviewers: Frédéric Chapoton, Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: 92a1d97 (Commits, GitHub, GitLab) Commit: 92a1d9783538ed425f24984ca425402275e6afa4
Dependencies: Stopgaps:

Status badges

Description


Change History (102)

comment:1 Changed 4 years ago by gh-sophiasage

Authors: Sophia Elia
Cc: Jean-Philippe Labbé added
Component: PLEASE CHANGEgeometry
Keywords: #Days98 added
Priority: majorminor
Type: PLEASE CHANGEenhancement

comment:2 Changed 3 years ago by Erik Bray

Milestone: sage-8.8

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

comment:3 Changed 2 years ago by Sophia Elia

Branch: public/27637

comment:4 Changed 2 years ago by git

Commit: 71b8b324400873ff46f6b34a1466e60db20f8e8c

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

71b8b32first version

comment:5 Changed 2 years ago by git

Commit: 71b8b324400873ff46f6b34a1466e60db20f8e8c86e811d13d96c2cd4fd33ff404f786adb9bef14c

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

86e811dcommented out one of the functions

comment:6 Changed 22 months ago by git

Commit: 86e811d13d96c2cd4fd33ff404f786adb9bef14cece5dd02414744d7bd4a317a5707890ddd47c2d6

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

ece5dd0fixed conjugacy classes bug

comment:7 Changed 22 months ago by git

Commit: ece5dd02414744d7bd4a317a5707890ddd47c2d6957107024f874037de565e19a8e7e35358c32adc

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

9571070removing some functions

comment:8 Changed 21 months ago by git

Commit: 957107024f874037de565e19a8e7e35358c32adca3239086b46e69945b9f235171dd208326a09058

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

a323908cleaned code

comment:9 Changed 20 months ago by git

Commit: a3239086b46e69945b9f235171dd208326a09058a846851e54c347bdfe45864626c70d8960361502

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

a846851added integral check to is effective function

comment:10 Changed 20 months ago by git

Commit: a846851e54c347bdfe45864626c70d896036150216d24e43acc1ec6405f98689c4dc188a80c1b5ce

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

16d24e4typos

comment:11 Changed 20 months ago by git

Commit: 16d24e43acc1ec6405f98689c4dc188a80c1b5cea7db8e607a3d875fb04a13fee05b879e6a7cb252

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

a7db8e6renaming eetheory and remove old file

comment:12 Changed 20 months ago by Jean-Philippe Labbé

Few comments:

  • Is there a reason for the H to be capitalized?
  • The references should be placed in the references in the doc (see other files in polyhedron folder)
  • "The group Z/2Z acts ..." could be math formatted
  • Look at the conventions for docstring (example is the function vertex_facet_graph in base.py of the polyhedron folder). Further info here: https://doc.sagemath.org/html/en/developer/coding_basics.html

Then, one has to figure out how to integrate the code to the polyhedron class appropriately. I will address this in another comment.

comment:13 Changed 20 months ago by Matthias Köppe

Milestone: sage-9.4

comment:14 Changed 20 months ago by git

Commit: a7db8e607a3d875fb04a13fee05b879e6a7cb252b4ba4a4a4ed780c485e7c1a112811e3889eae59d

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

164ad7ffirst version
9dcbc05commented out one of the functions
47ae918fixed conjugacy classes bug
d083097removing some functions
144f347cleaned code
46f5cfeadded integral check to is effective function
67fc5a1typos
8809c42renaming eetheory and remove old file
fb958a4starting to fix references
b4ba4a4Merge branch 'public/27637' of trac.sagemath.org:sage into 27637

comment:15 Changed 19 months ago by git

Commit: b4ba4a4a4ed780c485e7c1a112811e3889eae59dbc7b232afe1c1305aec57f3fb6bf13747887d060

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

bc7b232renamed a method

comment:16 Changed 19 months ago by git

Commit: bc7b232afe1c1305aec57f3fb6bf13747887d06002bdbc9356a014f10e97632a65bcbd948840170e

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

02bdbc9typos

comment:17 Changed 19 months ago by git

Commit: 02bdbc9356a014f10e97632a65bcbd948840170efb5898f3deaf378b7e1ad5ce5275852aacac5cc5

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

fb5898fstarting to move functions

comment:18 Changed 19 months ago by git

Commit: fb5898f3deaf378b7e1ad5ce5275852aacac5cc5cad7216224dfb943516f2321ad1ab52f6ce79b29

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

cad7216added is_polynomial to fraction_field_element

comment:19 Changed 19 months ago by git

Commit: cad7216224dfb943516f2321ad1ab52f6ce79b2927751abae245f60e24cceeccc6e98ee4009c5595

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

27751abwhite spaces

comment:20 Changed 19 months ago by git

Commit: 27751abae245f60e24cceeccc6e98ee4009c559561379f87597e064a53d1ca775f0a179ca984f7d3

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

61379f8renamed is_polynomial method

comment:21 Changed 19 months ago by git

Commit: 61379f87597e064a53d1ca775f0a179ca984f7d32558cfb47c91ab7a644d3fbf7711a96ea1a8db26

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

2558cfbmoved Hstar_function to base_QQ and backend_normaliz

comment:22 Changed 19 months ago by git

Commit: 2558cfb47c91ab7a644d3fbf7711a96ea1a8db26db427eee93bc41c3c3e6e82b94fd1415ae5e598c

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

db427eemoved last functions into place

comment:23 Changed 19 months ago by git

Commit: db427eee93bc41c3c3e6e82b94fd1415ae5e598c168a890adddefcd4cca22c2a9e163c07ff2ad01e

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

168a890deleted equivariant_ehrhart file and added compactness checks

comment:24 Changed 19 months ago by git

Commit: 168a890adddefcd4cca22c2a9e163c07ff2ad01eb30ec45f8a6d07b272f990c2a21183dfea5c862e

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

b30ec45working through docstring tests

comment:25 Changed 19 months ago by git

Commit: b30ec45f8a6d07b272f990c2a21183dfea5c862ef0a1dba64e02d6e036c800c1f82fb96f8e0be3db

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

f0a1dbaadding imports

comment:26 Changed 19 months ago by git

Commit: f0a1dba64e02d6e036c800c1f82fb96f8e0be3db7e7df59d409e2ec98fe016a9a9b5219c45b10290

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

7e7df59fixing docstrings and method for testing polynomiality

comment:27 Changed 19 months ago by Frédéric Chapoton

Hello, Sophia

  • In the bib file, use unicode characters and not latex syntax, here:
    Federico Ardila, Mariel Supina, and Andr\ ́{e}s R. Vindas-Mel\ ́{e}ndez, 
    
  • I will now lauch my patchbot on the branch, so that you can also benefit from the patchbot's report.

comment:28 Changed 19 months ago by Frédéric Chapoton

syntax error (missing import)

      File "/home/chapoton/sage/local/lib/python3.9/site-packages/sage/geometry/polyhedron/backend_normaliz.py", line 2521
        from sage.rings.fraction_field_element is_element_of_base_ring

comment:29 Changed 19 months ago by git

Commit: 7e7df59d409e2ec98fe016a9a9b5219c45b10290f27e43d0cb6b12094dd3c7a43f2a787cd463a61e

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

f27e43dunicode in bib

comment:30 Changed 19 months ago by Sophia Elia

Thank you Frédéric! I'm still trying to figure out the import error (besides just missing the word import)

Last edited 19 months ago by Sophia Elia (previous) (diff)

comment:31 Changed 19 months ago by Frédéric Chapoton

well, "is_element_of_base_ring" is a method in a class, it cannot be imported, but only used

comment:32 Changed 19 months ago by git

Commit: f27e43d0cb6b12094dd3c7a43f2a787cd463a61ef16d6a41c5f630f0f17d562d578b6f580ea53bc2

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

f16d6a4still fixing imports

comment:33 Changed 19 months ago by git

Commit: f16d6a41c5f630f0f17d562d578b6f580ea53bc2a3a0e9c1e3877d76741d13a4d70287a2a87ac92b

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

a3a0e9cadding is_effective to base QQ

comment:34 Changed 19 months ago by Frédéric Chapoton

patchbot says that the doc does not build:

polyhedron/base_QQ.py:docstring of sage.geometry.polyhedron.base_QQ.Polyhedron_QQ.Hstar_function:17: WARNING: Unexpected indentation.

because the lines

+           If ``None``, it is set to the full `restricted_automorphism_group`
+           of `self`. The acting group should always use output='permutation'.

there are indented by one more space than they should

comment:35 Changed 19 months ago by git

Commit: a3a0e9c1e3877d76741d13a4d70287a2a87ac92b5aa7c909e974566bfa912e9fa1bd14fa60187f02

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

5aa7c90extra spaces

comment:36 Changed 19 months ago by Frédéric Chapoton

Now the doc buils, but doctests fails:

sage -t --long --random-seed=0 src/sage/geometry/polyhedron/base_QQ.py  # 5 doctests failed
sage -t --long --random-seed=0 src/sage/rings/fraction_field_element.pyx  # 4 doctests failed
sage -t --long --random-seed=0 src/sage/geometry/polyhedron/backend_normaliz.py  # 2 doctests failed

Note also that that the patchbot plugins give some warnings (click on the round status badge, then on the links next to the small red crosses). The first one tells you that you should not remove the r in r""" here:

-r"""
+"""
 Base class for polyhedra over `\QQ`
 """

comment:37 Changed 19 months ago by git

Commit: 5aa7c909e974566bfa912e9fa1bd14fa60187f02941f357b70af5d2581f96f0bfa7b800ed93c35a9

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

941f357adding back an r

comment:38 Changed 19 months ago by git

Commit: 941f357b70af5d2581f96f0bfa7b800ed93c35a9c0824592fdeebab9bf28a511526f4d9e9338acb9

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

c082459pyflakes warnings

comment:39 Changed 19 months ago by Frédéric Chapoton

the failing doctests in src/sage/rings/fraction_field_element.pyx are caused by the wrong change made in the existing doc in this file:

-            sage: from sage.rings.fraction_field_element import FractionFieldElement
+            sage: from sage.rings import fraction_field_element

comment:40 Changed 19 months ago by git

Commit: c0824592fdeebab9bf28a511526f4d9e9338acb90df635a42d192553916e5f3eef4008b7e4d378e3

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

0df635afixing import typo

comment:41 Changed 19 months ago by Frédéric Chapoton

ok, making progress :)

now when you have a doctest taking several lines like

sage: 2+
....: 3

then you must tag both lines with # optional - pynormaliz and not only the first or the second.

comment:42 Changed 19 months ago by Sophia Elia

Thank you for helping so much :) sorry that it is going slowly.

comment:43 Changed 19 months ago by git

Commit: 0df635a42d192553916e5f3eef4008b7e4d378e3cc871dd066839fd6d30c8025dc03efdec083ef41

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

cc871ddfixing optionals

comment:44 Changed 19 months ago by Frédéric Chapoton

I am happy to help, one step after another. Not far from being good to go, I think.

Some other comments:

in /src/sage/rings/fraction_field_element.pyx, EXAMPLES: should take two colons.

in general, NOTE: should be .. NOTE:: and the content indented by 4

comment:45 Changed 19 months ago by git

Commit: cc871dd066839fd6d30c8025dc03efdec083ef41316fa4bda95865d53bbc10acb155192ba263bda4

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

316fa4bfixing NOTE in docstrings and working on docstring conventions

comment:46 Changed 19 months ago by Frédéric Chapoton

I am not sure it is worth introducing "is_element_of_base_ring" to use it only once.

In "_Hstar_is_effective_normaliz", you could avoid using flag and just return False as soon as you meet the condition. Then "return True" at the end.

Otherwise, patchbot is green, so this now needs a more mathematical review. I am afraid I cannot do that. Maybe Jean-Philippe can take the task ?

comment:47 in reply to:  46 Changed 18 months ago by Jean-Philippe Labbé

Replying to chapoton:

I am not sure it is worth introducing "is_element_of_base_ring" to use it only once.

In "_Hstar_is_effective_normaliz", you could avoid using flag and just return False as soon as you meet the condition. Then "return True" at the end.

Otherwise, patchbot is green, so this now needs a more mathematical review. I am afraid I cannot do that. Maybe Jean-Philippe can take the task ?

Salut Frédéric, merci beaucoup pour l'aide, c'est très apprécié!

Yes, I can go over the math for sure.

comment:48 Changed 18 months ago by git

Commit: 316fa4bda95865d53bbc10acb155192ba263bda41ada20d9b50846317cf1026e8e88ca6a22aa9392

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

1ada20dMerge branch 'develop' into sophiasticket

comment:49 Changed 18 months ago by git

Commit: 1ada20d9b50846317cf1026e8e88ca6a22aa93925fd81341aa08f6977e3eb07ea857e30c6a93be78

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

5fd8134remove element_of_base_ring, remove flag is_effective, jps comments

comment:50 Changed 18 months ago by git

Commit: 5fd81341aa08f6977e3eb07ea857e30c6a93be7848cd3b2aa79fc7308bf79e1cf891b5e7f6095071

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

48cd3b2remove extraneous import, add conj class reps to complete output of Hstar_function

comment:51 Changed 18 months ago by git

Commit: 48cd3b2aa79fc7308bf79e1cf891b5e7f60950712723746b85a2538f6b0addcac1319c37ffa22cd7

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

2723746added missing optional

comment:52 Changed 17 months ago by git

Commit: 2723746b85a2538f6b0addcac1319c37ffa22cd77ea4929483f5344d85fa2d75fa367b5b187ca53d

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

7ea4929Merge branch 'develop' into sophiasticket

comment:53 Changed 17 months ago by Jean-Philippe Labbé

Here are a few comments:

  • Since the GAP import is only used once, I would put it inside the function that uses only to limit global imports.
  • I'm not sure about the function ".is_full_dimensional". This is a bit a colloquial term and it isn't harder than checking if the ambient dim is the same as the dimension of the polytope (think: which is more transparent in the code "full_dimensional" or "ambient dim=dim"). I believe that the former is more transparent. I would delete the new method.
  • You don't need the import of "var". Check how this is done in ehrhart_series in backend_normaliz. "new_result" is going to complain, but you can easily get a list by using the .gens of the ring you create...
  • Could you use "set" instead of "Set"?
  • The docstring of "fixed_subpolytopes" should be revisited. What does it return? I expect polytopes from the fct name, but then the first sentence is super intricated. Then the output should say that "the elements of ..." are the keys.
  • Within the function Hstar_function_normaliz, you should try to put all the imports at the beginning of the function.
  • I suggest '_express_Hstar_as_rational_fn_in_t' -> "_Hstar_as_rat_fct"

comment:54 Changed 17 months ago by Jean-Philippe Labbé

Status: newneeds_review

comment:55 Changed 17 months ago by Jean-Philippe Labbé

Status: needs_reviewneeds_work

comment:56 Changed 17 months ago by Sophia Elia

Thank you for the comments Jean-Philippe. I didn't write the method is_full_dimensional... i don't know why it is showing up in the diff - it wasn't before. I will work on your other comments

comment:57 Changed 17 months ago by Matthias Köppe

+    def is_full_dimensional(self):
+        """
+        Return whether the polyhedron is full dimensional.
+
+        OUTPUT:

This comes from a bad merge, please remove.

is_full_dimensional was moved from Polyhedron_base to the new base class ConvexSet_base in #31919.

comment:58 Changed 16 months ago by git

Commit: 7ea4929483f5344d85fa2d75fa367b5b187ca53d7554ca5cef1c44ddd8d7751ee93c521a89e9c744

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

7904601remove and move imports, remove is_full_dim, docstring of fixed_subpolytopes, rename _express_Hstar_as_rational_fn_in_t
7554ca5Merge branch 'public/27637' of trac.sagemath.org:sage into 27637

comment:59 in reply to:  53 ; Changed 16 months ago by Sophia Elia

Replying to jipilab:

Here are a few comments:

  • Since the GAP import is only used once, I would put it inside the function that uses only to limit global imports.

Done

  • I'm not sure about the function ".is_full_dimensional". This is a bit a colloquial term and it isn't harder than checking if the ambient dim is the same as the dimension of the polytope (think: which is more transparent in the code "full_dimensional" or "ambient dim=dim"). I believe that the former is more transparent. I would delete the new method.

Removed method and use of is_full_dimensional

  • You don't need the import of "var". Check how this is done in ehrhart_series in backend_normaliz. "new_result" is going to complain, but you can easily get a list by using the .gens of the ring you create...

Done

  • Could you use "set" instead of "Set"?

Tried this but it gave an unhashable error

  • The docstring of "fixed_subpolytopes" should be revisited. What does it return? I expect polytopes from the fct name, but then the first sentence is super intricated. Then the output should say that "the elements of ..." are the keys.

Changed

  • Within the function Hstar_function_normaliz, you should try to put all the imports at the beginning of the function.

Done

  • I suggest '_express_Hstar_as_rational_fn_in_t' -> "_Hstar_as_rat_fct"

Done

I was forced to pull and merge before I could commit my changes.. I'm not sure if I did that correctly and/or messed anything up.

comment:60 in reply to:  59 Changed 16 months ago by Matthias Köppe

Replying to selia:

  • Could you use "set" instead of "Set"?

Tried this but it gave an unhashable error

There's also frozenset, which is immutable and hashable if its elements are

comment:61 Changed 16 months ago by git

Commit: 7554ca5cef1c44ddd8d7751ee93c521a89e9c744b56670a5741c29923e70ccce5637f5329c2b642f

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

c0bd3f9Merge branch 'develop' into 27637
b56670achanged Set to frozenset

comment:62 Changed 16 months ago by Sophia Elia

Status: needs_workneeds_review

comment:63 Changed 15 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:64 Changed 15 months ago by Jean-Philippe Labbé

Status: needs_reviewneeds_work

In the function Hstar_function_normaliz change

+        if perm == ident_perm:
+            pass
+        else:
+            raise ValueError("The conjugacy classes don't match with the character table")

To an AssertionError? using an assert, it is better than ValueError?.

I would not use a flag but simply write:

+        codim = self.ambient_dim() - self.dim()
+        for perm in conj_reps:
+            mat = group_dict[perm]
+            mat = mat.change_ring(Ring)
+            new_matrix = identity - mat*ts_matrix
-            if flag:
-                det = (1-t)**-codim*(new_matrix.determinant())
-            else:
-                det = new_matrix.determinant()
+            det = (1-t)**-codim*(new_matrix.determinant())
+            det_vector.append(det)

comment:65 Changed 15 months ago by git

Commit: b56670a5741c29923e70ccce5637f5329c2b642f9a349efa95da4d08aa367e19a8ce4894f8ab3bf5

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

a449cbbremoved imports of Set in base_QQ and deprecated_function_alias in base, changed output format of character table
9a349efchanged valueerror to assertion, removed if else for codimension

comment:66 Changed 14 months ago by git

Commit: 9a349efa95da4d08aa367e19a8ce4894f8ab3bf5f78a09f926258180b129b34ca2319cf83760e79f

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

f78a09fremoved flag, changed output of is_effective method for non effective Hstar series

comment:67 Changed 14 months ago by Sophia Elia

Status: needs_workneeds_review

comment:68 Changed 14 months ago by Frédéric Chapoton

I am seeing a few details that could be enhanced, but only cosmetics:

  • some
    return(something)
    

that should rather be

return something
  • and some raise SomeError('message') where the message starts with a capital letter (it should not)

comment:69 Changed 13 months ago by git

Commit: f78a09f926258180b129b34ca2319cf83760e79f0c4e50c9e1b616c195505e854a089b7444a9ada1

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

0e3d6famerging with devolop
0c4e50cremoves caps from error messages, fixed merge conflict, fixed return syntax

comment:70 Changed 13 months ago by Frédéric Chapoton

Reviewers: Frédéric Chapoton, Jean-Philippe Labbé
Status: needs_reviewpositive_review

ok, thanks. Let this enter, then.

Jean-Philippe, c'est bon pour toi ?

comment:71 Changed 13 months ago by Volker Braun

Status: positive_reviewneeds_work

PDF docs fail

[docpdf] ! Missing $ inserted.
[docpdf] <inserted text> 
[docpdf]                 $
[docpdf] l.49818 Here, \(\H^
[docpdf]                    *(t) = \sum\_{m} \chi\_{m\text{\)\}\}t\textasciicircum{}m...
[docpdf] 
[docpdf] ? 
[docpdf] ! Emergency stop.

comment:72 Changed 13 months ago by Samuel Lelièvre

In src/sage/geometry/polyhedron/base_QQ.py:

     def Hstar_function(self, acting_group=None, output=None):
         r"""
         Return `H^*` as a rational function in `t` with coefficients in
         the ring of class functions of the ``acting_group``
         of this polytope.

-        Here, `\H^*(t) = \sum\_{m} \chi\_{m\text{``self``}}t^m det(Id-\rho(t))`.
+        Here, `H^*(t) = \sum\_{m} \chi\_{m\text{``self``}}t^m \det(Id-\rho(t))`.
         The irreducible characters of ``acting_group`` form an orthonormal basis
         for the ring of class functions with values in `\mathbb C`.
         The coefficients of `H^*(t)` are expressed in this basis.

comment:73 Changed 13 months ago by git

Commit: 0c4e50c9e1b616c195505e854a089b7444a9ada1f96dd717a2e352ae53542a35d57304b5a3acdd1b

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

f96dd71fixed syntax for pdf docs

comment:74 Changed 13 months ago by Sophia Elia

There were some pyflakes errors:

src/sage/geometry/polyhedron/base.py:4191:17 'sage.graphs.graph' imported but unused src/sage/geometry/polyhedron/base.py:7354:17 'sage.graphs.graph' imported but unused src/sage/geometry/polyhedron/base.py:7940:17 'sage.graphs.graph' imported but unused src/sage/geometry/polyhedron/base.py:10822:13 'sage.graphs.graph' imported but unused

Should I remove these imports or ignore this?

comment:75 Changed 13 months ago by Matthias Köppe

These warnings are not coming from this ticket. If you find a way to silence them (perhaps there are comments that can control pyflakes?), that's great, but these imports are needed for feature testing.

comment:76 Changed 13 months ago by Sophia Elia

Status: needs_workneeds_review

comment:77 Changed 13 months ago by git

Commit: f96dd717a2e352ae53542a35d57304b5a3acdd1b42114697d45508f8e9d9707e6a375883de4e7cd2

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

4211469added comments to ignore pyflakes import errors on four lines in base.py

comment:78 Changed 13 months ago by Sophia Elia

Oops, the comments I added silence the warnings from base.py when I run

flake8 base.py 

but not

pyflakes base.py

So I will remove them again.

comment:79 Changed 13 months ago by git

Commit: 42114697d45508f8e9d9707e6a375883de4e7cd2183989601c80c3daecd99e18ffc74af35a3e598d

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

1839896removing comments again

comment:80 Changed 13 months ago by Frédéric Chapoton

Status: needs_reviewpositive_review

noch ein mal ; let's try again

comment:81 Changed 13 months ago by Volker Braun

Status: positive_reviewneeds_work

PDF docbuild fails:

[sagemath_doc_pdf-none] {\)\}\}t\textasciicircum {}m det(Id\sphinxhyphen {}rho(t))\textasciigrave \ETC.
[sagemath_doc_pdf-none] ! Paragraph ended before \text@ was complete.
[sagemath_doc_pdf-none] <to be read again> 
[sagemath_doc_pdf-none]                    \par 
[sagemath_doc_pdf-none] l.47822 
[sagemath_doc_pdf-none]         
[sagemath_doc_pdf-none] ? 
[sagemath_doc_pdf-none] ! Emergency stop.
[sagemath_doc_pdf-none] <to be read again> 
[sagemath_doc_pdf-none]                    \par 
[sagemath_doc_pdf-none] l.47822 

comment:82 Changed 13 months ago by Samuel Lelièvre

This might solve the issue:

-        Here, `H^*(t) = \sum\_{m} \chi\_{m\text{``self``}}t^m \det(Id-\rho(t))`.
+        Here, `H^*(t) = \sum\_{m} \chi\_{m\text{self}}t^m \det(Id-\rho(t))`.

comment:83 Changed 12 months ago by git

Commit: 183989601c80c3daecd99e18ffc74af35a3e598d6afa7916108afb8f254fc1262ab6d4c4e2898085

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

6afa791removed backticks

comment:84 Changed 12 months ago by Sophia Elia

Status: needs_workneeds_review

comment:85 Changed 12 months ago by Frédéric Chapoton

Sophia, would you please check that pdf (and html) documentation builds ? I cannot handle that, and it would be better not to send again the ticket to the release manager without being sure that everything now works.

comment:86 Changed 12 months ago by Sophia Elia

Status: needs_reviewneeds_work

Hi Frédéric - yes I'll do that! (Sorry not to have attempted yet) I don't remember how to do it, but I'll ask Jean-Philippe for a pointer to some documentation.

Last edited 12 months ago by Sophia Elia (previous) (diff)

comment:87 Changed 12 months ago by Frédéric Chapoton

This should be just

make doc-pdf

and

make doc-html

This may take some time, given the size of the documents.

comment:88 Changed 12 months ago by Sophia Elia

Thanks! They both built this time. However, they could look nicer. I will work on that.

comment:89 Changed 12 months ago by git

Commit: 6afa7916108afb8f254fc1262ab6d4c4e28980858bd3536aa19ba695d36989cd2b8c694dfd6df2b8

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

8bd3536making the pdfs look nicer

comment:90 Changed 12 months ago by Sophia Elia

I don't understand why the merge failed. Any help?

comment:91 Changed 12 months ago by Frédéric Chapoton

Hello, there a conflict with 9.5.beta8 in the imports header of

CONFLIT (contenu) : Conflit de fusion dans src/sage/geometry/polyhedron/base.py

This should be easy to fix.

comment:92 Changed 11 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:93 Changed 10 months ago by git

Commit: 8bd3536aa19ba695d36989cd2b8c694dfd6df2b892a1d9783538ed425f24984ca425402275e6afa4

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

92a1d97fixed merge conflict in base

comment:94 Changed 9 months ago by Frédéric Chapoton

pyflakes plugin complains about imports

comment:95 Changed 9 months ago by Sophia Elia

I don't think those errors are from this ticket - earlier mkoeppe said those imports are needed for feature testing

comment:96 Changed 9 months ago by Frédéric Chapoton

Status: needs_workpositive_review

ok, then let us try to merge this.

comment:97 Changed 9 months ago by Matthias Köppe

Priority: minormajor

comment:98 Changed 9 months ago by Volker Braun

Status: positive_reviewneeds_work
**********************************************************************
61
File "src/sage/modular/modform/ambient.py", line 791, in sage.modular.modform.ambient.?._compute_hecke_matrix
62
Failed example:
63
    [f[0]%p for p in prime_range(100)] # long time (0s, depends on above)
64
Expected:
65
    [0, 0, 0, 0, 1, 9, 2, 7, 0, 0, 0, 0, 1, 12, 9, 16, 37, 0, 21, 11, 70, 22, 0, 58, 76]
66
Got:
67
    [0,
68
     0,
69
     1,
70
     5,
71
     1,
72
     0,
73
     8,
74
     1,
75
     1,
76
     3,
77
     27,
78
     28,
79
     17,
80
     33,
81
     44,
82
     50,
83
     25,
84
     29,
85
     32,
86
     59,
87
     61,
88
     32,
89
     43,
90
     53,
91
     0]
92
**********************************************************************
93
File "src/sage/modular/modform/ambient.py", line 793, in sage.modular.modform.ambient.?._compute_hecke_matrix
94
Failed example:
95
    [f[42]%p for p in prime_range(100)] # long time (0s, depends on above)
96
Expected:
97
    [0, 0, 4, 0, 10, 4, 4, 8, 12, 1, 23, 13, 10, 27, 20, 13, 16, 59, 53, 41, 11, 13, 12, 6, 82]
98
Got:
99
    [0,
100
     2,
101
     2,
102
     3,
103
     5,
104
     1,
105
     9,
106
     2,
107
     19,
108
     4,
109
     6,
110
     36,
111
     1,
112
     32,
113
     39,
114
     19,
115
     23,
116
     58,
117
     29,
118
     14,
119
     30,
120
     71,
121
     49,
122
     80,
123
     31]
124
**********************************************************************
125
1 item had failures:
126
   2 of  12 in sage.modular.modform.ambient.?._compute_hecke_matrix
127
    [118 tests, 2 failures, 1.66 s]
128
----------------------------------------------------------------------
129
sage -t --long --warn-long 41.1 --random-seed=123 src/sage/modular/modform/ambient.py  # 2 doctests failed
130
----------------------------------------------------------------------
131

comment:99 Changed 9 months ago by Matthias Köppe

Unlikely that this failure is coming from this ticket

comment:100 Changed 9 months ago by Sophia Elia

I don't think this error is coming from the ticket either... that example isn't from this ticket.

comment:101 Changed 9 months ago by Frédéric Chapoton

Status: needs_workpositive_review

indeed, so let us switch back to "positive review"

comment:102 Changed 9 months ago by Volker Braun

Branch: public/2763792a1d9783538ed425f24984ca425402275e6afa4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.