Add `terms` kwarg to irreducible and primitive poly functions #463

mhostetter · 2023-01-17T21:37:43Z

Implements #458.

Examples

In [1]: import galois

In [2]: galois.irreducible_poly(7, 3)
Out[2]: Poly(x^3 + 2, GF(7))

In [3]: galois.irreducible_poly(7, 3, terms=3)
Out[3]: Poly(x^3 + x + 1, GF(7))

In [4]: galois.primitive_poly(7, 3)
Out[4]: Poly(x^3 + 3x + 2, GF(7))

In [5]: galois.primitive_poly(7, 3, terms=4)
Out[5]: Poly(x^3 + x^2 + x + 2, GF(7))

In [1]: import galois

In [2]: list(galois.primitive_polys(3, 5))
Out[2]: 
[Poly(x^5 + 2x + 1, GF(3)),
 Poly(x^5 + 2x^2 + x + 1, GF(3)),       
 Poly(x^5 + x^3 + x + 1, GF(3)),        
 Poly(x^5 + x^3 + 2x^2 + 1, GF(3)),     
 Poly(x^5 + x^3 + 2x^2 + 2x + 1, GF(3)),
 Poly(x^5 + 2x^3 + x^2 + 1, GF(3)),     
 Poly(x^5 + 2x^3 + 2x^2 + x + 1, GF(3)),
 Poly(x^5 + x^4 + 2x + 1, GF(3)),
 Poly(x^5 + x^4 + x^2 + 1, GF(3)),
 Poly(x^5 + x^4 + x^2 + x + 1, GF(3)),
 Poly(x^5 + x^4 + x^3 + x + 1, GF(3)),
 Poly(x^5 + x^4 + x^3 + x^2 + 2x + 1, GF(3)),
 Poly(x^5 + x^4 + x^3 + 2x^2 + x + 1, GF(3)),
 Poly(x^5 + x^4 + 2x^3 + 1, GF(3)),
 Poly(x^5 + x^4 + 2x^3 + x^2 + x + 1, GF(3)),
 Poly(x^5 + x^4 + 2x^3 + 2x^2 + 1, GF(3)),
 Poly(x^5 + 2x^4 + 1, GF(3)),
 Poly(x^5 + 2x^4 + x + 1, GF(3)),
 Poly(x^5 + 2x^4 + 2x^2 + 2x + 1, GF(3)),
 Poly(x^5 + 2x^4 + x^3 + x^2 + x + 1, GF(3)),
 Poly(x^5 + 2x^4 + 2x^3 + 2x + 1, GF(3)),
 Poly(x^5 + 2x^4 + 2x^3 + x^2 + 1, GF(3))]

In [3]: list(galois.primitive_polys(3, 5, terms=4))
Out[3]: 
[Poly(x^5 + 2x^2 + x + 1, GF(3)),
 Poly(x^5 + x^3 + x + 1, GF(3)),
 Poly(x^5 + x^3 + 2x^2 + 1, GF(3)),
 Poly(x^5 + 2x^3 + x^2 + 1, GF(3)),
 Poly(x^5 + x^4 + 2x + 1, GF(3)),
 Poly(x^5 + x^4 + x^2 + 1, GF(3)),
 Poly(x^5 + x^4 + 2x^3 + 1, GF(3)),
 Poly(x^5 + 2x^4 + x + 1, GF(3))]

In [4]: list(galois.primitive_polys(3, 5, terms="min"))
Out[4]: [Poly(x^5 + 2x + 1, GF(3)), Poly(x^5 + 2x^4 + 1, GF(3))]

Examples related to #464

In [1]: import galois

In [2]: galois.primitive_poly(2, 18)
Out[2]: Poly(x^18 + x^5 + x^2 + x + 1, GF(2))

In [3]: galois.primitive_poly(2, 18, terms="min")
Out[3]: Poly(x^18 + x^7 + 1, GF(2))

In [4]: galois.primitive_poly(2, 32)
Out[4]: Poly(x^32 + x^7 + x^5 + x^3 + x^2 + x + 1, GF(2))

In [5]: galois.primitive_poly(2, 32, terms="min")
Out[5]: Poly(x^32 + x^7 + x^6 + x^2 + 1, GF(2))

