Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable polynomial content over padic fields #13619

Closed
saraedum opened this issue Oct 19, 2012 · 35 comments
Closed

Enable polynomial content over padic fields #13619

saraedum opened this issue Oct 19, 2012 · 35 comments

Comments

@saraedum
Copy link
Member

Currently, one cannot call content() for a polynomial defined over Qp:

sage: K = Qp(3)
sage: R.<t> = K[]
sage: t.content()
TypeError: ground ring is a field.  Answer is only defined up to units.

The intention is apparently to protect the user from calling content() if he mistakenly got from Zp into Qp, since the content there would be always zero or one. However, this can be annoying when writing algorithms which work over Zp and Qp but which have to take the content into account over Zp.

Additionally, the content shows a somewhat strange behaviour for zero polynomials::

sage: R = Zp(3)
sage: S.<t> = R[]
sage: f = S(R(0,3)); f
(O(3^3))
sage: f.is_zero()
True
sage: f.content()
3 + O(3^23)
sage: _.is_zero()
False

I don't think that the current behaviour is mathematically incorrect, but I believe it's not very intuitive.

Component: padics

Keywords: sd59, days71

Author: Julian Rüth

Branch: 880984a

Reviewer: Jeroen Demeyer, Aly Deines

Issue created by migration from https://trac.sagemath.org/ticket/13619

@saraedum saraedum added this to the sage-5.11 milestone Oct 19, 2012
@saraedum
Copy link
Member Author

Author: Julian Rueth

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@roed314
Copy link
Contributor

roed314 commented Oct 24, 2012

comment:4

Attachment: trac_13619.patch.gz

I'm happy with this. Let me know when you want a review.

@tscrim
Copy link
Collaborator

tscrim commented Nov 25, 2012

comment:5

As a quick informal review, the OUTPUT: block is indented one too many times.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@nilesjohnson
Copy link
Mannequin

nilesjohnson mannequin commented Jan 28, 2014

Branch: u/niles/ticket/13619

@nilesjohnson
Copy link
Mannequin

nilesjohnson mannequin commented Jan 28, 2014

comment:8

rebased and converted to git branch; changed indentation of OUTPUT: block, but no other changes


New commits:

3ace36aTrac #13619: Enable content for polynomials defined over Qp.

@nilesjohnson
Copy link
Mannequin

nilesjohnson mannequin commented Jan 28, 2014

Commit: 3ace36a

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@saraedum
Copy link
Member Author

Changed branch from u/niles/ticket/13619 to u/saraedum/ticket/13619

@saraedum
Copy link
Member Author

Changed keywords from none to sd59

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer
Copy link

Changed commit from 3ace36a to e57bdb5

@jdemeyer
Copy link

comment:14

Is there are reason you are only doing this for capped_relative_dense rings and not in the generic Polynomial_padic?


New commits:

e57bdb5Merge branch 'develop' into ticket/13619

@jdemeyer
Copy link

comment:15

This should be done for generic padics polynomials. I would prefer to get the same answer for Zp and Qp, i.e. p^k where k is the minimal valuation. That would be more useful than always getting 1 as answer over Qp.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer jdemeyer modified the milestones: sage-6.4, sage-6.5 Jan 23, 2015
@jdemeyer
Copy link

comment:20

Replying to @saraedum:

What should be the answer if a coefficient has negative valuation?

p^k where k is the minimal valuation of the coefficients.

I would rather stick to the usual definition of content.

The "usual" definition doesn't care about the unit part, so answering p^k is a more useful and equally correct answer over Qp.

@saraedum
Copy link
Member Author

comment:21

Replying to @jdemeyer:

I think it's certainly not a good idea that content() sometimes returns an element and sometimes returns an ideal. Ideally, there would be two methods:

  • content() returning an ideal in the mathematical sense
  • content_gen() returning an element, preferably a generator of the fractional ideal generated by the coefficients.

What is the value of content_gen()? It is almost always equivalent to content().gen() unless in some cases where both should raise an error?

@saraedum
Copy link
Member Author

comment:22

Replying to @jdemeyer:

Replying to @saraedum:

What should be the answer if a coefficient has negative valuation?

p^k where k is the minimal valuation of the coefficients.

I would rather stick to the usual definition of content.

The "usual" definition doesn't care about the unit part, so answering p^k is a more useful and equally correct answer over Qp.

Ok. You are right. I'll change it.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 18, 2015

Changed commit from e57bdb5 to 55e1b96

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 18, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

dc1dceeMerge branch 'develop' into t/13619/ticket/13619
55e1b96changed content of p-adic polynomials over Qp

@jdemeyer
Copy link

comment:25

See [comment:14]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

355c144Merge branch 'develop' into t/13619/ticket/13619

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2016

Changed commit from 55e1b96 to 355c144

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 17, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

880984aMove capped relative content() implementation to the generic p-adic polynomial class.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 17, 2016

Changed commit from 355c144 to 880984a

@adeines
Copy link
Mannequin

adeines mannequin commented Mar 19, 2016

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Aly Deines

@adeines
Copy link
Mannequin

adeines mannequin commented Mar 19, 2016

comment:29

Looks good to me.

@vbraun
Copy link
Member

vbraun commented Mar 20, 2016

Changed branch from u/saraedum/ticket/13619 to 880984a

@adeines
Copy link
Mannequin

adeines mannequin commented Mar 23, 2016

Changed commit from 880984a to none

@adeines
Copy link
Mannequin

adeines mannequin commented Mar 23, 2016

Changed keywords from sd59 to sd59, sd71

@adeines
Copy link
Mannequin

adeines mannequin commented Mar 23, 2016

Changed keywords from sd59, sd71 to sd59, days71

@jdemeyer
Copy link

Changed author from Julian Rueth to Julian Rüth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants