-
Notifications
You must be signed in to change notification settings - Fork 157
Division by zero in backward_substitution while fitting spline #119
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
Comments
@GarthSnyder thanks for the bug report. I am guessing it would be a good idea to add a try-except block for
|
It looks like changing the input degree also fixes the issue.
Is there a specific reason for setting degree to 5 @GarthSnyder? Usually quadratic or cubic should be enough. |
Fixed by #121 |
Hi I seem to have this issue when fitting a surface. Here is my reproducible snippet.
Surface_1 results in a index error. Stack trace:
Trying to set the num_ctrl_pts to be 3 seems to have an issue. Surface_2 has this error:
According to the code comments, the size of u or v needs to be such that (size_u + 1) > num_ctrl_pts, so in theory, for a 3x11 patch, the 3 pt direction should still work with 3 ctrlpts right? Environment is: Python 3.7, geomdl 5.3.1, Linux. |
@neevparikh, did you try this on the #121 fork? It would be interesting to know whether that fix addresses the 2D case. The fix is in a utility routine, so it might or might not be in the path for surfaces. |
Oh okay I thought the #121 was merged in. I'll test and report back |
No fix from that sadly. The 2nd surface error changes to the same error as the first one now. |
It's been a while since I looked at NURBS math in detail, but I think you may be running afoul of some intrinsic requirements. I don't think the standard approximation method implemented by NURBS-Python (from The NURBS Book) will extrapolate additional points in a dimension. If you have an input dimension of N, you can't ask for N + M control points as an output in that dimension; it has to be at most N and is N - 1 by default. Similarly, you need at least degree + 1 control points in a dimension. This is a basic requirement for NURBS curves/surfaces. Otherwise, there's nothing to interpolate - all your control points are active at both the start and end of the interval. Your input data is 3 x 11, so the highest number of control points you can obtain in the first dimension is 3. And that limits your output degree for that dimension to 2. Rewriting your example in light of these limits and also using the #121 patch works fine. I'm not sure if it works without the #121 patch. (More specifically, it may run without errors without #121, although I believe the approximation is still potentially biased. You may see ringing in the output surface, but then again you might not. It depends on the input data.)
Note that you have to specify the number of control points in U explicitly for both cases. Otherwise it defaults to 3 - 1 = 2, which is not sufficient to support a degree-2 NURBS. |
Oh I see! I think I'd assumed 3 meant degree 3, but in hindsight that's obviously under determined, because you need n+1 pts to fit an n-dim polynomial. Thanks! |
I wonder if the default behavior should warn the user of this edge case? Or maybe the default number of control pts should be max(size_u - 1, degree_u + 1)? |
That seems like a good idea, but you should probably file it as a separate issue. |
Reproducible crash via division by zero. Should be reproducible from the code below. I took a look at the library code, but the exact issue isn't clear to me. Most likely it's something peculiar about the input, but I'm not sure exactly what.
Looks like
matrix_u[q - 1][q - 1]
is probably zero on line 616 of linalg.py.There are 88 points, and accepting the default number of control points from
fit.approximate_curve
(which I assume is 87) causes the crash, as does any number of control points above this. The points fit just fine if fewer control points are requested.I also notice that rounding all the floating point values avoids the problem, even if you round them to 15 digits. Very strange.
Environment: NURBS-Python 5.3.1, macOS Big Sur. Python 3.7 or 3.8.3. Regular Python is installed via Anaconda, but I'm working on a plugin for Fusion 360 which has its own Python environment; either reproduces the issue.
The error output is:
The text was updated successfully, but these errors were encountered: