#4715 closed defect (fixed)
[with patch; positive review] Two small bugs in KodairaSymbol
Reported by: | cremona | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-3.2.2 |
Component: | number theory | Keywords: | |
Cc: | tnagel | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
#4412 had a buglet: for Kodaira Class Im the _roman field was not being set (it should be 1). This is only currently used in the tamagawa_exponent() function for elliptic curves over number fields.
One-line patch coming up, plus a corresponding doctest.
This was reported by Tobias Nagel:
sage: E=EllipticCurve('117a3'); sage: E.tamagawa_exponent(13) --------------------------------------------------------------------------- AttributeError Traceback (most recent call last) /home/tobi/test_Sint/<ipython console> in <module>() /home/tobi/sage/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_rational_field.pyc in tamagawa_exponent(self, p) 2190 return cp 2191 ks = self.kodaira_type(p) -> 2192 if ks._roman==1 and ks._n%2==0 and ks._starred: 2193 return 2 2194 return 4 AttributeError: 'KodairaSymbol_class' object has no attribute '_roman'
Attachments (2)
Change History (10)
Changed 12 years ago by
comment:1 Changed 12 years ago by
- Cc tnagel added
- Summary changed from Small bug in KodairaSymbol to [with patch, needs review] Small bug in KodairaSymbol
comment:2 Changed 12 years ago by
- Milestone set to sage-3.2.2
comment:3 Changed 12 years ago by
- Summary changed from [with patch, needs review] Small bug in KodairaSymbol to [with patch, not ready for review] Small bug in KodairaSymbol
Changed 12 years ago by
comment:4 Changed 12 years ago by
- Summary changed from [with patch, not ready for review] Small bug in KodairaSymbol to [with patch, needs review] Two small bugs in KodairaSymbol
Fixing that also showed up the following completely independent bug (only on 32-bit machines though):
sage: E=EllipticCurve('903b3') sage: E.pari_curve() <boom> (PariError: precision too low)
The second patch fixes that as well as the other (which only applied to type I*0). Now I have checked tamagawa_index() for all curves in the database up to conductor 10000 and all bad primes for each, so I hope that's that.
To fix the pari precision problem I did a try/except which keeps doubling the precision until it's ok. I hope that is not against the rules: if pari's ellinit every crashes for a reason other than precision, this would be an infinite loop.
In the course of this testing I found that looping through thousands of curves ate up a lot of memory. I made a change so that for curves over QQ, local_data() uses prime integers arther than prime ideals, and that helps a bit, but there is still more memory begin eaten up than I would like. For example:
sage: for e in cremona_curves(srange(11,10000)): for p in e.conductor().support(): ld = e.local_data(p) print e.cremona_label()
On my machine the used RAM creeps up gradually, hits 1GB at around conductor 2400, and if I let it continue it starts to make my machine really suffer at 1.7GB (no prizes for guessing the amount of RAM I have).
This might deserve a separate ticket.
comment:5 Changed 12 years ago by
Hi John,
please open a ticket for the memory issues/leaks you are seeing. We are current chasing a number of leaks, most of which seem coercion related.
Cheers,
Michael
comment:6 Changed 12 years ago by
- Summary changed from [with patch, needs review] Two small bugs in KodairaSymbol to [with patch; positive review] Two small bugs in KodairaSymbol
comment:7 Changed 12 years ago by
- Resolution set to fixed
- Status changed from new to closed
Merged both patches in Sage 3.2.2.alpha1
comment:8 Changed 10 years ago by
- Report Upstream set to N/A
In #9931, I plan to revert this ticket's patch to sage/schemes/elliptic_curves/ell_rational_field.py
, since the workaround seems not needed anymore.
There's another problem, watch this space: