Fix order of characters in modular/local_comp/local_comp.py doctests

[sagetrac-tmonteil]

This doctest in src/sage/modular/local_comp/local_comp.py
fails on several patchbots:

sage -t --long --random-seed=0 src/sage/modular/local_comp/local_comp.py
**********************************************************************
File "src/sage/modular/local_comp/local_comp.py", line 653, in sage.modular.local_comp.local_comp.PrimitiveSupercuspidal.characters
Failed example:
    set(Pi.characters())
Expected:
    {Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> 1/3*j0^2*d - 1/3*j0^3, 5 |--> 5,
     Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> -1/3*j0^2*d, 5 |--> 5}
Got:
    {Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> -1/3*j0^2*d, 5 |--> 5,
     Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> 1/3*j0^2*d - 1/3*j0^3, 5 |--> 5}
**********************************************************************

Ticket #29977 provides a hotfix.
Here we aim for a better fix.

The test used to display Pi.characters(),
of type Sequence, directly.

The display order for the two characters in this
example changed, possibly due to a change in PARI.

The example was changed in #30801 to use Python sets
in an attempt to fix the display order, but that
failed and patchbots kept failing on that.

CC: @dimpase @kliem @slel

Component: doctest framework

Author: Thierry Monteil, Samuel Lelièvre, Jonathan Kliem

Branch: 2a175e5

Reviewer: Dima Pasechnik

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

[sagetrac-tmonteil]

Branch: u/tmonteil/doctests_in_src_sage_modular_local_comp_local_comp_py_make_patchbots_fail

[sagetrac-tmonteil]

Commit: ce78416

[sagetrac-tmonteil]

New commits:

ce78416

#32134 : use lists instead of sets in doctesting local_comp.py

[fchapoton]

comment:3

see #29977

[sagetrac-tmonteil]

comment:4

Replying to @fchapoton:

see #29977

I created this ticket because that particular issue has no link with random seeds, which is the purpose of #29977, see the comment #29977 comment:28

[fchapoton]

comment:5

The "set" were added very recently in #30801

[kliem]

comment:6

Btw, you don't use lists:

sage: f = Newforms(GammaH(25, [6]), 3, names='j')[0]                            
sage: Pi = LocalComponent(f, 5)                                                 
sage: type(Pi.characters())                                                     
<class 'sage.structure.sequence.Sequence_generic'>

[dimpase]

comment:7

Why is replacing one apparently unstable order with another apparently unstable order a good fix?
I didn't add set() just for fun in #30801.

[kliem]

comment:8

That is exactly why I didn't want to put it on positive review. I would really prefer sorted with key=str. That appears to be stable and if we think it's not pretty, we can figure out something nicer later.

[mkoeppe]

comment:10

Replying to @kliem:

I would really prefer sorted with key=str.

+1

[slel]

comment:11

I would also vote for a hotfix using the order given
by string representations.

Then we can calmly investigate what causes the different
display order, and find a nicer fix.

[kliem]

Changed author from Thierry Monteil to Thierry Monteil, ​Samuel Lelièvre, Jonathan Kliem

[kliem]

Changed branch from u/tmonteil/doctests_in_src_sage_modular_local_comp_local_comp_py_make_patchbots_fail to public/32134

[kliem]

Changed commit from ce78416 to 85ec7e2

[kliem]

comment:12

Moving this up to critical, as the issue reduces patchbot capacity by 50 percent and slows down review process of all tickets.

---

New commits:

85ec7e2

sort groups of smooth characters by string for stable doctesting

[dimpase]

comment:13

this is in part collides with #29977

[sagetrac-tmonteil]

comment:14

Replying to @kliem:

Btw, you don't use lists:

sage: f = Newforms(GammaH(25, [6]), 3, names='j')[0]                            
sage: Pi = LocalComponent(f, 5)                                                 
sage: type(Pi.characters())                                                     
<class 'sage.structure.sequence.Sequence_generic'>

Indeed.

Replying to @dimpase:

Why is replacing one apparently unstable order with another apparently unstable order a good fix?
I didn't add set() just for fun in #30801.

I was asking the same question, but I did not see any evidence (i might have missed something though). If the raw use of Pi.characters() is only apparently unstable, then i am not in favor to work around it, which is why i asked, otherwise we might end up sorting every Sage doctest. If there is a configuration that does not provide that order, which one it is ? If this is about the 32 vs 64 bit architecture, then why not specify it with two dedicated doctests ?

Replying to @slel:

I would also vote for a hotfix using the order given
by string representations.

Then we can calmly investigate what causes the different
display order, and find a nicer fix.

OK, then could you please all try the straightforward branch and tell if is passes on your various computers, so that we could see whether it can break and understand the cause (for example, it seems not related to the choice of the random seed).

[kliem]

comment:15

Replying to @sagetrac-tmonteil:

Replying to @kliem:

Btw, you don't use lists:

sage: f = Newforms(GammaH(25, [6]), 3, names='j')[0]                            
sage: Pi = LocalComponent(f, 5)                                                 
sage: type(Pi.characters())                                                     
<class 'sage.structure.sequence.Sequence_generic'>

Indeed.

Replying to @dimpase:

Why is replacing one apparently unstable order with another apparently unstable order a good fix?
I didn't add set() just for fun in #30801.

I was asking the same question, but I did not see any evidence (i might have missed something though). If the raw use of Pi.characters() is only apparently unstable, then i am not in favor to work around it, which is why i asked, otherwise we might end up sorting every Sage doctest. If there is a configuration that does not provide that order, which one it is ? If this is about the 32 vs 64 bit architecture, then why not specify it with two dedicated doctests ?

Replying to @slel:

I would also vote for a hotfix using the order given
by string representations.

Then we can calmly investigate what causes the different
display order, and find a nicer fix.

OK, then could you please all try the straightforward branch and tell if is passes on your various computers, so that we could see whether it can break and understand the cause (for example, it seems not related to the choice of the random seed).

https://github.com/kliem/sage/pull/47/checks

As #29977 is now positively review, we can now also go back to your branch and see whether this works.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ce78416	#32134 : use lists instead of sets in doctesting local_comp.py
2a175e5	merge

[sagetrac-git]

Changed commit from 85ec7e2 to 2a175e5

[dimpase]

Reviewer: Dima Pasechnik

[dimpase]

comment:18

Does this make bots happy?

[sagetrac-tmonteil]

comment:19

Replying to @dimpase:

Does this make bots happy?

I think we have to let 32bit bots to work on it. I will work on setting one up later in the summer. Let us keep this ticket to "needs review" so that bots can work on it.

I encourage everybody to try this branch to see if something wrong appears.

[dimpase]

comment:20

it works on 32-bit too.

[vbraun]

Changed branch from public/32134 to 2a175e5

[mkoeppe]

Changed commit from 2a175e5 to none

[mkoeppe]

Changed author from Thierry Monteil, ​Samuel Lelièvre, Jonathan Kliem to Thierry Monteil, Samuel Lelièvre, Jonathan Kliem