-
Notifications
You must be signed in to change notification settings - Fork 116
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
[FEATURE REQUEST] Add numpy.polynomial.polynomial.Polynomial.roots() #355
Comments
@jrmoserbaltimore Just to make sure that we are on the same page, you mean some of the functions from this package: https://numpy.org/doc/stable/reference/routines.polynomials.package.html
As far as I understand, you want to fit a quadratic polynomial, right? But you can find the roots of that via a closed formula. I wonder, whether the following snippet would actually address the issue epsilon = 1e-6
def root2(p):
if p[2] < epsilon:
raise ValueError('input is a linear polynomial')
if p[1] ** 2 < 4 * p[0] * p[2]:
raise ValueError('polynomial has no real roots')
sqrt_root = sqrt(p[1] ** 2 - 4 * p[0] * p[2])
return (-p[1] + sqrt_root) / (2 * p[2]), (-p[1] - sqrt_root) / (2 * p[2]),
p = np.polyfit(data, 2)
data = p[0] - data From your description, I don't actually see, where you need the roots, but you would get it with the I am not against adding new functions, but in this case, I have the feeling that the functionality can easily be implemented in |
True. Part of this is just looking at cross-platform compatibility, writing code to function in both without writing wrappers around If you did implement it, it would be viable to just raise an exception if finding the root with order |
OK, I haven't known about this, and this is definitely a valid point. But this would simply mean that we would have to re-label the
That's fine, but I see another issue here, namely, what should happen, if the roots are complex? If we wanted to keep the number of supported Finally, could you spell out the differences between |
It's considered old API but I may be misreading. On a closer look it looks like they just moved the location, but it's the same function. I want to say I'm a politician, not a programmer, but it's more accurate to say I'm just a bad programmer 😄
Shrug, then don't. I'm mostly interested in being able to do stuff like: try:
from numpy.polynomial.polynomial import Polynomial
except ImportError:
from ulab.numpy import Polynomial or wherever in
Probably not. It'd be viable to include |
OK, so we could simply re-package the functions that we already have, and add the roots for
But you still can't avoid the difficulty of having complex roots of a real polynomial. Something like this:
This last line would fail in from ulab import numpy as np
np.polynomial....
I wanted to bring this up: |
Fair enough. I didn't find the roots function in I'll think on this some more; lots of stuff I didn't consider or realize here. |
No, it is not there, so don't look for it! |
@jrmoserbaltimore Here is a new issue, you could comment on the specific questions there: #364 |
@jrmoserbaltimore I have just opened a draft under #366. With that, we should be able to support the Also, you should think about what you do, if you expect real output, but the results are slightly imaginary, i.e., you have something like
The In other words, do you need facilities to guard against such cases? If so, what should they be? Are you going to implement those in |
Describe the solution you'd like
ulab
already haspolyfit()
, but notPolynomial
.I would like to see
Polynomial
implementingPolynomial.fit()
andPolynomial.roots()
, as well asPolynomial.coef
as innumpy
.Additional context
For certain sensors, particularly Hall effect sensors, the reading is a function of the square of the distance to an object, or some other polynomial relationship. With calibration data, a curve can be generated with
Polynomial.fit()
orpolyfit()
; to get the real-world reading, subtract the sensor reading from the final coefficient (e.g.ax**2+bx+c
, subtract the raw sensor reading fromc
) and find the roots. For a Hall sensor, the smallest root is then the real-world distance.ulab
doesn't implementroots()
, so this is not possible today. A partial implementation ofPolynomial
would be good for compatibility with the modernnumpy
library.The text was updated successfully, but these errors were encountered: