/ sympy Public

# Implemented an algorithm to find a rational point on a conic#19807

Merged
merged 6 commits into from
Jul 30, 2020
Merged

# Implemented an algorithm to find a rational point on a conic #19807

merged 6 commits into from
Jul 30, 2020

## Conversation

This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters

### friyaz commented Jul 20, 2020 • edited

#19320

#### Brief description of what is fixed or changed

Implemented an algorithm to find a rational point on the conic. To parametrize a monoid of degree d, we need to find a point of multiplicity d - 1. This implies for curves of degree 2, we need to determine a rational point on it. While determining a point of multiplicity >= 2 is easy using sympy's non-linsolve, a separate algorithm needs to be implemented for points of multiplicity 1 or regular points.

The `regular_point` was based on iterating over a set of points and checking whether they lie on the conic. This PR fixes this for conics. I have not yet implemented the algorithm for quadrics.

#### Release Notes

• vector
• Added a function to find a rational point on conic

``` Implemented algorithm to find a rational point on a conic ```
``` 62f7a28 ```

### sympy-bot commented Jul 20, 2020 • edited

 ✅ Hi, I am the SymPy bot (v160). 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: vector Added a function to find a rational point on conic (#19807 by @friyaz and @Upabjojr) This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.7. 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. `````` #### References to other Issues or PRs #19320 #### Brief description of what is fixed or changed Implemented an algorithm to find a rational point on the conic. To parametrize a monoid of degree d, we need to find a point of multiplicity d - 1. This implies for curves of degree 2, we need to determine a rational point on it. While determining a point of multiplicity >= 2 is easy using sympy's non-linsolve, a separate algorithm needs to be implemented for points of multiplicity 1 or regular points. The `regular_point` was based on iterating over a set of points and checking whether they lie on the conic. This PR fixes this for conics. I have not yet implemented the algorithm for [quadrics](https://en.wikipedia.org/wiki/Quadric). #### Other comments #### Release Notes * vector * Added a function to find a rational point on conic `````` Update The release notes on the wiki have been updated.

marked this pull request as draft July 20, 2020 19:15
requested a review from Upabjojr July 20, 2020 19:15
reviewed
Show resolved Hide resolved
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
 @@ -245,6 +374,12 @@ def rational_parametrization(self, parameters=('t', 's'), reg_point=None): if len(self.singular_points()) != 0: singular_points = self.singular_points() for spoint in singular_points: syms = Tuple(*spoint).free_symbols

### Upabjojr Jul 20, 2020

what about `syms = set(spoint)` ?

reviewed
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
``` Improved docstrings and made minor corrections ```
``` 96056db ```
commented
Show resolved Hide resolved
 sol = list(sol)[0] syms = Tuple(*sol).free_symbols rep = {s: 3 for s in syms}

### friyaz Jul 21, 2020

There is a bug here. In cases like `x*y = 1`, the solution of diophantine equation is {(p2 + q2, -2pq, p2 - q2)}. If I substitute p = 3 and q = 3, z turns out to be zero. This leads to the result being nan. The algorithm requires the solution of diophantine equation to be non-trivial.

### Upabjojr Jul 22, 2020

What about pushing the solution of the diophantine equation into the equation solver? For example, in the case `{(p2 + q2, -2pq, p2 - q2)}`, can it help if you get `p` as a function of `q`?

### friyaz Jul 22, 2020

Yes, I think this should work. Solving for z != 0, then using one of its solutions.

### friyaz Jul 26, 2020

This is how I am trying to get the solutions for z != 0

```for sol in solutions:
syms = Tuple(*sol).free_symbols

z = sol[2]

z_syms = ztemp.free_symbols

if len(z_syms) == 2:
print(solveset(Unequality(z, 0), next(iter(z_syms)), S.Integers))```

But solveset is too slow for returning an Integer solution.

### Upabjojr Jul 26, 2020

Have you thought about solving the equality and then taking the complement of the solutions? It should be equivalent to `solveset(Unequality( ... ))`.

### friyaz Jul 26, 2020

This should work. In the case of two variables p and q, I will iterate p over S.Integers and then solve q for z != 0.

### friyaz Jul 29, 2020

ping @Upabjojr.

requested a review from Upabjojr July 21, 2020 12:38
reviewed
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
reviewed
Show resolved Hide resolved
reviewed
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
and others added 3 commits July 26, 2020 23:03
``` Update sympy/vector/implicitregion.py ```
``` 6542c0d ```
`Co-authored-by: Naman Gera <namangera15@gmail.com>`
``` regular_point: Use only non-trivial solutions of diophantine equation ```
``` ea478b4 ```
``` Resolved merge conflict in implicitregion.py ```
``` 895675b ```
marked this pull request as ready for review July 28, 2020 14:32
requested a review from Upabjojr July 28, 2020 14:33

# Codecov Report

Merging #19807 into master will decrease coverage by `0.009%`.
The diff coverage is `71.328%`.

```@@              Coverage Diff              @@
##            master    #19807       +/-   ##
=============================================
- Coverage   75.719%   75.709%   -0.010%
=============================================
Files          664       666        +2
Lines       172374    172712      +338
Branches     40653     40717       +64
=============================================
+ Hits        130520    130759      +239
- Misses       36147     36225       +78
- Partials      5707      5728       +21     ```

reviewed
 @@ -210,7 +376,8 @@ def rational_parametrization(self, parameters=('t', 's'), reg_point=None): ========= - Christoph M. Hoffmann, Conversion Methods between Parametric and Implicit Curves and Surfaces. Implicit Curves and Surfaces, 1990. Available:

### Upabjojr Jul 30, 2020

Publisher and authors are more important than the title itself. Christoph M. Hoffmann? Perdue e-Pubs?

### Upabjojr commented Jul 30, 2020

 This looks good to me. Please add the authors and publisher of all citations you've added (in the literatura, authors and publishing review company are usually more important than the title of the paper itself).

requested a review from Upabjojr July 30, 2020 10:07
``` implicitregion.py: Added publisher's name in citation ```
``` 85da83b ```