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

Documentation: polynomials and division #11425

Merged
merged 6 commits into from
Oct 26, 2017
Merged

Documentation: polynomials and division #11425

merged 6 commits into from
Oct 26, 2017

Conversation

jksuom
Copy link
Member

@jksuom jksuom commented Jul 23, 2016

Documentation for some basic concepts in polys.

multiplications before additions and subtractions.
The products of generators thus obtained are called
*monomials*. They are usually written in the form
`x_1^{\nu_1}x_2^{\nu_2}\cdots x_n^{\nu_n}` where the exponents `\nu_i`
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need to use :math:4 +5 `` for this to render?

Copy link
Member Author

Choose a reason for hiding this comment

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

That does not seem necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I thought you had to use the math directive.

Copy link
Member

Choose a reason for hiding this comment

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

Backquotes automatically do inline math. You only need a .. math:: for display math.

@moorepants
Copy link
Member

This is very nice and helpful! Please run sphinx and see if the math renders properly.

@jksuom
Copy link
Member Author

jksuom commented Jul 24, 2016

I have done that. I could put up a html version in Google drive if desired.

@moorepants
Copy link
Member

No need. Just wanted to make sure the math renders. This is fine to merge. It's a great improvement.

properly as an algebraic number.

Divisibility
------------
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to see some code examples in the following sections to show the relevancy to what a user would do in SymPy with polynomials.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is explained, at least to some extent, in the sequel of the same file basics.rst. See http://docs.sympy.org/latest/modules/polys/basics.html, Basic functionality .

Copy link
Contributor

Choose a reason for hiding this comment

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

I would not expect an example necessarily, since this is a section named "Basic Concepts", otherwise in the same file we do have a section named "Basic functionality" (mostly contains code examples) which does contain examples of divisibility i.e div function.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jksuom I didn't saw your comment (commented there on the outdated version). I can't believe I almost commented the same thing :)

Copy link
Member

Choose a reason for hiding this comment

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

Sure that's all fine. I'm just giving you my opinion as a user that doesn't care too much about polynomial theory. The polynomial module's dosctrings and documentation seem to lack a pointed introduction for people that just want to use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The polynomial module's dosctrings and documentation seem to lack a pointed introduction for people that just want to use it.

I think so too. This text aims to give background information for the developers, those who would write the docstrings or implement polynomial methods in symengine.

@jksuom
Copy link
Member Author

jksuom commented Jul 28, 2016

Thanks for the review, @smichr !

Given a family `(x_i)` of symbols, or other suitable objects,
expressions derived from them by repeated addition, subtraction
and multiplication are called *polynomial expressions in the
generators* `x_i`.
Copy link
Member

Choose a reason for hiding this comment

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

And some base field or ring of coefficients.

Copy link
Member

Choose a reason for hiding this comment

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

I see, so they are included in the x_i. In that case, maybe mention "or other suitable objects, including numbers" or something like that. I think I understand what you are defining here, but it's easy to confuse this with what people usually think of as a "polynomial".

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to make a distinction between 'polynomial expressions' and 'polynomials'. The belong to different classes in SymPy. Adding 'including numbers' here seems a good idea. But I did not want to introduce base rings at this point.
(Writing expository texts is always balancing between brevity and completeness.)

@asmeurer
Copy link
Member

The definition in the beginning seems a bit confusing, especially since you jump directly to SymPy expressions, but the definition doesn't exactly match how expressions work in SymPy.

I read through the rest of the document and it looks fine, excepting my comments.

@asmeurer
Copy link
Member

Maybe the division and gcd examples below this could be incorporated into the text.

@jksuom
Copy link
Member Author

jksuom commented Jul 29, 2016

The definition in the beginning seems a bit confusing, especially since you jump directly to SymPy expressions, but the definition doesn't exactly match how expressions work in SymPy.

Can you elaborate on this?

@jksuom
Copy link
Member Author

jksuom commented Jul 29, 2016

Maybe the division and gcd examples below this could be incorporated into the text.

