ENH: add capability to specify orders in pade func #9211
Conversation
Currently interpolate.pade only takes the order of the denominator <m> with the order of the numerator assumed from the order of the inputted array <an>. Added an optional parameter <l> (following standard naming convention of the pade function) which specifies the order of the numerator. This is useful if the order of the inputted array returns a higher order (and computationally more expensive) numerator than needed. This change does not affect any currently written code.
|
I have not yet been accepted to the scipy-dev mailing list. So I cannot discuss the change (as instructed to here https://docs.scipy.org/doc/numpy/dev/gitwash/development_workflow.html#asking-for-your-changes-to-be-merged-with-the-main-repo). |
scipy/interpolate/_pade.py
Outdated
| raise ValueError("Order of q <m> must be smaller than len(an)-1.") | ||
| Akj = eye(N+1, n+1) | ||
| if N > len(an)-1: | ||
| raise ValueError("Order of q+p <m+l> must be smaller than len(an).") |
There was a problem hiding this comment.
These error messages can maybe be improved. The first one is no longer accurate.
There was a problem hiding this comment.
Thank you, a good point.
I left the initial error message in place for when only the parameter m is given. If both parameters are given the first error message is never called. If both parameters are given, we only care about the sum of them, hence why only warning the user about the sum.
If you think it is worthwhile, I could how much they are over by to the error message?
There was a problem hiding this comment.
It'd be nice if you specified the l < 0 part, what if the user themselves passes in l < 0?
|
I've added the recommended changes of checking if l<0 |
|
Looks good modulo a standard complaint to using the lowercase |
|
I changed it to |
scipy/interpolate/_pade.py
Outdated
| The order of the returned approximating polynomial q. | ||
| n : int, optional | ||
| The order of the returned approximating polynomial p. By default, | ||
| the order is len(an)-m. |
There was a problem hiding this comment.
Minor, optional: can wrap code into double backticks. Ditto for p and q.
|
Since p and q are being used as mathematical variables here, I assumed that I would treat them as being inline maths, as is described in the documentation. |
|
Fair enough. +1 from me. |
|
Backticks are still not perfect: single backticks create hyperlinks. Double backticks typeset code, inline math is Thanks @NPDW and congrats with what I believe is your first scipy commit. Keep them coming! |
|
Thank you Evgeni, I'm proud to now be a contributor to such a great product! |
Currently interpolate.pade only takes the order of the denominator
with the order of the numerator assumed from the order of the inputted
array . Added an optional parameter (following standard naming
convention of the pade function) which specifies the order of the
numerator. This is useful if the order of the inputted array returns
a higher order (and computationally more expensive) numerator than needed.
This change does not affect any currently written code.