New class NumberFieldZeroIdeal

[JohnCremona]

There are several issues with the zero ideal in number fields:

sage: K.<a> = NumberField(x^2-10)
sage: K.<a> = NumberField(x^2-10)
sage: 0/K.ideal(a)
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred in Sage.
This probably occurred because a *compiled* component of Sage has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Sage will now terminate.
------------------------------------------------------------------------

sage: K.<a> = NumberField(x^2-15)
sage: three = K.ideal(3)
sage: zero = K(0)
sage: three.divides(zero)
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred in Sage.
This probably occurred because a *compiled* component of Sage has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Sage will now terminate.
------------------------------------------------------------------------

sage: K.<a> = QuadraticField(-5) 
sage: K.ideal(0) * K.ideal(a+1,3)
...
PariError: incorrect type in basistoalg (t_MAT)

(and probably more)

I propose to do some redesign and add a new class NumberFieldZeroIdeal for just the zero ideal. Then most methods of the current NumberFieldIdeal should probably be moved to either or both of NumberFieldZeroIdeal/NumberFieldFractionalIdeal.

Depends on #18622

CC: @mmasdeu

Component: number fields

Author: John Cremona

Branch/Commit: u/cremona/18639 @ 4f60fb4

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

[JohnCremona]

Commit: 30ca395

[JohnCremona]

New commits:

30ca395

#18639: ideal divides 0

[JohnCremona]

Branch: u/cremona/18639

[jdemeyer]

comment:2

Wouldn't the proper fix to just allow (0) / I = (0) for a fractional ideal I? Can you check if #18622 does that?

[JohnCremona]

comment:3

OK, I will check that. Certainly the 0 ideal passes the is_integral() test, and that should be done anyway, as it is already p ossible to multiply the 0 idal with a nonzero (fractional) one.

[jdemeyer]

Dependencies: #186222

[jdemeyer]

comment:4

It's worse with #18622: it gives a segmentation fault.

In any case, I still think that allowing (0)/I is the proper fix here, so I'm boldly changing the ticket to do that.

[JohnCremona]

comment:5

I see that #18622 has been merged but is not in the current beta, so the branch name has disappeared from that ticket, so I am hoping that schecking out rremote branch trac/u/jdemeyer/ticket/18611 is the right thing to do.

[JohnCremona]

Changed dependencies from #186222 to #18622

[jdemeyer]

comment:6

Replying to @JohnCremona:

I see that #18622 has been merged but is not in the current beta, so the branch name has disappeared from that ticket, so I am hoping that schecking out rremote branch trac/u/jdemeyer/ticket/18611 is the right thing to do.

You can always use the commit hash, that should always work:

git fetch trac  # or just "git fetch" depending on your configuration
git checkout f61588ecabefc84103c4daa40e648b76e6e58828

[jdemeyer]

comment:7

There are actually many issues with the zero ideal...

[sagetrac-git]

Changed commit from 30ca395 to 7be3884

[sagetrac-git]

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

7be3884

#18639: handle division for 0 ideal

[jdemeyer]

comment:9

I hope you don't mind that I'm making this ticket much more general than you intended, but there is really not much point in fixing one specific method when maybe the whole approach needs to be changed.

I am now thinking about the following: introduce a new class NumberFieldZeroIdeal to represent the zero ideal. Then inherit both NumberFieldZeroIdeal and NumberFieldFractionalIdeal from NumberFieldIdeal (which shouldn't have much functionality).

[jdemeyer]

comment:10

Replying to @sagetrac-git:

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

7be3884

#18639: handle division for 0 ideal

Sorry, but this will conflict with #18622.

[JohnCremona]

comment:11

I have done what was suggested. Basically the __div__ function is moved from NumberFieldFractionalIdeal to NumberFieldIdeal with appropriate tests. The test from the original patch are included too, to show that the porioginal issue is resolved. I based this on my original branch but also tested it based on #18622.

I'm sure you will find something I did not do right and/or want to do more (I just saw your comment). Feel free to take this over from me 100% as I have other things to do today.

[jdemeyer]

comment:12

Replying to @JohnCremona:

but also tested it based on #18622.

Are you sure? I ask because #18622 actually changes __div__ to _div_ so I find it hard to believe that you didn't get a conflict.

Feel free to take this over from me 100% as I have other things to do today.

Me too :-)

What do you think about the approach I propose in [comment:9]?

[JohnCremona]

comment:13

Replying to @jdemeyer:

Replying to @JohnCremona:

but also tested it based on #18622.

Are you sure? I ask because #18622 actually changes __div__ to _div_ so I find it hard to believe that you didn't get a conflict.

Feel free to take this over from me 100% as I have other things to do today.

Me too :-)

I am sure you do! I am in the middle of implementing Kraus's condictoins and global minimal (or almost global minimal) models for elliptic curves over number fields of class number 1, which no-one has ever done before. Nearly finished...

What do you think about the approach I propose in [comment:9]?

Good idea. I think WIlliam and I considered this way back (see line 10 of the file in question -- it was in fact 28 Jan 2008, almost my first ever Sage development session, and took place in a hotel room in New Jersey where he and I had nothing else to do all day...)

[jdemeyer]

comment:14

Replying to @JohnCremona:

Good idea. I think WIlliam and I considered this way back (see line 10 of the file in question -- it was in fact 28 Jan 2008, almost my first ever Sage development session, and took place in a hotel room in New Jersey where he and I had nothing else to do all day...)

I know. The problem is that currently there is really no way to write special code for just the zero ideal. I also think that NumberFieldZeroIdeal is more explicit about its intentions, which is a good thing.

[sagetrac-git]

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

4f60fb4

Merge branch 'develop' into 18639

[sagetrac-git]

Changed commit from 7be3884 to 4f60fb4

[JohnCremona]

comment:18

Although this has become inactive, I thought it worth at least merging in the current develop branch and resolving conflicts (there were very few).

[joelrosenberg]

As of version 9.7 (python-3.10.5), asking for 0 / I no longer segfaults, but it still triggers a Pari error:

sage: K. = QuadraticField(5)
sage: J = K.ideal(1)
sage: 0 * J
"works"

sage: 0 / J
"TypeError: [;] has unsupported PARI type t_MAT"

sage: J1 = ~J
sage: 0 * J1
"same TypeError"

In some way the NumberFieldIdeal class should act like a monoid, with all invertible elements being 'NumberFieldFractionalIdeal's. Does making the zero ideal be in another subclass help the situation?

Joel