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

Adding classes to represent implicitly defined regions #19681

Merged
merged 7 commits into from Jul 18, 2020

Conversation

friyaz
Copy link
Member

@friyaz friyaz commented Jul 1, 2020

References to other Issues or PRs

#19320

Brief description of what is fixed or changed

The vector module has support to define regions using parametric representation and integrate over them.
In some cases, It is easier to work to define regions using their implicit equation. This Pull Request aims to add classes to represent implicitly defined regions.

Other comments

This PR is a draft and the structure of the PR is not fixed.

Release Notes

  • vector
    • Added support to create implictly defined regions.

@friyaz
Copy link
Member Author

friyaz commented Jul 1, 2020

Algorithm to determine the parametric representation of a Conic:

  1. Fix a point p on the conic. Consider the pencil of lines through p. Formulate the line equations.
  2. Substitute for y in the conic equation, solve for x(t).
  3. Use the fine equations to determine y(t).

@sympy-bot
Copy link

sympy-bot commented Jul 1, 2020

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 support to create implictly defined regions. (#19681 by @friyaz)

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.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
#19320 

#### Brief description of what is fixed or changed
The vector module has support to define regions using parametric representation and integrate over them.
In some cases, It is easier to work to define regions using their implicit equation. This Pull Request aims to add classes to represent implicitly defined regions.

#### Other comments
This PR is a draft and the structure of the PR is not fixed. 
#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* vector
    * Added support to create implictly defined regions.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@sympy-bot
Copy link

sympy-bot commented Jul 1, 2020

🟠

Hi, I am the SymPy bot (v160). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at sympy/sympy-bot#75.

The following commits add new files:

  • 4e5ddc0:
    • sympy/vector/implicitregion.py
  • 1ba0aca:
    • sympy/vector/tests/test_implicitregion.py

If these files were added/deleted on purpose, you can ignore this message.


# Finding a point on the Curve.
# Needs to find a better way than random looping.
for i in range(100):
Copy link
Member Author

@friyaz friyaz Jul 1, 2020

Choose a reason for hiding this comment

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

I need to find a better way to find a point on the conic.

Copy link
Contributor

@Upabjojr Upabjojr Jul 4, 2020

Choose a reason for hiding this comment

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

have you thought about some deterministic-iterative algorithm?

Copy link
Contributor

@Upabjojr Upabjojr Jul 4, 2020

Choose a reason for hiding this comment

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

If you cannot solve the equation... maybe you are at a singular point, right? Can you determine singular points from the gradient of the implicit equation? Maybe thinking of example equations where this case happens, may help think about a solution.

Copy link
Member Author

@friyaz friyaz Jul 6, 2020

Choose a reason for hiding this comment

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

I am working on it. I think I will implement a general algorithm for monoids which will work for all conics.

Copy link
Member Author

@friyaz friyaz Jul 13, 2020

Choose a reason for hiding this comment

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

Found on an algorithm for determining a point on the conic:
https://www3.risc.jku.at/publications/download/risc_1355/Rational%20Points%20on%20Conics.pdf.

@Upabjojr the algorithm is non-trivial. Is it worth it to implement? Even parametrizing conics seems to be a tough task,

Copy link
Member Author

@friyaz friyaz Jul 14, 2020

Choose a reason for hiding this comment

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

Another option is to make the user input a point on the curve apart from implicit equation.

An example:

C = ImplicitCurve(Eq(x**2 + y**2 - 4))
C.parametrization(Point(0, 2)) #Takes a point on a curve
(2*cos(theta), 2*sin(theta))

@Upabjojr Any thoughts?

Copy link
Contributor

@Upabjojr Upabjojr Jul 14, 2020

Choose a reason for hiding this comment

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

Suggesting the point is fine with me, especially if it makes the code simpler to implement.

Copy link
Member Author

@friyaz friyaz Jul 14, 2020

Choose a reason for hiding this comment

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

We can work with this and maybe later implement the algorithm for finding a rational point on the curve.

Copy link
Contributor

@Upabjojr Upabjojr Jul 14, 2020

Choose a reason for hiding this comment

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

Of course... always start with a simple algorithm. We can open as many pull requests as we like, so no need to do it now.

Copy link
Member Author

@friyaz friyaz Jul 14, 2020

Choose a reason for hiding this comment

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

Thanks. I will complete this work then. This point will be optional. If not given, the algorithm will iterate over some point. If it is unable to find a point on the curve, it raises NotImplementedError.

@friyaz
Copy link
Member Author

friyaz commented Jul 1, 2020

A better way is to use an algorithm to parameterize a monoid. A monoid is an algebraic curve of degree n that has a point of multiplicity n -1. All conics and singular conics are monoids. But this requires determining the (n -1) fold point. I currently do not know how to determine this.

@friyaz friyaz marked this pull request as draft Jul 1, 2020
@friyaz friyaz requested a review from Upabjojr Jul 1, 2020
@friyaz
Copy link
Member Author

friyaz commented Jul 3, 2020

ping @Upabjojr

sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
@friyaz friyaz requested a review from Upabjojr Jul 8, 2020
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
sympy/vector/implicitregion.py Outdated Show resolved Hide resolved
@friyaz friyaz requested a review from Upabjojr Jul 9, 2020
@friyaz
Copy link
Member Author

friyaz commented Jul 13, 2020

@Upabjojr My concern is that the parametric representation determined by these algorithms are mostly in terms of large and complex expression. 'ParametricIntegralis not able to integrate over such regions due tointegrate` function being slow.
For example:

>>> ellipse = ImplicitRegion(x**2/4 + y**2/16 - 1, x, y)
>>> parametric_region_list(ellipse)
[ParametricRegion((8*tan(theta)/(tan(theta)**2 + 4), -4 + 8*tan(theta)**2/(tan(theta)**2 + 4)), (theta, 0, pi))]
>>> vector_integrate(2, ellipse)
Too slow

There is little benefit of adding functions to determine parametric representation of implicit region if integrate is not able to work on it.

@friyaz
Copy link
Member Author

friyaz commented Jul 14, 2020

Sage has a function for rational parametrization of Curves. See rational_parametrization from https://github.com/sagemath/sage/blob/develop/src/sage/schemes/curves/affine_curve.py

But it requires the implementation of many other related classes.

@Upabjojr
Copy link
Contributor

Upabjojr commented Jul 14, 2020

Sage has a function for rational parametrization of Curves. See rational_parametrization from https://github.com/sagemath/sage/blob/develop/src/sage/schemes/curves/affine_curve.py

But it requires the implementation of many other related classes.

Sage is GPL'd. We cannot reuse their code. Their approach to this problem is a lot more theoretical... they define lots of algebraic structures. Is this really the purpose of sympy.vector? Complicated algebraic structures will make the code useful only to mathematicians or very theoretical people, cutting off many SymPy users.

@Upabjojr
Copy link
Contributor

Upabjojr commented Jul 14, 2020

There is little benefit of adding functions to determine parametric representation of implicit region if integrate is not able to work on it.

That is a bug of integrate... maybe someone will fix that in the future? My understanding is that integrate tries a list of fast algorithms, if all fail, it falls back to a very slow backup algorithm.

@friyaz
Copy link
Member Author

friyaz commented Jul 14, 2020

Sage is GPL'd. We cannot reuse their code. Their approach to this problem is a lot more theoretical... they define lots of algebraic structures. Is this really the purpose of sympy.vector? Complicated algebraic structures will make the code useful only to mathematicians or very theoretical people, cutting off many SymPy users.

Probably something like this can become part of the geometry module. I had to email authors of a few research papers to understand some of these concepts. I highly doubt Mathematica integrates over implicit regions this way. It can handle many complex curves and surfaces including inequalities that most likely are not easily parametrizable.

That is a bug of integrate... maybe someone will fix that in the future? My understanding is that integrate tries a list of fast algorithms, if all fail, it falls back to a very slow backup algorithm.

I agree. I did not thought it in this way.

@Upabjojr
Copy link
Contributor

Upabjojr commented Jul 14, 2020

Probably something like this can become part of the geometry module. I had to email authors of a few research papers to understand some of these concepts. I highly doubt Mathematica integrates over implicit regions this way. It can handle many complex curves and surfaces including inequalities that most likely are not easily parametrizable.

That's a nice research. In case you decide not to implement this algorithm, what about writing a page on SymPy's wiki?

@friyaz
Copy link
Member Author

friyaz commented Jul 14, 2020

That's a nice research. In case you decide not to implement this algorithm, what about writing a page on SymPy's wiki?

OK, For either case, I will try to summarize what I have learned on parametrizing rational curves/surfaces.

@friyaz
Copy link
Member Author

friyaz commented Jul 15, 2020

@Upabjojr I have implemented the algorithm for the parametrization of monoids. We can now determine the rational parametrization of all algebraic curves and surfaces of degree > 2 if a parametrization exists.
This algorithm also works for some cases of degree 2 curves/surfaces. To handle every case, proper implementation of regular_point is needed.

It is important to note that the algorithm returns rational parametrization so in some cases the result may not be convenient.
For example, In the case of a circle:

>>> c = ImplicitRegion((x, y), x**2 + y**2 - 16)
>>> c.rational_parametrization()
(-4 + 8/(t**2 + 1), 8*t/(t**2 + 1))

@friyaz friyaz marked this pull request as ready for review Jul 16, 2020
@friyaz friyaz requested review from Upabjojr and removed request for Upabjojr Jul 16, 2020
@codecov
Copy link

codecov bot commented Jul 16, 2020

Codecov Report

Merging #19681 into master will decrease coverage by 0.004%.
The diff coverage is 86.776%.

@@              Coverage Diff              @@
##            master    #19681       +/-   ##
=============================================
- Coverage   75.709%   75.704%   -0.005%     
=============================================
  Files          659       664        +5     
  Lines       171401    172364      +963     
  Branches     40437     40652      +215     
=============================================
+ Hits        129766    130487      +721     
- Misses       35982     36164      +182     
- Partials      5653      5713       +60     

@Upabjojr
Copy link
Contributor

Upabjojr commented Jul 16, 2020

@Upabjojr I have implemented the algorithm for the parametrization of monoids. We can now determine the rational parametrization of all algebraic curves and surfaces of degree > 2 if a parametrization exists.
This algorithm also works for some cases of degree 2 curves/surfaces. To handle every case, proper implementation of regular_point is needed.

It is important to note that the algorithm returns rational parametrization so in some cases the result may not be convenient.
For example, In the case of a circle:

>>> c = ImplicitRegion((x, y), x**2 + y**2 - 16)
>>> c.rational_parametrization()
(-4 + 8/(t**2 + 1), 8*t/(t**2 + 1))

That's pretty nice.

@friyaz
Copy link
Member Author

friyaz commented Jul 16, 2020

I will try to implement the algorithm for finding a regular point on a Conic in a separate PR. Maybe we can then get rid of reg_point parameter in rational_parametrization.


return m

def rational_parametrization(self, parameter1='t', parameter2='s', reg_point=None):
Copy link
Contributor

@Upabjojr Upabjojr Jul 16, 2020

Choose a reason for hiding this comment

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

Is this supposed to work for 2D only? What about parameters = ('t', 's') instead?

Copy link
Member Author

@friyaz friyaz Jul 16, 2020

Choose a reason for hiding this comment

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

It will work for curves and surfaces given that they are monoid.

Copy link
Member Author

@friyaz friyaz Jul 16, 2020

Choose a reason for hiding this comment

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

The report which I followed had algorithms only for curves and surfaces. No Volume regions or higher dimensional region.

What about parameters = ('t', 's') instead?

This looks better. I will fix it.

Copy link
Member Author

@friyaz friyaz Jul 18, 2020

Choose a reason for hiding this comment

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

Is this supposed to work for 2D only? What about parameters = ('t', 's') instead?

It does not work for 3-D curves. It works for 3-D surfaces but does not support higher-dimensional surfaces..

@friyaz friyaz changed the title [WIP] Adding classes to represent implicitly defined regions Adding classes to represent implicitly defined regions Jul 17, 2020
@friyaz friyaz requested a review from Upabjojr Jul 17, 2020
@Upabjojr Upabjojr merged commit 1b8a63b into sympy:master Jul 18, 2020
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants