Dispersion #2679

raoulb · 2013-12-14T19:36:55Z

Dispersion computation

raoulb · 2013-12-14T19:39:52Z

Complete references
Move to correct place in sympy tree
Include into documentation
What about polynomials over Q[a] instead of Q?

raoulb · 2013-12-14T19:41:00Z

For point 2 I'll need some advise from the people working on the poly submodule.
I think this should go in polys.

skirpichev · 2013-12-14T20:49:19Z

better names?

raoulb · 2013-12-14T21:39:31Z

better names?

For what exactly?

The two functions are named like
the official mathematical things
they implement. (I'm not aware of
other names for the concept of
the "dispersion" of polynomials.)

asmeurer · 2013-12-17T00:32:53Z

Add doctests.

asmeurer · 2013-12-17T00:33:45Z

sympy/concrete/dispersion.py

+    ..[3]: "Hypergeometric Summation: An Algorithmic Approach to Summationand Special Function Identities.
+    ..[4]: "Fast Polynomial Dispersion Computation and its Application to Indefinite Summation"
+    """
+    # Check for valid input


The first line should probably be something like p, q = Poly(p), Poly(q).

The first line should probably be something like p, q = Poly(p), Poly(q).

Yes, probably. Depending on where we put the code in the end.

asmeurer · 2013-12-17T00:35:01Z

If you put this in the polys, you can implement a low-level algorithm and implement the wrappers at the different layers (which do various levels of type checking). I'm not sure what the state of the polys is, though, i.e., using Poly vs. ring().

asmeurer · 2013-12-17T00:37:35Z

sympy/concrete/dispersion.py

+
+    .. math::
+        J(f, g) & := \{a \in Z^{+} | gcd(f(x), g(x+a)) \neq 1\} \\
+                &  = \{a \in Z^{+} | deg gcd(f(x), g(x+a)) \geq 1\}


So intuitively, the set of positive integers n where the root of one polynomial can be shifted to the left by n to get the root of another?

So intuitively, the set of positive integers n where the root of
one polynomial can be shifted to the left by n to get the root of
another?

To cite from W. Koepf's book:

"Note that the dispersion of two polynomials gives information about
their shift structure (answering the question: Does a shift generate
a common divisor?)."

I'd rather talk about factors than roots but the difference is small.
In fact if you compute the dispersion by using resultants and solving
the polynomial for the shifts, this gives exactly your statement.

asmeurer · 2013-12-17T00:40:53Z

Is there a Wikipedia article for this?

raoulb · 2013-12-17T10:40:15Z

If you put this in the polys, you can implement a low-level algorithm
and implement the wrappers at the different layers (which do various
levels of type checking). I'm not sure what the state of the polys
is, though, i.e., using Poly vs. ring().

That would be the idea. But for that I'd need some guide
from the people who wrote the poly module. I'm not sure if
there is enough difference for layers, the algorithms is
highly efficient per se.

I plan to do a p.dispersion() and p.dispersion(q) method
on poly objects and a free method dispersion(p, q).

raoulb · 2013-12-17T10:40:17Z

Is there a Wikipedia article for this?

I did not check. But there are at least two good
descriptions in literature. (I did not finish
the references yet, adding authors and journals
etc.)

If I find some articles on wiki, mathworld or
enc. of mathematics, I'll add them too.

raoulb · 2013-12-17T10:40:17Z

Add doctests.

Of course, the doc is not finished yet.

raoulb · 2013-12-21T17:14:02Z

Ok, this is in a state where we can move it to the poly module now.
Please help.

raoulb · 2013-12-21T17:14:24Z

Ok, this is in a state where we can move it to the poly module now.
Please help.

asmeurer · 2013-12-21T17:16:36Z

Unfortunately, I don't even know what the state of the polys module is now, because of Poly vs. ring. @mattpap would have to comment. I know how to put this in Poly, but it probably would be better to put new code in ring.

Anyway, I don't think it's too bad to leave high-level algorithms like this as only using Poly (or ring). Ideally they should be fast enough to handle it. And that's kind of the whole point of having the objects in the first place. The Risch stuff for instance uses Poly for everything, and only at the highest level.

raoulb · 2013-12-21T17:16:50Z

There seem to be no articles on:

Wikipedia
Mathworld
Encyclopedia of mathematics

If someone finds a reference, please tell.
The given literature should suffice anyway.
(At least the Journal of Symbolic computation is open archive I think.)

asmeurer · 2013-12-21T17:21:14Z

I guess you'll have to write one :)

asmeurer · 2013-12-21T17:22:33Z

Regardless, I would say this belongs in the polys module, not the concrete module.

asmeurer · 2013-12-21T17:23:07Z

I'll leave it to you to verify that the LaTeX math renders correctly.

raoulb · 2013-12-21T17:47:37Z

Regardless, I would say this belongs in the polys module, not the
concrete module.

Ok, then I'll move this file as it is to the poly
and adapt the imports. Unless someone else has
a better solution.

raoulb · 2013-12-21T17:47:37Z

I'll leave it to you to verify that the LaTeX math renders correctly.

Sure. I'll test this once the docs are imported into sphinx.

raoulb · 2013-12-21T17:47:38Z

Unfortunately, I don't even know what the state of the polys module
is now, because of Poly vs. ring. @mattpap would have to comment. I
know how to put this in Poly, but it probably would be better to put
new code in ring.

Oh, poly is out? I did not know that.
I've never heard of ring I think until now.

Anyway, I don't think it's too bad to leave high-level algorithms
like this as only using Poly (or ring). Ideally they should be fast
enough to handle it.

Probably the factorization is the slowest part anyway.

raoulb · 2014-01-12T01:37:12Z

It seems travis had a timeout.

raoulb · 2014-01-12T01:39:32Z

I'd like to integrate this deeper within the polys. Each poly instance should get a dispersion the same way they all have a discriminant method. But I do not know what and on which layers I have to add callbacks. So either someone guides me here or we make that part of another, future PR and review and merge this as it is.

asmeurer · 2014-01-17T01:31:41Z

Do you want to implement it completely layered, or just at the Poly level? It didn't strike me as an algorithm that would benefit much from a low level implementation.

asmeurer · 2014-01-17T01:32:52Z

Let's just fix this as it is. It needs a doctest. Otherwise, assuming you fixed the definitional issues I pointed out, it should be fine.

raoulb · 2014-01-18T00:21:44Z

Do you want to implement it completely layered, or just at the Poly
level? It didn't strike me as an algorithm that would benefit much
from a low level implementation.

I'd like to have an additional OO public API like:

In [4]: P = poly(x**2 - 3*x, x)

In [5]: P.discriminant()
Out[5]: 9

We can then just call the functions
like P.dispersion() and P.dispersion(Q).

asmeurer · 2014-01-18T00:26:06Z

In that case I wouldn't bother with all the lower levels (unless you find yourself needing it there too). Just implement the method on Poly.

raoulb · 2014-01-18T01:03:43Z

Thanks. I added a high level OOP interface. Please review. There was a circular import, hence the imports inside the function definition.

asmeurer · 2014-01-19T00:55:12Z

sympy/polys/dispersion.py

+    4. [Man93]_
+    """
+    J = dispersionset(p, q)
+    if len(J) == 0:


