Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#5792 closed defect (fixed)

[with patch, positive review] Dirichlet character bug

Reported by: davidloeffler Owned by: craigcitro
Priority: major Milestone: sage-4.0
Component: modular forms Keywords: dirichlet characters
Cc: Merged in: 4.0.alpha0
Authors: Craig Citro Reviewers: David Loeffler
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges


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/ 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)
    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.

Attachments (2)

trac-5792.patch (3.3 KB) - added by craigcitro 14 years ago.
trac_5792_rebase.patch (3.1 KB) - added by davidloeffler 14 years ago.
replaces previous patch, apply over #4357 and #5250

Download all attachments as: .zip

Change History (9)

comment:1 Changed 14 years ago by craigcitro

Summary: Dirichlet character bug[with patch, needs review] Dirichlet character bug

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()

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()

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 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.

Changed 14 years ago by craigcitro

Attachment: trac-5792.patch added

comment:2 Changed 14 years ago by davidloeffler

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.

comment:3 Changed 14 years ago by mabshoff

Milestone: sage-3.4.2

comment:4 Changed 14 years ago by craigcitro

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.

Changed 14 years ago by davidloeffler

Attachment: trac_5792_rebase.patch added

replaces previous patch, apply over #4357 and #5250

comment:5 Changed 14 years ago by davidloeffler

Summary: [with patch, needs review] Dirichlet character bug[with patch, positive review] Dirichlet character bug

OK, I'm convinced. Positive review.

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

comment:6 Changed 14 years ago by mabshoff

Resolution: fixed
Status: newclosed

Merged trac_5792_rebase.patch in Sage 4.0.alpha0.



comment:7 Changed 14 years ago by davidloeffler

Authors: Craig Citro
Merged in: 4.0.alpha0
Reviewers: David Loeffler
Note: See TracTickets for help on using tickets.