Opened 10 years ago

Closed 7 years ago

#11554 closed defect (fixed)

Fix for denominator_ideal function

Reported by: bleveque Owned by: davidloeffler
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: number fields Keywords: denominator, ideal, number field
Cc: wstein Merged in:
Authors: Reviewers: Ben LeVeque, William Stein, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by bleveque)

The denominator_ideal of 0 should return the ideal (1), but it currently returns a ValueError.

Attachments (1)

trac_11554.patch (2.0 KB) - added by bleveque 10 years ago.
fixes denominator_ideal for input 0

Download all attachments as: .zip

Change History (20)

Changed 10 years ago by bleveque

fixes denominator_ideal for input 0

comment:1 Changed 10 years ago by bleveque

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by bleveque

  • Description modified (diff)

comment:3 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-4.7.2 to sage-duplicate/invalid/wontfix

The problem really is that Sage only supports the zero ideal and fractional ideals of number fields. Sage simply does not know about the ideal (1) in a number field.

So proposing to close this as "wontfix".

comment:4 Changed 10 years ago by davidloeffler

  • Status changed from needs_review to positive_review

I'm with Jeroen on this. I think ValueError? is the right behaviour here. Positive review to the proposal to close as wontfix.

comment:5 Changed 10 years ago by jdemeyer

  • Authors bleveque deleted
  • Resolution set to wontfix
  • Reviewers set to David Loeffler
  • Status changed from positive_review to closed

comment:6 follow-up: Changed 10 years ago by was

  • Resolution wontfix deleted
  • Status changed from closed to new

Huh? Obviously, the denominator ideal of 0 should be the unit ideal, just at the denominator if 0 is 1:

sage: K.<a> = NumberField(x^2 + 1)
sage: K(0).denominator()
1

The statement "Sage only supports the zero ideal and fractional ideals of number fields. Sage simply does not know about the ideal (1) in a number field." sounds like nonsense to me. The ideal generated by 1 is a perfectly good fractional ideal. That's why people say "the *group* of fractional ideals" -- groups have an identity element.

sage: K.maximal_order().ideal(1)
Fractional ideal (1)

comment:7 Changed 10 years ago by was

  • Status changed from new to needs_review

Also, I give this patch a positive review.

comment:8 Changed 10 years ago by was

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-4.8
  • Status changed from needs_review to positive_review

comment:9 follow-up: Changed 10 years ago by davidloeffler

  • Priority changed from major to minor

Sage simply does not know about the ideal (1) in a number field." sounds like nonsense to me. The ideal generated by 1 is a perfectly good fractional ideal.

What I believe Jeroen meant here is not the OK-submodule OK of K, but the K-submodule K of K -- the unit ideal of K in the purely ring-theoretic sense. This is indeed an object Sage genuinely doesn't know about -- we special-case the zero ideal, but not the whole-of-K ideal.

This is just FYI; I'm not trying to dispute your decision to give the patch a positive review. Frankly I'm not convinced that the denominator of 0 is worth losing much sleep over.

comment:10 Changed 10 years ago by davidloeffler

  • Authors set to Ben LeVeque
  • Reviewers changed from David Loeffler to William Stein

comment:11 in reply to: ↑ 6 Changed 10 years ago by jdemeyer

Please DO NOT reopen tickets!

If you disagree with me closing the ticket, that's fine, just state it in the comments.

But DO NOT reopen tickets!

You can totally disagree with me about closing a ticket. I am a reasonable person and will very likely listen. But it's very annoying if my idea of which tickets are open/closed does not correspond to reality.

So, DO NOT reopen tickets!

comment:12 in reply to: ↑ 9 Changed 10 years ago by jdemeyer

  • Reviewers changed from William Stein to William Stein, Jeroen Demeyer
  • Status changed from positive_review to needs_work

Replying to davidloeffler:

What I believe Jeroen meant here is not the OK-submodule OK of K, but the K-submodule K of K -- the unit ideal of K in the purely ring-theoretic sense. This is indeed an object Sage genuinely doesn't know about -- we special-case the zero ideal, but not the whole-of-K ideal.

This is what I meant, yes. But it is indeed mostly matter of definition.

I don't understand why the patch changes the documentation of the function. I think the old documentation is more clear and I think it also justifies better the choice that the numerator of (0) is the fractional ideal (1).

However, if you want to define the denominator_ideal of zero to be the fractional ideal (1), then certainly the numerator_ideal of zero should be (0). So this still needs work.

comment:13 Changed 10 years ago by bleveque

Hello,

In response to Jeroen's comment, I opened a new ticket -- http://trac.sagemath.org/sage_trac/ticket/12046 -- with a proposed change to the numerator_ideal function. Sorry if it's confusing having two tickets dealing with a similar issue.

-Ben

comment:14 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:15 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:16 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:17 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:18 Changed 7 years ago by jdemeyer

  • Authors Ben LeVeque deleted
  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Reviewers changed from William Stein, Jeroen Demeyer to Ben LeVeque, William Stein, Jeroen Demeyer
  • Status changed from needs_work to positive_review

Fixed by #12046.

comment:19 Changed 7 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.