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

primes_of_degree_one is broken for relative extensions #6396

Closed
loefflerd mannequin opened this issue Jun 24, 2009 · 15 comments
Closed

primes_of_degree_one is broken for relative extensions #6396

loefflerd mannequin opened this issue Jun 24, 2009 · 15 comments

Comments

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Jun 24, 2009

This is kind of irritating:

sage: N.<a,b> = NumberField([x^2 + 1, x^2 - 5])
sage: N.primes_of_degree_one_list(10)
[Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1),
 Fractional ideal (1)]

CC: @ncalexan

Component: number theory

Author: David Loeffler

Reviewer: Nick Alexander, Minh Van Nguyen

Merged: sage-4.1.1.alpha0

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

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 24, 2009

comment:1

Here's a patch. Turns out that the bug was due to using the wrong generator. I've set it to use absolute_generator rather than gen, and patched absolute number fields so absolute_generator is an alias for gen in that case. The patch also ReSTifies small_primes_of_degree_one.py and adds it to the reference manual.

(Nick, since he wrote the original code.)

David

@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Jun 24, 2009

comment:2

This looks good to me, and I am cautiously optimistic that the method will select the same generator on all architectures. Apply.

@ncalexan ncalexan mannequin changed the title primes_of_degree_one is broken for relative extensions [with path] primes_of_degree_one is broken for relative extensions Jun 24, 2009
@ncalexan ncalexan mannequin added the s: positive review label Jun 24, 2009
@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Jun 24, 2009

Reviewer: Nick Alexander

@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Jun 24, 2009

Author: David Loeffler

@ncalexan ncalexan mannequin added this to the sage-4.1 milestone Jun 24, 2009
@loefflerd loefflerd mannequin changed the title [with path] primes_of_degree_one is broken for relative extensions primes_of_degree_one is broken for relative extensions Jun 24, 2009
@boothby
Copy link

boothby commented Jun 26, 2009

comment:5

Doctest failures when applied to 4.1.alpha1

sage -t -long devel/sage/sage/rings/number_field/small_primes_of_degree_one.py
**********************************************************************
File "/space/boothby/sage-4.1.alpha1/devel/sage-main/sage/rings/number_field/small_primes_of_degree_one.py", line 204:
    sage: N.primes_of_degree_one_list(10)
Expected:
    [Fractional ideal ((-1/2*b + 1/2)*a + 2),
     Fractional ideal (-b*a + 1/2*b + 1/2),
     Fractional ideal ((1/2*b + 3/2)*a - b),
     Fractional ideal ((-1/2*b - 3/2)*a + b - 1),
     Fractional ideal (-b*a - b + 1),
     Fractional ideal (3*a + 1/2*b - 1/2),
     Fractional ideal ((-3/2*b + 1/2)*a + 1/2*b - 1/2),
     Fractional ideal ((-1/2*b - 5/2)*a - b + 1),
     Fractional ideal (2*a - 3/2*b - 1/2),
     Fractional ideal (3*a + 1/2*b + 5/2)]
Got:
    [Fractional ideal (2*a + 1/2*b - 1/2), Fractional ideal ((-1/2*b - 1/2)*a - b), Fractional ideal (b*a + 1/2*b + 3/2), Fractional ideal ((-1/2*b - 3/2)*a + b - 1), Fractional ideal ((-b + 1)*a + b), Fractional ideal (3*a + 1/2*b - 1/2), Fractional ideal ((1/2*b - 1/2)*a + 3/2*b - 1/2), Fractional ideal ((-1/2*b - 5/2)*a - b + 1), Fractional ideal (2*a - 3/2*b - 1/2), Fractional ideal (3*a + 1/2*b + 5/2)]
**********************************************************************
1 items had failures:
   1 of   8 in __main__.example_5
***Test Failed*** 1 failures.
For whitespace errors, see the file /space/boothby/sage-4.1.alpha1/tmp/.doctest_small_primes_of_degree_one.py
         [3.2 s]

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 26, 2009

comment:6

Aargh! Every damn thing we do with ideals seems to return different answers on different platforms -- I've never understood why this is and it's fantastically annoying. The ideals are the same ones, of course, but their string representations are different because Pari picks generators in a totally unpredictable way.

I'll fix this when I get around to it (probably after SD16 is over)

David

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 26, 2009

Attachment: trac_6396-deg1primes.patch.gz

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 26, 2009

comment:7

Can you try this new patch and see if it works better for you? It works on my machine, but then so did the last one, and there is no sage.math binary of 4.0.2 available AFAIK.

@loefflerd loefflerd mannequin added s: needs review and removed s: needs work labels Jun 26, 2009
@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jun 27, 2009

comment:9

BTW: having built myself a 4.1.alpha1 on sage.math overnight, I've checked that it passes doctests there too. Can I just reinstate the positive review now, or does it need to be re-reviewed?

@loefflerd
Copy link
Mannequin Author

loefflerd mannequin commented Jul 14, 2009

comment:10

Hello? I'm marking this as "positive review" so it comes to the attention of the release managers for 4.1.1, who can decide as they see fit what to do given the slightly ambiguous status. All that's needed is for someone to check that the doctests pass on both 64-bit and 32-bit.

(I really would prefer it if this patch didn't hang around bitrotting forever -- this is necessary for a major project I'm working on, which seems to have exposed a remarkable number of bugs; see also #6457, #6458, #6462 and #6463.)

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

Attachment: trac_6396-reviewer.patch.gz

reviewer patch; fix minor docstring formatting

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

Changed reviewer from Nick Alexander to Nick Alexander, Minh Van Nguyen

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

comment:11

The patch trac_6396-reviewer.patch makes some adjustment to the docstring introduced by trac_6396-deg1primes.patch, and fixes some typos therein. All tests passed on sage.math (a 64-bit machine) and on my 32-bit Debian Lenny machine. Just to let people know, both patches have been merged in sage-4.1.1-alpha0. I can't close this ticket because I don't have the privilege to do so. Sorry, folks :-(

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

Merged: sage-4.1.1.alpha0

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Jul 16, 2009
@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 22, 2009

comment:13

See #6806 for a follow up to this ticket.

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

2 participants