Do you mean that the examples in the next section on 'Basic Functionality' should be moved here?

@asmeurer
Copy link
Member

Do you mean that the examples in the next section on 'Basic Functionality' should be moved here?

Yes

@jksuom
Copy link
Member Author

jksuom commented Jul 29, 2016

What should be done with the rest of 'Basic Functionality'?

In particular the subsections on Division and GCD.

@asmeurer
Copy link
Member

I guess just leave it there at the end. There is no in-depth discussion of factoring and groebner bases (although there could be).

@asmeurer
Copy link
Member

An alternative might be to split out the theory discussion and the examples into separate documents.

@asmeurer
Copy link
Member

Advantages of splitting theory and examples/usage:

  • Separates the theory from distracting (but important) discussions on gotchas, like algebraically dependent generators and behavior of div when the coefficient domain is not a field (currently in the "basic functionality/division" section)
  • Removes potential confusion between presentation of the theory and the implementation in SymPy

Disadvantages:

  • The theory may be easier to follow and more motivated with examples.

What do you think? I think it would be great to have more theoretical discussions like this in the SymPy docs (contingent on people writing it, of course).

@jksuom
Copy link
Member Author

jksuom commented Aug 2, 2016

I think that examples are well suited for this type of introductory texts. Although there are 'theoretical discussions' I have tried to make them readable for everybody explaining only such concepts that seem essential for an understanding of the subject and its implementation.

I have expanded the polynomial part of the text and added examples to the divisibility section.

@asmeurer
Copy link
Member

Do you intend to add more here? There are still a couple of minor typo comments to address, but otherwise I am +1 to merge if you are done.

@jksuom
Copy link
Member Author

jksuom commented Aug 19, 2016

This is as far as I had intended to go. I only wanted to clarify some concepts to help those who want to write docstrings or implement polys methods in symengine.


`a = qb + r`

and such that either `r = 0` or `w(r) < w(b)`.
Copy link
Member

Choose a reason for hiding this comment

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

I would strike 'and' here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you are right. In a mathematical text these conditions are typically added on the same line after the equation with a quad or two to separate them. Here, I felt that some words were needed.

Copy link
Member

Choose a reason for hiding this comment

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

you could add 'are' after 'and' if you want to keep the 'and', but I don't see a problem with just writing

"such that either r = 0 or w(r) < w(b). "

But since you don't refer to 'q' you might just say

"where either r = 0 or w(r) < w(b). "

Copy link
Member Author

Choose a reason for hiding this comment

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

I feel quite comfortable with your first suggestion.

Copy link
Member

@smichr smichr left a comment

Choose a reason for hiding this comment

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

Just a few typos raised by @asmeurer and @smichr.

@jksuom
Copy link
Member Author

jksuom commented Sep 24, 2016

I have been waiting for #11558 to be finished. Then some examples have to be modified. I was planning to fix the minor flaws in the same commit.

@smichr
Copy link
Member

smichr commented Aug 15, 2017

I have been waiting for #11558 to be finished

ping @jksuom -- the blocking PR has been merged so I think this is ready to finish off when you get the chance.

@jksuom
Copy link
Member Author

jksuom commented Aug 16, 2017

Yes, I'll try to find some time for this.

@jksuom
Copy link
Member Author

jksuom commented Aug 16, 2017

The typos should also be fixed. Can you point out those that I didn't find?

@smichr
Copy link
Member

smichr commented Aug 17, 2017

I didn't re-read everything but looked at where comments had been made to see if anything still needed to be done and made comments as necessary.

Symbolic numbers will no more lead to the EX domain
after sympy#11558.
@smichr
Copy link
Member

smichr commented Aug 17, 2017

The comments that I added aren't showing up here. See the commit tab and search for today's date to see the comments I left.

@smichr smichr merged commit 97571bb into sympy:master Oct 26, 2017
@jksuom
Copy link
Member Author

jksuom commented Oct 26, 2017

Thanks! This can now go in the next release.

@jksuom jksuom deleted the division branch October 26, 2017 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants