Opened 3 years ago
Closed 2 years ago
#20244 closed defect (fixed)
root_field() does not work for p-adic fields
Reported by: | saraedum | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.2 |
Component: | padics | Keywords: | days71, beginner, sd87 |
Cc: | Merged in: | ||
Authors: | Immi Halupczok | Reviewers: | David Roe, Aly Deines |
Report Upstream: | N/A | Work issues: | |
Branch: | 66ba958 (Commits) | Commit: | 66ba9582ed3226655276322c368bfdaa5687552b |
Dependencies: | #20254 | Stopgaps: |
Description (last modified by )
Currently it creates a quotient of a polynomial ring over the p-adics. It should call extension()
to create a proper extension instead.
sage: R.<x> = Qp(3)[] sage: (x^3+3).root_field('a') Univariate Quotient Polynomial Ring in a over 3-adic Field with capped relative precision 20 with modulus (1 + O(3^20))*x^3 + (3 + O(3^21))
Also the documentation is somewhat confusing. It should be made clear that this is not the same as splitting_field()
.
Change History (15)
comment:1 Changed 3 years ago by
- Description modified (diff)
comment:2 Changed 3 years ago by
- Branch set to u/immi/root_field___does_not_work_for_p_adic_fields
comment:3 Changed 3 years ago by
- Commit set to 66ba9582ed3226655276322c368bfdaa5687552b
- Status changed from new to needs_review
comment:4 Changed 3 years ago by
- Reviewers set to David Roe
- Status changed from needs_review to positive_review
Looks good. I doubt that anywhere else in Sage was using root_field for p-adic polynomials, since polynomial quotient rings have so little functionality.
comment:5 Changed 3 years ago by
Maybe, in to avoid rewriting doctest twice, it would be good to put this in after #20254.
comment:6 Changed 3 years ago by
Why not have #20254 depend on this? With this this change, using root_field
should be perfectly fine.
comment:7 Changed 3 years ago by
If I were to do both I would do it the other way around to save me the work of changing the doctests in padic_lseries twice. But if this is urgent, then just change the output there and I adapt mine on top of this.
comment:8 Changed 3 years ago by
I don't understand why you would need to change the doctests in padic_lseries
twice. Just because root_field works now doesn't mean you need to use it....
Can you push a branch to #20254 so I can look at the doctests you're referring to?
comment:9 Changed 3 years ago by
if you change root_field
the printing of the p-adic L-functions of elliptic curves will change. I found some errors and they will actually change by some factor 2, too. So they will be changed twice.
It doesn't matter. By now I have written more then the double change will cause :)
comment:10 Changed 3 years ago by
sage -t --long src/sage/misc/cachefunc.pyx # 1 doctest failed sage -t --long src/sage/schemes/elliptic_curves/padic_lseries.py # 7 doctests failed sage -t --long src/sage/schemes/elliptic_curves/sha_tate.py # 1 doctest failed
comment:11 Changed 3 years ago by
- Status changed from positive_review to needs_work
comment:12 Changed 3 years ago by
- Dependencies set to #20254
I believe that applying this ticket after #20254 should get rid of the doctest problems found above. I will run the test later today.
comment:13 Changed 2 years ago by
- Keywords sd87 added
comment:14 Changed 2 years ago by
- Reviewers changed from David Roe to David Roe, Aly Deines
- Status changed from needs_work to positive_review
The doctests that failed above now pass (all doctest now pass.)
comment:15 Changed 2 years ago by
- Branch changed from u/immi/root_field___does_not_work_for_p_adic_fields to 66ba9582ed3226655276322c368bfdaa5687552b
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
fixed root_field for polynomials over Qp