Opened 11 years ago
Closed 11 years ago
#11586 closed defect (fixed)
bug in p-adic extension norm method
Reported by: | William Stein | Owned by: | David Roe |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | padics | Keywords: | padic |
Cc: | Merged in: | sage-5.0.beta10 | |
Authors: | David Roe | Reviewers: | Jennifer Balakrishnan |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #8241 | Stopgaps: |
Description (last modified by )
R.<x> = QQ[] f = x^2 + 3*x + 1 K = Qp(7) M.<a> = K.extension(f) M(7).norm()
The above outputs 1 + O(7^20)
, which is completely wrong. The norm should be 7^2 + O(7^20)
, I think -- anyways, it should be divisible by 7.
The problem is that the actual code for the norm, which ones sees by typing a.norm??
assumes that the generator for M is in fact a generator for the maximal ideal of the ring of integers. However, this assumption is just completely false.
Typing a.norm???
we see
... norm_of_uniformizer = (-1)**self.parent().degree() * self.parent().defining_polynomial()[0] if self.valuation() == 0: return self.parent().ground_ring()(self.unit_part().matrix_mod_pn().det()) else: return self.parent().ground_ring()(self.unit_part().matrix_mod_pn().det()) * norm_of_uniformizer**self.valuation()
The above is wrong as mentioned above. Moreover, it is nonoptimal in that norm_of_uniformizer is computed but never used in the case when self.valuation()==0
.
This bug caused some confusion when computing with p-adic L-series for a research paper...
Apply 11586.patch to the Sage library.
Attachments (1)
Change History (10)
Changed 11 years ago by
Attachment: | 11586.patch added |
---|
comment:1 Changed 11 years ago by
Authors: | → David Roe |
---|---|
Dependencies: | → 8241 |
Keywords: | padic added |
Status: | new → needs_review |
comment:2 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
Great; this fixes the bug. All tests pass!
comment:3 Changed 11 years ago by
Dependencies: | 8241 → #8241 |
---|---|
Reviewers: | → Jennifer Balakrishnan |
comment:5 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:6 follow-up: 7 Changed 11 years ago by
Milestone: | sage-4.7.2 → sage-pending |
---|
Isn't this what we have a milestone "sage-pending" for?
comment:7 Changed 11 years ago by
Replying to davidloeffler:
Isn't this what we have a milestone "sage-pending" for?
I personally don't use "sage-pending" for that situation, since my script does dependency checking, so won't attempt to merge it, but considers it for inclusion, i.e., I'll see that it has positive review, and all of its dependencies with their state.
My interpretation (or use) of "sage-pending" is to postpone tickets for other reasons, so I currently only see that they're postponed, and don't consider them further.
I thought this was the original intent, to have some "formal" way to plan which tickets get merged when during management of a specific release, also since we don't have milestones for devel releases.
comment:8 Changed 11 years ago by
Milestone: | sage-pending → sage-5.0 |
---|
comment:9 Changed 11 years ago by
Merged in: | → sage-5.0.beta10 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Sorry about that. The attached patch should fix the problem (note that you need to apply #8241 first).