asmeurer · 2014-01-19T00:57:43Z

Just those comments.

asmeurer · 2014-01-19T01:14:34Z

sympy/polys/dispersion.py

+    if p.degree() < 1 or q.degree() < 1:
+        return set([0])
+
+    # Factor p and q over the rationals


That's only the case if the domain of p and q is QQ or ZZ. What if there's an algebraic domain? Or a symbolic one?

But it does look like you tested those cases, so maybe it works.

Dispersion

asmeurer reviewed Dec 17, 2013
View reviewed changes

raoul added 12 commits December 21, 2013 03:37

More efficient dispersion computation not using resultants

1771578

Docstrings for dispersion functions

2ea7286

Clean up generator handling

e6e8468

Define maximum of empty set

8b116a8

Add tests

bd15388

Avoid factoring the same polynomial twice

9986afa

Fix mathematical description

2935737

More and better tests for dispersion computation

8819469

An even better implementation of the dispersion algorithm

054d524

Add complete references

9ed6a53

Fix a bug and factor over the correct domain

754d13e

Construct test polys directly

5d319af

asmeurer mentioned this pull request Dec 21, 2013

Gosper improve #2702

Open

raoul added 2 commits December 29, 2013 18:24

Move dispersion code to polynomials module

192f83c

Proper documentation for all dispersion stuff

19f40c7

raoul added 2 commits January 18, 2014 01:42

Add dispersion examples to docstrings

0713afe

Add OO interface in polys for the computation of the dispersion

4fadb0c

raoul added 2 commits January 18, 2014 02:09

Document OO interface to dispersion

bef88f7

-> Fix a documentation issue

4f7d381

asmeurer reviewed Jan 19, 2014
View reviewed changes

sympy/polys/dispersion.py

4. [Man93]_

"""

J = dispersionset(p, q)

if len(J) == 0:

Copy link

Member

asmeurer Jan 19, 2014

Choose a reason for hiding this comment

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

if not J

Address review comments

1ef511b

asmeurer reviewed Jan 19, 2014
View reviewed changes

raoul added 2 commits January 19, 2014 02:21

Allow the caller to specify the generator

a84be26

Test that we can work with Exprs directly

54fbf84

raoulb pushed a commit that referenced this pull request Jan 25, 2014

Merge pull request #2679 from raoulb/dispersion

9874779

Dispersion

raoulb merged commit 9874779 into sympy:master Jan 25, 2014

raoulb deleted the dispersion branch January 25, 2014 14:17

raoulb restored the dispersion branch March 26, 2015 22:42

Dispersion #2679

Dispersion #2679

Conversation

raoulb commented Dec 14, 2013

raoulb commented Dec 14, 2013

raoulb commented Dec 14, 2013

skirpichev commented Dec 14, 2013

raoulb commented Dec 14, 2013

asmeurer commented Dec 17, 2013

asmeurer Dec 17, 2013

Choose a reason for hiding this comment

raoulb Dec 17, 2013

Choose a reason for hiding this comment

asmeurer commented Dec 17, 2013

asmeurer Dec 17, 2013

Choose a reason for hiding this comment

raoulb Dec 17, 2013

Choose a reason for hiding this comment

asmeurer commented Dec 17, 2013

raoulb commented Dec 17, 2013

raoulb commented Dec 17, 2013

raoulb commented Dec 17, 2013

raoulb commented Dec 21, 2013

raoulb commented Dec 21, 2013

asmeurer commented Dec 21, 2013

raoulb commented Dec 21, 2013

asmeurer commented Dec 21, 2013

asmeurer commented Dec 21, 2013

asmeurer commented Dec 21, 2013

raoulb commented Dec 21, 2013

raoulb commented Dec 21, 2013

raoulb commented Dec 21, 2013

raoulb commented Jan 12, 2014

raoulb commented Jan 12, 2014

asmeurer commented Jan 17, 2014

asmeurer commented Jan 17, 2014

raoulb commented Jan 18, 2014

asmeurer commented Jan 18, 2014

raoulb commented Jan 18, 2014

asmeurer Jan 19, 2014

Choose a reason for hiding this comment

asmeurer commented Jan 19, 2014

asmeurer Jan 19, 2014

Choose a reason for hiding this comment

asmeurer Jan 19, 2014

Choose a reason for hiding this comment