In [6]: galois.primitive_poly(2, 33)
Out[6]: Poly(x^33 + x^6 + x^4 + x + 1, GF(2))

In [7]: galois.primitive_poly(2, 33, terms="min")
Out[7]: Poly(x^33 + x^13 + 1, GF(2))

In [8]: galois.primitive_poly(2, 34)
Out[8]: Poly(x^34 + x^7 + x^6 + x^5 + x^2 + x + 1, GF(2))

In [9]: galois.primitive_poly(2, 34, terms="min")
Out[9]: Poly(x^34 + x^8 + x^4 + x^3 + 1, GF(2))

codecov · 2023-01-17T21:53:50Z

Codecov Report

Base: 96.39% // Head: 96.40% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (32de045) compared to base (5e45744).
Patch coverage: 98.71% of modified lines in pull request are covered.

Additional details and impacted files

@@                Coverage Diff                @@
##           release/0.3.x     #463      +/-   ##
=================================================
+ Coverage          96.39%   96.40%   +0.01%     
=================================================
  Files                 43       43              
  Lines               5624     5701      +77     
=================================================
+ Hits                5421     5496      +75     
- Misses               203      205       +2

Impacted Files	Coverage Δ
src/galois/_polys/_poly.py	`96.54% <97.22%> (+0.06%)`	⬆️
src/galois/_polys/_irreducible.py	`100.00% <100.00%> (ø)`
src/galois/_polys/_primitive.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mhostetter · 2023-01-20T19:44:59Z

@iyanmv can you review this PR and see how you think it'll fit into our plans for #462 and #464?

I'm not 100% happy with it, but pretty happy.

iyanmv

Looks good to me! I will start testing things with the polynomials from the databases.

src/galois/_polys/_irreducible.py

iyanmv · 2023-01-22T17:16:33Z

src/galois/_polys/_primitive.py

+        else:
+            if terms == "min":
+                terms = _minimum_terms(order, degree, "is_primitive")
+            poly = _random_search(order, degree, terms)


Maybe a future improvement for performance is not to use a random search when terms="min"? Or is this the best known algorithm?

Good point.

I struggled a lot getting an efficient iteration through only the polynomials with t non-zero terms. I used combinations and permutations from itertools, but was never able to get the iteration order in precisely lexicographical order. So, instead, I created this recursive algorithm to iterate in lexicographical order.

But now that you mention this, I could resurrect my combinations and permutations code, add an extra shuffle, and iterate through that for the method="random" and fixed-term case. I'll work on that.

Probably it's also a good idea to check how people using NTL find these polynomials?

Yes, that could be helpful.

iyanmv · 2023-01-22T18:23:41Z

src/galois/_polys/_primitive.py

+    try:
+        if method == "min":
+            poly = next(primitive_polys(order, degree, terms))
+        elif method == "max":
+            poly = next(primitive_polys(order, degree, terms, reverse=True))
+        else:
+            if terms == "min":
+                terms = _minimum_terms(order, degree, "is_primitive")
+            poly = _random_search(order, degree, terms)


What do you think about moving the if terms == "min" to the top? I would like to only include the code to search in the database in one place, but as it is, if a user writes, for example, primitive_poly(7, 129, terms="min"), it would still enter the first if method="min".

I have a commit I'm about to push. Can you let me know your suggested change to my latest?

As an alternative, you could also change the default value of method to "random" when the user specifies terms="min". What do you think?

This was the kind of thing I had in mind for the database lookup. What are your thoughts?

try: if terms == "min" and method == "min": try: # Database lookup return poly except LookupError: pass if method == "min": return next(irreducible_polys(order, degree, terms)) if method == "max": return next(irreducible_polys(order, degree, terms, reverse=True)) # Random search if terms is None: return next(_random_search(order, degree, "is_irreducible")) if terms == "min": terms = _minimum_terms(order, degree, "is_irreducible") return next(_random_search_fixed_terms(order, degree, terms, "is_irreducible"))

That works too, yes!

Actually, even better is this:

