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

Regression in Pari finite field interface #16540

Closed
defeo opened this issue Jun 25, 2014 · 10 comments
Closed

Regression in Pari finite field interface #16540

defeo opened this issue Jun 25, 2014 · 10 comments

Comments

@defeo
Copy link
Member

defeo commented Jun 25, 2014

In Sage 6.2

sage: GF(11^23, 'a').gen()^Zmod(11)(1)
1

In 5.9 this used to give

sage: GF(11^23, 'a').gen()^Zmod(11)(1)
a

CC: @pjbruin

Component: finite rings

Keywords: finite_field regression pari

Author: Peter Bruin

Branch/Commit: 5f6810f

Reviewer: Jeroen Demeyer

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

@defeo defeo added this to the sage-6.3 milestone Jun 25, 2014
@jdemeyer
Copy link

comment:1

Do you agree that the correct behaviour is a ValueError?

@pjbruin
Copy link
Contributor

pjbruin commented Jun 26, 2014

comment:3

PARI's FF_pow requires the exponent to be a t_INT, but does not check the type, and FiniteFieldElement_pari_ffelt.__pow__() does not enforce this either. (Actually it also assumes the exponent to be an integer, since it checks if it is negative.) We could simply convert the exponent into an Integer; the only drawback is that we would have to do a separate check for IntegerMod with an incorrect modulus if we want to raise a ValueError in that case.

@defeo
Copy link
Member Author

defeo commented Jun 27, 2014

comment:4

Replying to @jdemeyer:

Do you agree that the correct behaviour is a ValueError?

I agree that mathematically the operation does not make sense, unless the modulus is equal to the order of the element (but computing it would be overkill in my opinion) or a multiple (the only reasonable one being cardinality - 1).

Now, if we want to raise a ValueError, this is not a Pari issue anymore: it is a huge issue in basically any component of Sage!

sage: 3^Zmod(3)(2)
9
sage: GF(101)(2)^Zmod(3)(2)
4
sage: QQ['x'].gen()^Zmod(3)(2)
x^2
sage: i^Zmod(3)(2)
-1
sage: AbelianGroup([10])[1]^Zmod(3)(2)
f^2

etc...

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@pjbruin
Copy link
Contributor

pjbruin commented Aug 13, 2014

Author: Peter Bruin

@pjbruin
Copy link
Contributor

pjbruin commented Aug 13, 2014

@pjbruin
Copy link
Contributor

pjbruin commented Aug 13, 2014

Commit: 5f6810f

@pjbruin
Copy link
Contributor

pjbruin commented Aug 13, 2014

comment:6

Here is a branch to make FiniteFieldElement_pari_ffelt.__pow__() always convert the exponent to an integer. This fixes the following mathematically wrong answer to return a:

sage: q = 11^23
sage: F.<a> = FiniteField(q)
sage: a^Mod(1, q - 1)
1

The new code actually seems to be marginally faster than the old one (about 10% on this example). It also adds a warning to the documentation that we do not check for well-definedness before converting the exponent to an integer.

@vbraun
Copy link
Member

vbraun commented Aug 18, 2014

comment:8

Reviewer name

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Aug 19, 2014

Changed branch from u/pbruin/16540-finite_field_exponentiation to 5f6810f

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