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

avoid constructing list of all base-field elements in QuaternionAlgebra_ab.modp_splitting_data() #34355

Closed
yyyyx4 opened this issue Aug 13, 2022 · 5 comments

Comments

@yyyyx4
Copy link
Member

yyyyx4 commented Aug 13, 2022

This patch is from Rémy Oudompheng:

diff --git a/src/sage/algebras/quatalg/quaternion_algebra.py b/src/sage/algebras/quatalg/quaternion_algebra.py
index 3bde7b2153..3e12785e14 100644
--- a/src/sage/algebras/quatalg/quaternion_algebra.py
+++ b/src/sage/algebras/quatalg/quaternion_algebra.py
@@ -1232,7 +1232,7 @@ class QuaternionAlgebra_ab(QuaternionAlgebra_abstract):
             raise NotImplementedError("algorithm for computing local splittings not implemented in general (currently require the first invariant to be coprime to p)")
         i2inv = ~i2
         a = None
-        for b in list(F):
+        for b in F:
             if not b:
                 continue
             c = j2 + i2inv * b*b

Obviously, constructing a list of all elements is a very bad idea for non-tiny base fields.

CC: @slel @tscrim

Component: algebra

Author: Rémy Oudompheng

Branch/Commit: 91d3645

Reviewer: Travis Scrimshaw

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

@yyyyx4 yyyyx4 added this to the sage-9.7 milestone Aug 13, 2022
@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 13, 2022

Commit: 91d3645

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 13, 2022

Branch: public/34355

@tscrim
Copy link
Collaborator

tscrim commented Aug 15, 2022

comment:3

Indeed, indeed. LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Aug 15, 2022

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Aug 30, 2022

Changed branch from public/34355 to 91d3645

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

3 participants