if terms is "min" and method == "min": try: # Database lookup return poly except LookupError: pass try: if method == "min": return next(irreducible_polys(order, degree, terms)) if method == "max": return next(irreducible_polys(order, degree, terms, reverse=True)) # Random search if terms is None: return next(_random_search(order, degree, "is_irreducible")) if terms == "min": terms = _minimum_terms(order, degree, "is_irreducible") return next(_random_search_fixed_terms(order, degree, terms, "is_irreducible")) except StopIteration as e: terms_str = "any" if terms is None else str(terms) raise RuntimeError( f"No monic irreducible polynomial of degree {degree} over GF({order}) with {terms_str} terms exists." ) from e

…fficient

mhostetter · 2023-01-22T19:14:51Z

@iyanmv I think this branch is good-to-go. If you agree, I'll merge into release/0.3.x and you can rebase your branches off of that. If you still have requested changes, that's cool too.

iyanmv · 2023-01-22T19:47:23Z

Sure, go ahead!

mhostetter added the poly Related to polynomials label Jan 17, 2023

mhostetter mentioned this pull request Jan 17, 2023

Add irreducible polynomials for GF(2^m) (2<=m<=10_000) #462

Merged

5 tasks

mhostetter added 2 commits January 17, 2023 16:40

Add terms kwarg to irreducible poly functions

9d0de26

Add terms kwarg to primitive poly functions

33b3449

mhostetter force-pushed the add-terms-kwarg branch from 2aab4da to 33b3449 Compare January 17, 2023 21:41

mhostetter mentioned this pull request Jan 17, 2023

Issue with verify_isinstance() and generators #459

Closed

mhostetter mentioned this pull request Jan 18, 2023

Add Wolfram's primitive polynomials #464

Open

5 tasks

Make search for polynomials of fixed-term much more efficient

7e5ce57

mhostetter force-pushed the add-terms-kwarg branch from 723f242 to ae1f535 Compare January 20, 2023 19:34

Add a terms="min" option to irreducible/primitive poly functions

5b04764

mhostetter force-pushed the add-terms-kwarg branch from ae1f535 to 5b04764 Compare January 21, 2023 14:59

iyanmv reviewed Jan 22, 2023

View reviewed changes

src/galois/_polys/_irreducible.py Outdated Show resolved Hide resolved

Move common irreducible/primitive poly functions to _poly.py

5b41b48

iyanmv reviewed Jan 22, 2023

View reviewed changes

Make search for a random irreducible/primitive polynomial much more e…

ec4289f

…fficient

mhostetter force-pushed the add-terms-kwarg branch from bc30bb7 to ec4289f Compare January 22, 2023 18:40

Move polynomial deterministic search into _poly.py

32de045

mhostetter merged commit 0c4386b into release/0.3.x Jan 22, 2023

mhostetter deleted the add-terms-kwarg branch January 22, 2023 20:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `terms` kwarg to irreducible and primitive poly functions #463

Add `terms` kwarg to irreducible and primitive poly functions #463

mhostetter commented Jan 17, 2023 •

edited

Loading

codecov bot commented Jan 17, 2023 •

edited

Loading

mhostetter commented Jan 20, 2023

iyanmv left a comment

iyanmv Jan 22, 2023

mhostetter Jan 22, 2023

iyanmv Jan 22, 2023

mhostetter Jan 22, 2023

iyanmv Jan 22, 2023

mhostetter Jan 22, 2023

iyanmv Jan 22, 2023

mhostetter Jan 22, 2023

iyanmv Jan 22, 2023

mhostetter Jan 22, 2023

mhostetter commented Jan 22, 2023

iyanmv commented Jan 22, 2023

Add terms kwarg to irreducible and primitive poly functions #463

Add terms kwarg to irreducible and primitive poly functions #463

Conversation

mhostetter commented Jan 17, 2023 • edited Loading

Examples

Examples related to #464

codecov bot commented Jan 17, 2023 • edited Loading

Codecov Report

mhostetter commented Jan 20, 2023

iyanmv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhostetter commented Jan 22, 2023

iyanmv commented Jan 22, 2023

Add `terms` kwarg to irreducible and primitive poly functions #463

Add `terms` kwarg to irreducible and primitive poly functions #463

mhostetter commented Jan 17, 2023 •

edited

Loading

codecov bot commented Jan 17, 2023 •

edited

Loading