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

Series and Series expansion. #41

Merged
merged 18 commits into from
Apr 25, 2016
Merged

Series and Series expansion. #41

merged 18 commits into from
Apr 25, 2016

Conversation

shivamvats
Copy link
Member

@leosartaj I have reopened our PR here. Were you able to compile the code you added?

@shivamvats
Copy link
Member Author

@leosartaj I have added you as a collaborator.

@asmeurer
Copy link
Member

You also could have created a branch on this repo.

@@ -17,6 +17,10 @@ \subsection{Matrices}
% Physics module (some sampling, to show that it is there)
\subsection{Physics}

% Series module (Formal Power Series, Fourier Series)
\subsection{Series}
\input{series.tex}
Copy link
Member

Choose a reason for hiding this comment

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

\input{series} ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This imports content of another .tex file. This way fewer merge conflicts for everyone.

observation that a truncated Taylor series, is in fact a polynomial.
Ring-series uses the efficient representation and operations of sparse
polynomials. The choice of sparse polynomials is deliberate as it performs
well in a wider range of cases than a dense representation. It gives the user
Copy link
Member

Choose a reason for hiding this comment

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

It -> Ring series

@leosartaj
Copy link
Member

Please review. Ping @isuruf @scopatz @shivamvats

@moorepants
Copy link
Member

This needs to be updated wrt to master and still needs a review.

The function \texttt{rs\_series} makes use of these elementary functions to
intelligently expand an arbitrary SymPy expression. Currently it only supports
expansion about 0 and is under active development. Ring Series is several times
faster than the default implementation with the speed difference increasing
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 a comment here. When you have the benchmarks you can add them here. @shivamvats

@shivamvats
Copy link
Member Author

@scopatz Could you please review this?

>>> x, y = symbols('x, y')
>>> series(sin(x+y) + cos(x*y), x, 0, 2)
1 + sin(y) + x*cos(y) + O(x**2)
\end{verbatim}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this code block is tied well to the text around 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.

Resolved.

1 + sin(y) + x*cos(y) + O(x**2)
\end{verbatim}

The newer and much faster approach called Ring Series makes use of the
Copy link
Collaborator

Choose a reason for hiding this comment

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

citation needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

also, I am not sure that the adjective newer is useful here. Maybe alternative.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly with faster, do we have a citation for this or is this something that we can easily prove? If not, we may want to leave it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a few speed comparisons here. What sort of citation would do?

Every series that ring series expands, it does so a couple of times faster than the default series method. We were planning to benchmark the two methods. What would be a good way to go about that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe you could consolidate the information in that link into a table?

1 + sin(y) + x*cos(y) + O(x**2)
\end{verbatim}

The newer and much faster\cite{sympyRingSeries} approach called Ring Series makes use of the
Copy link
Member Author

Choose a reason for hiding this comment

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

@scopatz I have cited the SymPy doc here. Will update the doc with more speed comparisons.

@leosartaj
Copy link
Member

Ping @asmeurer @scopatz

@asmeurer asmeurer merged commit 2debddb into sympy:master Apr 25, 2016
@scopatz
Copy link
Collaborator

scopatz commented Apr 26, 2016

Thanks @leosartaj! Just the one small issue that I am sure to notice in the full read through.

@scopatz
Copy link
Collaborator

scopatz commented Apr 26, 2016

Thanks to @asmeurer for merging.

@leosartaj
Copy link
Member

Thanks @asmeurer @scopatz. We will resolve the issue, in the next PR. 😄

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

6 participants