Skip to content

add fmpz_is_square #172

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.

Sign up for GitHub

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

Merged
oscarbenjamin merged 6 commits into from
Jul 29, 2024
Merged

add fmpz_is_square #172

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd08ee9
add is_square
roos-j Jul 29, 2024
228f0af
fix typo
roos-j Jul 29, 2024
d1a8758
add gh-172
roos-j Jul 29, 2024
c42fa0a
add docstring for is_perfect_power
roos-j Jul 29, 2024
c2f873d
update docstring
roos-j Jul 29, 2024
dfc20da
test is_square
roos-j Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ CHANGELOG

Next release:

- [gh-172](https://github.com/flintlib/python-flint/pull/161)
Add `fmpz_is_square`.
- [gh-161](https://github.com/flintlib/python-flint/pull/161)
Add `acb.lerch_phi` to compute the Lerch transcendent.
- [gh-132](https://github.com/flintlib/python-flint/pull/132)
Expand All @@ -146,7 +148,7 @@ Next release:
- [gh-148](https://github.com/flintlib/python-flint/pull/148)
Remove debug symbols to make smaller Linux binaries.
- [gh-144](https://github.com/flintlib/python-flint/pull/144)
Add `rel_one_ccuracy_bits` to `arb` and `acb`.
Add `rel_one_accuracy_bits` to `arb` and `acb`.
- [gh-142](https://github.com/flintlib/python-flint/pull/142)
Add `acb_theta` module for the numerical evaluation of [theta
functions](https://flintlib.org/doc/acb_theta.html) (only available for
Expand Down
19 changes: 19 additions & 0 deletions src/flint/types/fmpz.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,11 +683,30 @@ cdef class fmpz(flint_scalar):
return fmpz_is_probabprime(self.val)

def is_perfect_power(self):
r"""
Return True if this integer is of the form `r^k`, False otherwise.
Comment thread
roos-j marked this conversation as resolved.
Outdated

>>> fmpz(81).is_perfect_power()
True
>>> fmpz(1234).is_perfect_power()
False
"""
cdef int k
cdef fmpz v = fmpz()
k = fmpz_is_perfect_power(v.val, self.val)
return k != 0
Comment on lines 685 to 698
Copy link
Copy Markdown
Collaborator

@oscarbenjamin oscarbenjamin Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have both methods but more useful than is_perfect_power would be a method that returns the power like SymPy's perfect_power function:

In [1]: perfect_power(81)
Out[1]: (3, 4)

In [2]: perfect_power(80)
Out[2]: False

I'm not sure what is best to return if it is not a perfect power. Returning False is perhaps a bit strange...

Alternatives:

  1. Raise an exception.
  2. Return clearly invalid number like (0, -1).
  3. Return a tuple with a boolean like (3, 4, True) or (0, -1, False).
  4. Return None.

Maybe returning False is good as anything else...

Copy link
Copy Markdown
Collaborator

@oscarbenjamin oscarbenjamin Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps it can just be:

>>> perfect_power(80)
(80, 1)

So perfect_power(n) always returns (r, k) such that n = r^k where k is either maximal or 1 if no maximal k exists:

>>> perfect_power(1)
(1, 1)

Copy link
Copy Markdown
Contributor Author

@roos-j roos-j Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_perfect_power was there before this PR, but when I saw it, I also thought it should best return (r,k) since that data is provided by the C function (in that case I would lean towards None if not a perfect power), so I'd be happy to change that.
Are you also suggesting it be renamed to perfect_power? That could make sense since one might expect a method called is_something to return a boolean. On the other hand, it seems the currently prevailing convention is to mirror the C library names, so maybe it should stay is_perfect_power?

Copy link
Copy Markdown
Collaborator

@oscarbenjamin oscarbenjamin Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed the fact that the function was already there. I guess it is better not to change it then.

We could add a separate .perfect_power() method though to return (r, k). The convention it seems in other places is that a perfect power should have r,k>=2 although I guess that the .perfect_power() method should be consistent with is_perfect_power().

Copy link
Copy Markdown
Contributor Author

@roos-j roos-j Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be good. The issue is though that (r,k) returned by fmpz_is_perfect_power is not guaranteed to have maximal k; the right way to proceed would be to implement that feature in flint's fmpz_is_perfect_power and then use it in python-flint to implement a method .perfect_power() as you are suggesting.

Copy link
Copy Markdown
Collaborator

@oscarbenjamin oscarbenjamin Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay. I suppose we could just call fmpz_is_perfect_power in a loop until we've reduced down to a number that is not a perfect power.

Maybe returning a result with non-maximal k is useful. I would be more inclined to reduce as far as possible (i.e. find maximal k). That is what Sage's perfect_power apparently does:
https://doc.sagemath.org/html/en/reference/rings_standard/sage/rings/integer.html#sage.rings.integer.Integer.perfect_power

This seems like the sort of situation where what is best in python-flint might be a bit different from what Flint itself does. I'm not sure what the performance implications would be though of calling fmpz_is_perfect_power in a loop (with successively smaller arguments) compared to just calling it once.


def is_square(self):
r"""
Return True if perfect square and False otherwise.

>>> fmpz(25).is_square()
True
>>> fmpz(101).is_square()
False
"""
return fmpz_is_square(self.val) != 0

def partitions_p(n):
r"""
Returns `p(n)`, the number of partitions of `n`, as an *fmpz*.
Expand Down