Opened 9 years ago
Closed 8 years ago
#11586 closed defect (fixed)
bug in p-adic extension norm method
Reported by: | was | Owned by: | roed |
---|---|---|---|
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 9 years ago by
comment:1 Changed 9 years ago by
- Dependencies set to 8241
- Keywords padic added
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Status changed from needs_review to positive_review
Great; this fixes the bug. All tests pass!
comment:3 Changed 9 years ago by
- Dependencies changed from 8241 to #8241
- Reviewers set to Jennifer Balakrishnan
comment:5 Changed 9 years ago by
- Description modified (diff)
comment:6 follow-up: ↓ 7 Changed 9 years ago by
- Milestone changed from sage-4.7.2 to sage-pending
Isn't this what we have a milestone "sage-pending" for?
comment:7 in reply to: ↑ 6 Changed 9 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 8 years ago by
- Milestone changed from sage-pending to sage-5.0
comment:9 Changed 8 years ago by
- Merged in set to sage-5.0.beta10
- Resolution set to fixed
- Status changed from positive_review to closed
Sorry about that. The attached patch should fix the problem (note that you need to apply #8241 first).