Composite number for nthroot_mod #18199

abhinav-anand-addepar · 2020-01-01T19:08:59Z

ntheory
- nthroot_mod now supports composite moduli

sympy-bot · 2020-01-01T19:09:08Z

✅

Hi, I am the SymPy bot (v149). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

ntheory
- nthroot_mod now supports composite moduli (#18199 by @abhinav28071999)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

Fixes #17373
Fixes #17377
Fixes #18212

<!-- BEGIN RELEASE NOTES -->
* ntheory
  * `nthroot_mod` now supports composite moduli
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

abhinav-anand-addepar · 2020-01-01T20:05:02Z

@smichr Could you please look into this.

sympy/ntheory/residue_ntheory.py

czgdp1807 · 2020-01-02T16:08:52Z

There is a lot of branching in the code using continue. Is it possible to figure out a way to write the code without branching using continue.

Sc0rpi0n101 · 2020-01-03T11:47:58Z

I think a better release notes entry will be:

ntheory
- Improved nthroot_mod to support composite moduli

The release notes are just meant to notify the users of major changes in the releases. If you want to include a reference to an important algorithm or method, you should do it in the documentation or a wiki guide, not the release notes.

sympy/ntheory/residue_ntheory.py

abhinav-anand-addepar · 2020-01-03T14:27:09Z

Don't know why the check failed.

abhinav-anand-addepar · 2020-01-03T16:40:47Z

dont know why, but it was not able to import crt when at start of file.

jksuom · 2020-01-03T16:54:48Z

It seems that crt should be imported in the function because of circularity.

jksuom · 2020-01-03T17:38:52Z

sympy/ntheory/residue_ntheory.py

+        tot_roots = set()
+        if e == 1:
+            root = nthroot_mod(a, n, p, True) or []
+            if (pow(p, n) - a) % p == 0:


nthroot_mod should return also this case.

No is doesn't. nthroot_mod(0, 5, 17, True) returns None but it should return [0]. see issue #18212

So let's get the PR that fixes that in first.

i am working on it. Will send it by tomorrow.

I think is_nthpow_residue(a, n, m) works only when m is prime. Because is_nthpow_residue(36010, 8, 87382) returns false but it has solutions [40208, 47174]

sympy/ntheory/residue_ntheory.py

abhinav-anand-addepar · 2020-01-03T18:56:37Z

I think i have solved the issue #18212. when p is prime and a % p == 0, then [0] is the only root.
This i have taken care in nthroot_modd and when p is not prime then _nthroot_mod_composite handles it.

abhinav-anand-addepar · 2020-01-03T19:00:25Z

But we still have to fix is_nthpow_residue(a, n, m) because currently it only works for prime m otherwise it returns wrong answer. I confirmed it -> theorem 1.2.

abhinav-anand-addepar · 2020-01-04T20:53:06Z

what happened? Is everything okay?

smichr · 2020-01-04T21:42:00Z

When this happens, I rebase locally on a fresh copy of master, create a diff, check out master, delete my branch and recreate it from master and then apply the diff and commit it:

git merge master
git diff master > dif
git checkout master
git branch -D composite
git checkout -b composite
git apply dif
git commit -am "composite number for nthroot_mod"

codecov · 2020-01-04T22:04:14Z

Codecov Report

Merging #18199 into master will increase coverage by 0.027%.
The diff coverage is 100%.

@@              Coverage Diff              @@
##            master    #18199       +/-   ##
=============================================
+ Coverage   74.945%   74.972%   +0.027%     
=============================================
  Files          642       649        +7     
  Lines       167182    167379      +197     
  Branches     39340     39393       +53     
=============================================
+ Hits        125296    125489      +193     
+ Misses       36359     36347       -12     
- Partials      5527      5543       +16

smichr · 2020-01-05T02:25:39Z

we still have to fix is_nthpow_residue(a, n, m)

Does this still need to be done?

abhinav-anand-addepar · 2020-01-05T02:49:41Z

we still have to fix is_nthpow_residue(a, n, m)

Does this still need to be done?

I will send a separate pr regarding is_nthpow_residue(a, n, m).

sympy/ntheory/residue_ntheory.py

smichr · 2020-01-05T13:45:01Z

I will send a separate pr

this can be committed without that, right?

abhinav-anand-addepar · 2020-01-05T14:26:21Z

I will send a separate pr

this can be committed without that, right?

yes

sympy/ntheory/residue_ntheory.py

smichr · 2020-01-06T22:09:52Z

sympy/ntheory/residue_ntheory.py

+                diff = (n * pow(root, n - 1)) % p
+                if diff != 0:
+                    for j in range(1, e):
+                        root = (root - (pow(root, n) - a)* mod_inverse(diff, p)) % pow(p, j + 1)


Suggested change

root = (root - (pow(root, n) - a)* mod_inverse(diff, p)) % pow(p, j + 1)

root = (root - (rootn - a)* mod_inverse(diff, p)) % ppow

smichr · 2020-01-06T22:14:05Z

sympy/ntheory/residue_ntheory.py

+            tot_roots.update(nthroot_mod(a, n, p, True) or [])
+        else:
+            for root in nthroot_mod(a, n, p, True) or []:
+                diff = (n * pow(root, n - 1)) % p


Suggested change

diff = (n * pow(root, n - 1)) % p

rootn = pow(root, n)

diff = (rootn//root*n) % p

root can be zero , so its better to use pow(root, n-1) and avoid division by root. I have made the changes.

smichr · 2020-01-06T22:14:43Z

Just trying to factor out the power calculations. See what you think.

sympy/ntheory/residue_ntheory.py

smichr · 2020-01-09T08:19:57Z

OK, thanks for this work -- it's in!

jksuom · 2020-01-09T10:55:29Z

I think that more work is needed in the prime power modulus case.

In [11]: nthroot_mod(4, 4, 27, True)
Out[11]: [mpz(7), mpz(8)]

In [12]: 7**4 % 27
Out[12]: 25

In [13]: 8**4 % 27
Out[13]: 19

jksuom · 2020-01-09T10:55:39Z

sympy/ntheory/residue_ntheory.py

+                    ppow = p
+                    for j in range(1, e):
+                        ppow *= p
+                        root = (root - (rootn - a) * mod_inverse(diff, p)) % ppow


This line should probably be modified.

yes you are right, the rootn will change with change in root

abhinav-anand-addepar changed the title ~~Composite number for nsqrt_mod~~ Composite number for nthroot_mod Jan 1, 2020

czgdp1807 added the ntheory label Jan 2, 2020

czgdp1807 reviewed Jan 2, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

czgdp1807 reviewed Jan 2, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

jksuom reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

jksuom reviewed Jan 3, 2020

View reviewed changes

smichr reviewed Jan 3, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

composite number for nthroot_mod

9e58be3

smichr reviewed Jan 5, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 5, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 5, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 5, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

abhinav-anand-addepar and others added 2 commits January 5, 2020 20:01

made suggested changes

c2b9980

minor change

7ba0503

smichr reviewed Jan 6, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Show resolved Hide resolved

smichr reviewed Jan 6, 2020

View reviewed changes

abhinav-anand-addepar closed this Jan 6, 2020

abhinav-anand-addepar reopened this Jan 6, 2020

abhinav-anand-addepar added 2 commits January 8, 2020 22:54

made suggested changes

f5d6e8b

Root can be 0 , corrected the code

9fcb418

smichr reviewed Jan 8, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 8, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

smichr reviewed Jan 8, 2020

View reviewed changes

sympy/ntheory/residue_ntheory.py Outdated Show resolved Hide resolved

abhinav-anand-addepar added 3 commits January 9, 2020 09:01

Made suggested changes

6ed39e4

fixed syntax error

52fd2e6

made changes to diff

c885a62

smichr merged commit 4bdfba0 into sympy:master Jan 9, 2020

jksuom reviewed Jan 9, 2020

View reviewed changes

abhinav-anand-addepar mentioned this pull request Jan 10, 2020

changed none to [] #18285

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Composite number for nthroot_mod #18199

Composite number for nthroot_mod #18199

abhinav-anand-addepar commented Jan 1, 2020 •

edited

sympy-bot commented Jan 1, 2020 •

edited

abhinav-anand-addepar commented Jan 1, 2020

czgdp1807 commented Jan 2, 2020 •

edited

Sc0rpi0n101 commented Jan 3, 2020 •

edited

abhinav-anand-addepar commented Jan 3, 2020

abhinav-anand-addepar commented Jan 3, 2020

jksuom commented Jan 3, 2020

jksuom Jan 3, 2020

abhinav-anand-addepar Jan 3, 2020

smichr Jan 3, 2020

abhinav-anand-addepar Jan 3, 2020

abhinav-anand-addepar Jan 3, 2020

abhinav-anand-addepar commented Jan 3, 2020

abhinav-anand-addepar commented Jan 3, 2020 •

edited

abhinav-anand-addepar commented Jan 4, 2020 •

edited

smichr commented Jan 4, 2020

codecov bot commented Jan 4, 2020 •

edited

smichr commented Jan 5, 2020

abhinav-anand-addepar commented Jan 5, 2020

smichr commented Jan 5, 2020

abhinav-anand-addepar commented Jan 5, 2020

smichr Jan 6, 2020

smichr Jan 6, 2020

abhinav-anand-addepar Jan 8, 2020

smichr commented Jan 6, 2020

smichr commented Jan 9, 2020

jksuom commented Jan 9, 2020

jksuom Jan 9, 2020

abhinav-anand-addepar Jan 9, 2020

	root = (root - (pow(root, n) - a)* mod_inverse(diff, p)) % pow(p, j + 1)
	root = (root - (rootn - a)* mod_inverse(diff, p)) % ppow

	diff = (n * pow(root, n - 1)) % p
	rootn = pow(root, n)
	diff = (rootn//root*n) % p

Composite number for nthroot_mod #18199

Composite number for nthroot_mod #18199

Conversation

abhinav-anand-addepar commented Jan 1, 2020 • edited

sympy-bot commented Jan 1, 2020 • edited

abhinav-anand-addepar commented Jan 1, 2020

czgdp1807 commented Jan 2, 2020 • edited

Sc0rpi0n101 commented Jan 3, 2020 • edited

abhinav-anand-addepar commented Jan 3, 2020

abhinav-anand-addepar commented Jan 3, 2020

jksuom commented Jan 3, 2020

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

abhinav-anand-addepar commented Jan 3, 2020

abhinav-anand-addepar commented Jan 3, 2020 • edited

abhinav-anand-addepar commented Jan 4, 2020 • edited

smichr commented Jan 4, 2020

codecov bot commented Jan 4, 2020 • edited

Codecov Report

smichr commented Jan 5, 2020

abhinav-anand-addepar commented Jan 5, 2020

smichr commented Jan 5, 2020

abhinav-anand-addepar commented Jan 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smichr commented Jan 6, 2020

smichr commented Jan 9, 2020

jksuom commented Jan 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinav-anand-addepar commented Jan 1, 2020 •

edited

sympy-bot commented Jan 1, 2020 •

edited

czgdp1807 commented Jan 2, 2020 •

edited

Sc0rpi0n101 commented Jan 3, 2020 •

edited

abhinav-anand-addepar commented Jan 3, 2020 •

edited

abhinav-anand-addepar commented Jan 4, 2020 •

edited

codecov bot commented Jan 4, 2020 •

edited