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

Ideals in p-adic rings #12053

Open
roed314 opened this issue Nov 19, 2011 · 8 comments
Open

Ideals in p-adic rings #12053

roed314 opened this issue Nov 19, 2011 · 8 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Nov 19, 2011

Adds a new class for ideals in discrete valuation rings that improves speed and comparison.

Prerequisite for #12077, #8240.

Component: padics

Author: David Roe

Branch: u/saraedum/12053

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

@roed314 roed314 added this to the sage-5.11 milestone Nov 19, 2011
@roed314 roed314 self-assigned this Nov 19, 2011
@saraedum
Copy link
Member

comment:2

Attachment: 12053.patch.gz

Reviewing this I wondered why this is in the padics/ directory. Of course, the main application right now would be over the padics but it seems that everything would work like that for any DVR so should it probably be moved to rings/?

@sagetrac-dmharvey
Copy link
Mannequin

sagetrac-dmharvey mannequin commented Feb 23, 2012

comment:3

What is reduce() supposed to do? I don't understand that function.

This is a little alarming:

sage: ~R.ideal(5) * 5
Fractional ideal (5^-1) of 5-adic Field with capped relative precision 20

But I see fractional ideals are not implemented. Perhaps there should be a NotImplementedError somewhere rather than manifestly incorrect behaviour. Is this fixed in a later patch?

I agree with saraedum's comment above, but for the moment it wouldn't hurt for it to live in padics/, especially as all that code is under heavy active development at the moment.

It wouldn't hurt to add some doctests to cover some unramified and ramified extensions of Zp (I tried a few myself and everything seems to work).

Apart from these issues the patch looks good to me.

@roed314
Copy link
Contributor Author

roed314 commented Feb 23, 2012

comment:4

I agree that ~R.ideal(5) * 5 is a bug. I'll try to fix it soon.

@loefflerd

This comment has been minimized.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@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
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@saraedum
Copy link
Member

saraedum commented Oct 5, 2018

Branch: u/saraedum/develop

@saraedum
Copy link
Member

saraedum commented Oct 5, 2018

Changed branch from u/saraedum/develop to none

@saraedum
Copy link
Member

saraedum commented Oct 5, 2018

comment:13

Not sure whether you still think that this code is useful…I turned it into a branch so it is easier to see what's going on.

@saraedum
Copy link
Member

saraedum commented Oct 5, 2018

Branch: u/saraedum/12053

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

4 participants