Opened 13 years ago
Closed 13 years ago
#8133 closed defect (fixed)
changing the string representation of Dirichlet charachters
Reported by: | wuthrich | Owned by: | Craig Citro |
---|---|---|---|
Priority: | major | Milestone: | sage-4.4 |
Component: | modular forms | Keywords: | dirichlet characters |
Cc: | William Stein | Merged in: | sage-4.4.alpha0 |
Authors: | Chris Wuthrich | Reviewers: | John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The current representation of Dirichlet characters as something like [1,zeta6,-1]
is not very helpful, especially because it is not even clear what generator we are talking about in (Z/N)*.
Attachments (1)
Change History (16)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 13 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 13 years ago by
Status: | needs_review → needs_work |
---|
sorry, this is not ready for review ! I fail to see some tests.
comment:5 Changed 13 years ago by
Cc: | William Stein added |
---|
Hmmm .... I should also modify book_stein_modform.py.
I guess this requires the authorisation of the author. Of course, this patch would not change the answers or the useability of the code in the book, but it would make the output look somewhat prettier.
William, what is the verdict on such a change ?
comment:6 follow-up: 7 Changed 13 years ago by
Replying to wuthrich:
The current representation of Dirichlet characters as something like
[1,zeta6,-1]
is not very helpful, especially because it is not even clear what generator we are talking about in (Z/N)*.
I have concerns:
- First, you can easily get the generators of the Dirichlet group. I do not think they have to be given explicitly in the print representation (just like the basis for a vector space doesn't have to be given in a matrix). See below -- just use the
unit_gens()
method to find out the gens that are being mapped.sage: G.<a,b,c> = DirichletGroup(40) sage: b [1, -1, 1] sage: a [-1, 1, 1] sage: c [1, 1, zeta4] sage: G.unit_gens() [31, 21, 17]
- There are potential issues with your suggested change:
Dirichlet character modulo 12 of conductor 4 mapping the generators to {5: 1, 7: -1}
The problem is that it literally makes no sense to read it. The generators don't get mapped to a Python dictionary. It's like a mixed metaphor. Moreover, if you use Python dictionary notation, maybe you really have a dictionary there, so the keys can come in random order, which is bad.
- If we're going to make some big change, it would be better to make it consistent with ring homomorphisms, which all do print in the same way:
sage: R.<x,y> = QQ[]}}} sage: phi = R.hom([y^3,x-3]); phi Ring endomorphism of Multivariate Polynomial Ring in x, y over Rational Field Defn: x |--> y^3 y |--> x - 3
However, this notation is definitely too heavy as is for Dirichlet characters.
I'm not going to suggest a change, since I actually like how Dirichlet characters are currently printed.
comment:7 Changed 13 years ago by
Thanks a lot for the valuable comments. Sorry to insist a bit more on this change. I have been using Dirichlet characters quite a lot recently for p-adic L-functions (of elliptic curves and for zeta functions) and I thought the current printing was not useful.
Replying to was:
- First, you can easily get the generators of the Dirichlet group. I do not think they have to be given explicitly in the print representation (just like the basis for a vector space doesn't have to be given in a matrix). See below -- just use the
unit_gens()
method to find out the gens that are being mapped.
I almost agree with you, only with a few minor points :
- The group (Z/N)* is not a vector space, for instance for N=16, we have Z/2 + Z/4 and so the two generators have different order and it would be great to recognise that immediately in the string representation.
- Usually, in a vector space the elements do not have canonical names as the residue classes in Z/N have.
- There is no need to have it, of course; but I think it would improve the user-friendliness of sage. It is completely counter-intuitive for a new user to ask for a Dirichlet character, which is a map from ZZ to R, and to be given back a list of values in R.
- There is no place in sage where a Dirichlet character is treated as a list of values, apart from the _repr_. So I do not see the advantage of the list representation.
- I think it is advantage when the _repr_ tells what sort of type we are dealing with. Seeing a list, one is tempted to do things like
chi[0]
.
Totally agree with you on this one. My English is bad and it should read "mapping 5> 1, 7> -1" instead. Yes I did put a dictionary there and I agree that it is not good, exactly because of what I said earlier.
- There are potential issues with your suggested change:
[...] The problem is that it literally makes no sense to read it. The generators don't get mapped to a Python dictionary. It's like a mixed metaphor. Moreover, if you use Python dictionary notation, maybe you really have a dictionary there, so the keys can come in random order, which is bad.
- If we're going to make some big change, it would be better to make it consistent with ring homomorphisms, [...]
However, this notation is definitely too heavy as is for Dirichlet characters.
Yes, I agree with both. So as a second attempt I would propose a long representation of the form
sage: chi Dirichlet character modulo 12 of conductor 4 mapping 5 |--> 1, 7 |--> -1
or maybe one could print the order, too. And a short representation as in
sage: ModularForms(chi,7) Modular Forms space of dimension 14, character 5 |--> 1, 7 |--> -1 and weight 7 over Rational Field
or maybe with () around it. Or we could leave it there as values_on_gens().
I'm not going to suggest a change, since I actually like how Dirichlet characters are currently printed.
... this is of course a valid poitn of taste about which I won't argue about at all.
comment:8 Changed 13 years ago by
Authors: | → Chris Wuthrich |
---|---|
Status: | needs_work → needs_review |
So, here is a new version of the patch. This one is ready to be reviewed. It changes the string representation of a Dirichlet character to
sage: chi Dirichlet character modulo 12 of conductor 4 mapping 5 |--> 1, 7 |--> -1
but leaves the usual representation within modular form etc, like
sage: ModularForms(chi,7) Modular Forms space of dimension 14, character [1,-1] and weight 7 over Rational Field
If this change is not approved by some, I would propose to make a vote on sage-nt.
comment:9 Changed 13 years ago by
I like the new output format. But I can see that William might be attached to the old one, being used to (and also responsible for) it.
Testing now...
comment:10 Changed 13 years ago by
Status: | needs_review → positive_review |
---|
...all tests pass, so I am giving this a positive review --but would not mind opening the discussion up, so will post on sage-nt also).
comment:11 Changed 13 years ago by
Reviewers: | → John Cremona |
---|
comment:12 Changed 13 years ago by
After 4 days, there were two responses on sage-nt: a positive one from William, and a suggestion for a few further changes from Robert Bradshaw (see http://groups.google.co.uk/group/sage-nt/browse_thread/thread/be56e6f0e29b44e8).
I certainly like the new representation better. Perhaps the _latex_ method should use \mapsto rather than the ASCII art |-->, but it should certainly produce \zeta_{n}^{k} rather than returning just plain _repr_. Another alternative short representation would be something like (31, 41, 37) |--> (1, 1, zeta4) - Robert
comment:13 Changed 13 years ago by
Latexing of Dirichlet characters is being tracked at #8584. I have uploaded a new patch there which implements Robert's suggestions (using \mapsto
and the latex representation of roots of unity).
For what it's worth, I'm also in favour of the change.
comment:14 Changed 13 years ago by
I was just about to submit an additional patch (equal to the new one in #8584), because I completely agree with changing the LaTeX representation as well. Thanks David, I will review yours.
comment:15 Changed 13 years ago by
Merged in: | → sage-4.4.alpha0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Merged "trac_8133.patch" in 4.4.alpha0.
I put this in the component 'modular forms' since the relevant file dirichlet.py is there. But I think it is a bit odd that this file is there; it should be with number fields, I believe.
The attached patch changes the string representation of a Dirichlet character to
I have also created a shorter representation that is used in other _repr_ like in