Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dirichlet character bug #5792

Closed
loefflerd mannequin opened this issue Apr 15, 2009 · 10 comments
Closed

Dirichlet character bug #5792

loefflerd mannequin opened this issue Apr 15, 2009 · 10 comments

Comments

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Apr 15, 2009

This is pretty worrying:

sage: half_integral_weight_modform_basis(trivial_character(16), 9, 10)
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)

/home/david/.sage/temp/groke/13438/_home_david__sage_init_sage_0.py in <module>()

/home/david/sage-3.4.1.rc2/local/lib/python2.5/site-packages/sage/modular/modform/half_integral.pyc in half_integral_weight_modform_basis(chi, k, prec)
    115     chi = chi.minimize_base_ring()
    116     psi = chi.parent()(DirichletGroup(4, chi.base_ring()).gen())
--> 117     eps = chi*psi**((k+1) // 2)
    118     eps = eps.minimize_base_ring()
    119     M   = constructor.ModularForms(eps, (k+1)//2)

/home/david/sage-3.4.1.rc2/local/lib/python2.5/site-packages/sage/modular/dirichlet.pyc in __pow__(self, n)
    463             [1, -1]
    464         """
--> 465         return DirichletCharacter(self.parent(), n * self.element(), check=False)
    466
    467     def _repr_(self):

/home/david/sage-3.4.1.rc2/local/lib/python2.5/site-packages/sage/modular/dirichlet.pyc in element(self)
   1447             M    = P._module
   1448             dlog = P._zeta_dlog
-> 1449             v = M([dlog[x] for x in self.values_on_gens()])
   1450             self.__element = v
   1451             return v

KeyError: -1

So something is going a bit wrong when multiplying trivial_character(16) by another character.

I thought I had fixed this one, as part of #5648: there was a bug with arithmetic operations on characters whose parents had the same moduli but different zeta orders. But it looks like it isn't fixed after all.

Component: modular forms

Keywords: dirichlet characters

Author: Craig Citro

Reviewer: David Loeffler

Merged: 4.0.alpha0

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

@loefflerd loefflerd mannequin assigned craigcitro Apr 15, 2009
@craigcitro
Copy link
Member

comment:1

Well, I've tracked down the problem, but I'm not sure what the "right" fix is. (There are several possibilities.)

Amusingly, it's not actually the trivial character that causes trouble directly -- it's the other character here. (The exception gets raised trying to raise psi above to a power.) Here's what's going on ... the trivial character gets created in the group of characters of conductor 16 and whose only value can be 1:

sage: id = trivial_character(16)
sage: id.parent()
Group of Dirichlet characters of modulus 16 over Rational Field
sage: id.parent().zeta_order()
1

Then the nontrivial character of conductor 4 gets coerced in, which actually works -- but that's the character that causes trouble:

sage: H = DirichletGroup(4,base_ring=QQ)
sage: psi = id.parent()(H.0)
sage: psi.values()
BOOM!

In some sense, this is silly: it should at least have zeta_order give 2, since there are two roots of unity in QQ. However, the other issue is that coercing H.0 into id.parent() should never have worked -- its order doesn't divide the zeta_order of id.parent(), so it should have died there. Then step 1 is to make the call id.parent()(H.0) fail.

Of course, now that just makes the call to half_integral_weight_modform_basis fail -- so we need to do something there. We have (at least) two choices:

  • change the trivial_character call to give zeta_order=2
  • change half_integral.py so that it calls differently.

I think the first is probably the more pragmatic choice, though maybe less "philosophically correct"?

I've attached a patch that does the former.

@craigcitro
Copy link
Member

Attachment: trac-5792.patch.gz

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Apr 16, 2009

comment:2

There is a bit more too this, though.

Basically, the whole Dirichlet character machinery goes a bit mad whenever you have two Dirichlet characters with the same modulus and base ring, but where the zeta orders aren't the same. For a start, the parent DirichletGroups compare as equal but their elements are different -- or at least they will be if we fix the !call! method properly.

Similarly, arithmetic coercion is screwed up, in some interesting ways:

sage: K.<w> = NumberField(x^2 + x + 1)
sage: G2 = DirichletGroup(7, K, zeta_order=2, zeta=K(-1))
sage: G3 = DirichletGroup(7, K, zeta_order=3, zeta=w)
sage: G2.0 * G3.0

Here the coercion model will fail miserably to find the right parent.

One option would be to prevent Dirichlet groups ever being created for which the zeta order is not maximal for the given base ring. This would sort of work, but would restrict us to Dirichlet groups over integral domains (so we can guarantee that the group of roots of unity in the ring is cyclic) and it would make it a pain to implement base extension (at present the base_extend method for DirichletGroup_class just base extends the zeta element it already knows about.

The alternative is to be "honest" and make Dirichlet groups with the same modulus and base ring but different zeta order into genuinely different objects, comparing as unequal and with different string representations. One can then sort out coercion arithmetic using the machinery from sage.categories.pushout; I coded this up as an experiment (creating a class "DirichletGroupExtensionFunctor" whose effect was to extend Dirichlet groups by adding an nth root of unity into their value group). But it's a bit of a cheat since my "functors" aren't actually functors in any natural category I can think of.

@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-3.4.2 milestone Apr 23, 2009
@craigcitro
Copy link
Member

comment:4

I agree -- the business of zeta_order causes all sorts of confusion. However, I'm leaning the other direction -- I say we just eliminate it altogether from the defining data for a DirichletGroup. We could just avoid computing the zeta order in the ring until we need to, and when forced, just bite the bullet and do it. There will be several bonuses from doing this, and in particular, we can neatly clean up the trouble with basically-the-same-but-not-really-the-same groups, which is nice. The only potential downside I see is what you bring up in your comment -- if someone gives us a pretty ugly non-domain as the ring to work over (other than integers mod some N, where we can use other tricks to figure out the zeta order), we might spend time computing the zeta order. But that really seems like a rare case in my mind ... that said, it's 3am, so I won't make any promises about how my mind is operating. :)

So my vote: we commit this patch, and then open a new ticket to clean up this zeta_order business.

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented May 11, 2009

Attachment: trac_5792_rebase.patch.gz

replaces previous patch, apply over #4357 and #5250

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented May 11, 2009

comment:5

OK, I'm convinced. Positive review.

The patch has bit-rotted slightly, because I added some more doctests to dirichlet.py in #5727 and then edited it again in #5250. Also I am seeing a doctest failure in modform/ambient_eps.py -- nothing serious, the code expects a TypeError and gets a ValueError. I have done a rebased version which changes the doctest.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented May 11, 2009

comment:6

Merged trac_5792_rebase.patch in Sage 4.0.alpha0.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed May 11, 2009
@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 7, 2009

Reviewer: David Loeffler

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 7, 2009

Author: Craig Citro

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 7, 2009

Merged: 4.0.alpha0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant