Opened 9 years ago

Last modified 2 years ago

#12053 needs_work enhancement

Ideals in p-adic rings

Reported by: roed Owned by: roed
Priority: major Milestone: sage-6.4
Component: padics Keywords:
Cc: Merged in:
Authors: David Roe Reviewers:
Report Upstream: N/A Work issues:
Branch: u/saraedum/12053 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by davidloeffler)

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

Prerequisite for #12077, #8240.

Attachments (1)

12053.patch (13.9 KB) - added by roed 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by roed

  • Status changed from new to needs_review

Changed 9 years ago by roed

comment:2 Changed 9 years ago by saraedum

  • Status changed from needs_review to needs_info

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/?

comment:3 Changed 9 years ago by dmharvey

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.

comment:4 Changed 9 years ago by roed

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

comment:5 Changed 9 years ago by roed

  • Status changed from needs_info to needs_review

comment:6 Changed 9 years ago by roed

  • Status changed from needs_review to needs_work

comment:7 Changed 9 years ago by davidloeffler

  • Description modified (diff)

comment:8 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:9 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:10 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:11 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:12 Changed 2 years ago by saraedum

  • Branch set to u/saraedum/develop

comment:13 Changed 2 years ago by saraedum

  • Branch u/saraedum/develop deleted

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.

comment:14 Changed 2 years ago by saraedum

  • Branch set to u/saraedum/12053
Note: See TracTickets for help on using tickets.