View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0026339 | Community | OCCT:Modeling Algorithms | public | 2015-06-13 19:56 | 2016-06-05 13:30 |
Reporter | Roman Lygin | Assigned To | bugmaster | ||
Priority | normal | Severity | major | ||
Status | closed | Resolution | fixed | ||
Product Version | 6.9.0 | ||||
Target Version | 6.9.1 | Fixed in Version | 6.9.1 | ||
Summary | 0026339: [Regression in 6.9.0] Projecting a curve hangs | ||||
Description | The curve projection enters a seemingly infinite loop in 6.9.0. This worked correctly in 6.8.0 and 6.7.1 at least. There are several seeming observations when running thorough tests in CAD Exchanger. Preparing each reproducer is time consuming, so only one is provided so far. The severity has been raised to major as hang is effectively loss of data - nothing can be done with the application in this state, also due to the regression status. 6.9.0 contained several fixes in ProjLib. Perhaps one/several is/are to blame. The stack shows: TKGeomBase.dll!Extrema_SequenceOfPOnSurf::Append(const Extrema_POnSurf & T) Line 71 C++ TKGeomBase.dll!Extrema_ExtPS::TreatSolution(const Extrema_POnSurf & PS, const double Val) Line 133 C++ TKGeomBase.dll!Extrema_ExtPS::Perform(const gp_Pnt & thePoint) Line 347 C++ TKGeomBase.dll!Extrema_ExtPS::Extrema_ExtPS(const gp_Pnt & theP, const Adaptor3d_Surface & theS, const double theTolU, const double theTolV, const Extrema_ExtFlag theF, const Extrema_ExtAlgo theA) Line 173 C++ TKGeomBase.dll!Function_Value(const double U, const Handle_Adaptor3d_HSurface & Surf, const Handle_Adaptor3d_HCurve & Curve, const Handle_Adaptor2d_HCurve2d & InitCurve2d, const double DistTol3d, const double tolU, const double tolV) Line 266 C++ TKGeomBase.dll!ProjLib_PolarFunction::Value(const double theT, NCollection_Array1<gp_Pnt2d> & thePnt2d, NCollection_Array1<gp_Pnt> & __formal) Line 338 C++ TKGeomBase.dll!AppCont_LeastSquare::AppCont_LeastSquare(const AppCont_Function & SSP, const double U0, const double U1, const AppParCurves_Constraint FirstCons, const AppParCurves_Constraint LastCons, const int Deg, const int myNbPoints) Line 171 C++ TKGeomBase.dll!Approx_FitAndDivide2d::Compute(const AppCont_Function & Line, const double Ufirst, const double Ulast, double & TheTol3d, double & TheTol2d) Line 232 C++ TKGeomBase.dll!Approx_FitAndDivide2d::Perform(const AppCont_Function & Line) Line 157 C++ TKGeomBase.dll!Approx_FitAndDivide2d::Approx_FitAndDivide2d(const AppCont_Function & Line, const int degreemin, const int degreemax, const double Tolerance3d, const double Tolerance2d, const unsigned int cutting, const AppParCurves_Constraint FirstC, const AppParCurves_Constraint LastC) Line 55 C++ TKGeomBase.dll!ProjLib_ComputeApproxOnPolarSurface::ProjectUsingInitialCurve2d(const Handle_Adaptor3d_HCurve & Curve, const Handle_Adaptor3d_HSurface & Surf, const Handle_Adaptor2d_HCurve2d & InitCurve2d) Line 1693 C++ TKGeomBase.dll!ProjLib_ComputeApproxOnPolarSurface::Perform(const Handle_Adaptor2d_HCurve2d & InitialCurve2d, const Handle_Adaptor3d_HCurve & Curve, const Handle_Adaptor3d_HSurface & S) Line 677 C++ TKGeomBase.dll!ProjLib_ComputeApproxOnPolarSurface::ProjLib_ComputeApproxOnPolarSurface(const Handle_Adaptor3d_HCurve & theCurve, const Handle_Adaptor3d_HSurface & theSurface, const double theTolerance3D) Line 429 C++ TKGeomBase.dll!ProjLib_ProjectedCurve::Load(const Handle_Adaptor3d_HCurve & C) Line 432 C++ TKGeomBase.dll!ProjLib_ProjectedCurve::ProjLib_ProjectedCurve(const Handle_Adaptor3d_HSurface & S, const Handle_Adaptor3d_HCurve & C) Line 292 C++ TKShHealing.dll!ShapeConstruct_ProjectCurveOnSurface::PerformByProjLib(Handle_Geom_Curve & c3d, const double First, const double Last, Handle_Geom2d_Curve & c2d, const GeomAbs_Shape __formal, const int __formal, const int __formal) Line 405 C++ TKShHealing.dll!ShapeConstruct_ProjectCurveOnSurface::Perform(Handle_Geom_Curve & c3d, const double First, const double Last, Handle_Geom2d_Curve & c2d, const GeomAbs_Shape __formal, const int __formal, const int __formal) Line 319 C++ TKShHealing.dll!ShapeFix_Edge::FixAddPCurve(const TopoDS_Edge & edge, const Handle_Geom_Surface & surf, const TopLoc_Location & location, const unsigned int isSeam, const Handle_ShapeAnalysis_Surface & sas, const double prec) Line 481 C++ | ||||
Steps To Reproduce | restore a_1886.brep f fixshape r f 1e-5 | ||||
Tags | No tags attached. | ||||
Test case number | bugs moddata_3 bug26339 | ||||
|
a_1886.zip (7,857 bytes) |
|
It might be not a complete hang but severely undermined performance - it takes a few dozens seconds to complete what took a fraction of second in the past: in Release mode, win64, vc12, 6.9.0 Draw[3]> chrono s start; fixshape r f 1e-5; chrono s stop; chrono s show Elapsed time: 0 Hours 0 Minutes 54.2847178604 Seconds CPU user time: 54.132347 seconds CPU system time: 0 seconds |
2015-06-15 09:36 developer |
c2d_new (38,727 bytes) |
2015-06-15 09:36 developer |
c2d_old (13,701 bytes) |
2015-06-15 09:39 developer |
c3d (13,769 bytes) |
2015-06-15 09:39 developer |
s (2,260 bytes) |
2015-06-15 09:40 developer |
2dcurve_difference_beginning.png (42,661 bytes) |
2015-06-15 09:40 developer |
2dcurve_difference_ending.png (50,862 bytes) |
|
Dear msv, In old version we had much better performance, but incorrect result (curvature lost at the beginning and oscillations at the end). In 6.9.0 result is correct but computes very long. For my point of view the reason of bad performance is C0 smoothness of input curve. Thats why I think that it is not a regression, but rather bad performance. Please share your opinion about further steps on this issue. |
|
Dear aml, Thank you for your initial analysis. Let me offer some thoughts as well. The original C0 continuity (of both 3D curve and surface) may come from an input file which can be generated by other CAD system/kernel or by other modeling OCC algorithm (e.g. intersection curve coming from Parasolid). So requirement of C1-continuity may not be respected. 50+ seconds on a single edge in the model which can have thousands or dozen thousands thereof is just prohibitively expensive and makes the whole algorithm impractical. From feasibility perspective, the oscillations and/or lost curvature can be of significantly less evil that unacceptable performance which just invalidates the whole 3D model and leads to data loss in user application (due to natural user's action to kill the application that hanged from his/her perspective). Please try to consider other trade-offs, user-defined parameters, and overall performance profiling and improvements. I would also appreciate your indication of the OCC bugfix that triggered this regression, so that we could at least roll-back to a more practical OCC version. Thank you for your work. Roman |
|
Dear Alexander, please prepare a specification (proposal) that is aimed to improve performance of approximator when it deals with non-regular distributions. Dear Roman, the fix that made this performance change is from bug fix 0017129. |
|
Branch CR26339 has been created by aml. SHA-1: 5dd32bdbb76a3b9d56614be479fff41385ab2e29 Detailed log of new commits: Author: aml Date: Wed Jun 17 10:46:04 2015 +0300 0026339: [Regression in 6.9.0] Projecting a curve hangs Changed computation of point projection to more correct. |
|
Improved performance nearly 20 times. Draw[4]> chrono s start; fixshape r f 1e-5; chrono s stop; chrono s show Elapsed time: 0 Hours 0 Minutes 2.68899735843 Seconds CPU user time: 2.5584164 seconds CPU system time: 0.1248008 seconds |
|
Hello Alexander, Thank you for your progress. Still 2.5s per edge is prohibitively expensive, it still makes the overall Shape Healing not feasible for models where just a few curves like this may occur. Thank you. |
|
We cannot return to the version, where quality is too bad. So, the only way to go is to improve performance. One major step in this direction is done. I propose to fix this success. Therefore, it is needed to test the patch for regressions and integrate this solution. I propose to create a new bug to make next improvement of performance of this function. |
|
Hi Mikhail, Committing a patch that already makes a relief is certainly worth doing. Those who use the master branch will benefit. Whether to create a follow up bug to complete the issue or to do this within this frame is up to you as a bug owner. Frankly I don't see convincing benefits of the fork but this is not a strong opinion. My concern as a user, is to get the issue fixed and rather sooner than later; the owner has to apply smart judgment to do what it takes. Thank you, Roman P.S. #17129 is not available to public, however its low number indicates it should be something very old. If the bug can be disclosed please do. That might be helpful to get initial information. Thanks |
|
Roman, the mentioned bug contains confidential data and cannot be disclosed. But you can see the corresponding patch with comments in the GIT repository. BTW, that patch was made not so long time ago, 26.02.2015. |
|
No worries Mikhail. Yes, I tracked it down in the git log. |
|
Branch CR26339 has been updated by aml. SHA-1: 7aa5018f2c38217943488e61fb400337356ade67 Detailed log of new commits: Author: aml Date: Wed Jun 17 22:14:56 2015 +0300 Calculation periodicity information added to cache. |
|
Another improvement, speedup: ~ 1.66 (40% reduced computation time). |
|
Dear msv, Please check current state of branch CR26339. |
|
Reviewed. |
|
Branch CR26339 has been updated forcibly by mkv. SHA-1: 66ed942d6e6d18a2221ac33901effa6853297eaa |
|
Dear BugMaster, Branch CR26339 was rebased on branch IR-2015-06-18 of occt git-repository. SHA-1: 66ed942d6e6d18a2221ac33901effa6853297eaa |
|
Dear BugMaster, Branch CR26339 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode. SHA-1: 66ed942d6e6d18a2221ac33901effa6853297eaa Number of compiler warnings: occt component : Linux: 25 (25 on master) Windows: 0 (0 on master) products component : Linux: 37 (37 on master) Windows: 0 (0 on master) Regressions/Differences/Improvements: No regressions/differences Testing cases: http://occt-tests/CR26339-master-occt-64/Debian70-64/bugs/moddata_3/bug26339.html http://occt-tests/CR26339-master-occt-64/Windows-64-VC10/bugs/moddata_3/bug26339.html bugs moddata_3 bug26339: OK Testing on Linux: occt component : Total MEMORY difference: 97483975 / 97880880 [-0.41%] Total CPU difference: 18855.539999999964 / 18426.10000000013 [+2.33%] products component : Total MEMORY difference: 24142093 / 24294294 [-0.63%] Total CPU difference: 8153.45000000002 / 7856.280000000035 [+3.78%] Testing on Windows: occt component : http://occt-tests/CR26339-master-occt-64/Windows-64-VC10/diff-Windows-64-VC10.html CPU bugs moddata_1 bug20391: 16.6141065 / 5.6940365 [+191.78%] CPU bugs moddata_1 bug22194: 8.2992532 / 1.3728088 [+504.55%] CPU bugs step bug24383: 12.1836781 / 5.7564369 [+111.65%] CPU bugs step bug16351: 71.1208559 / 45.6614927 [+55.76%] CPU bugs moddata_2 bug466: 3.588023 / 0.2652017 [+1252.94%] CPU bugs modalg_2 bug22109_2: 4.0092257 / 2.3088148 [+73.65%] CPU bugs modalg_2 bug22109_5: 3.9156251 / 2.1372137 [+83.21%] CPU bugs modalg_2 bug22109_3: 3.9936256 / 2.2464144 [+77.78%] CPU bugs modalg_2 bug22109_1: 3.9936256 / 2.2620145 [+76.55%] CPU bugs modalg_2 bug22109_4: 3.9780255 / 2.3088148 [+72.30%] CPU bugs fclasses bug24931: 17.5501125 / 5.8032372 [+202.42%] Total MEMORY difference: 56516561 / 56796542 [-0.49%] Total CPU difference: 17915.669643298454 / 17625.60138389885 [+1.65%] products component : Total MEMORY difference: 15542140 / 15547034 [-0.03%] Total CPU difference: 6199.308138899959 / 6414.527118499968 [-3.36%] There are no differences in images found by testdiff. |
|
Branch CR26339 has been updated by mkv. SHA-1: e69cf2a31fb9dc12c173a0d10fd6c64c5d01cf6d Detailed log of new commits: Author: mkv Date: Fri Jun 19 14:10:10 2015 +0300 Test case for issue CR26339 |
|
Dear aml, could you please review following test case bugs moddata_3 bug26339 |
|
Branch CR26339 has been updated forcibly by aml. SHA-1: 8b330574dff3e21f60ef33cdd1929e43c24feaee |
|
Dear mkv, I Please re-test updated branch CR26339. II Test case bugs moddata_3 bug26339 is OK. |
|
Branch CR26339 has been updated forcibly by mkv. SHA-1: ccc2596ef29d6092be9c014d94677aedc4f103f2 |
|
Dear BugMaster, Branch CR26339 was rebased on current master of occt git-repository. SHA-1: ccc2596ef29d6092be9c014d94677aedc4f103f2 |
|
Dear BugMaster, Branch CR26339 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode. SHA-1: ccc2596ef29d6092be9c014d94677aedc4f103f2: Number of compiler warnings: occt component : Linux: 25 (25 on master) Windows: 0 (0 on master) products component : Linux: 37 (37 on master) Windows: 0 (0 on master) Regressions/Differences/Improvements: Bad performance: http://occt-tests/CR26339-master-occt-64/Windows-64-VC10/bugs/moddata_1/bug22759.html bugs moddata_1 bug22759: OK - TOTAL CPU TIME: 640.5557061 sec; cpulimit 1000 See reference test: http://occt-tests/IR-2015-06-18-IR-2015-06-18-occt-64/Windows-64-VC10/bugs/moddata_1/bug22759.html bugs moddata_1 bug22759: OK - TOTAL CPU TIME: 281.7846063 sec; cpulimit 400 Testing cases: http://occt-tests/CR26339-master-occt-64/Debian70-64/bugs/moddata_3/bug26339.html http://occt-tests/CR26339-master-occt-64/Windows-64-VC10/bugs/moddata_3/bug26339.html bugs moddata_3 bug26339: OK Testing on Linux: occt component : Total MEMORY difference: 97519719 / 98253056 [-0.75%] Total CPU difference: 18024.15999999997 / 18552.66000000014 [-2.85%] products component : Total MEMORY difference: 24195623 / 24306506 [-0.46%] Total CPU difference: 7833.040000000021 / 7856.900000000035 [-0.30%] Testing on Windows: occt component : http://occt-tests/CR26339-master-occt-64/Windows-64-VC10/diff-Windows-64-VC10.html CPU mesh advanced_incmesh_parallel B5: 40.9814627 / 24.6793582 [+66.06%] CPU bugs moddata_1 bug22759: 640.5557061 / 281.7846063 [+127.32%] Total MEMORY difference: 56769952 / 56802504 [-0.06%] Total CPU difference: 17654.35236819892 / 17631.155019498852 [+0.13%] products component : Total MEMORY difference: 15546000 / 15550884 [-0.03%] Total CPU difference: 6806.41723060001 / 6414.932721099968 [+6.10%] There are no differences in images found by testdiff. |
|
Dear bugmaster, I have built OCCT master and CR26339 on my PC in x64 Release config and run the problematic tests several times. I have obtained the following CPU times: mesh advanced_incmesh_parallel B5 - about 18 sec in both branches. bugs moddata_1 bug22759 - 380 sec in CR26339, and timeout (>400 sec) in master. So, I propose to mark these tests as unstable. |
|
Branch CR26339 has been updated by mkv. SHA-1: c8aed7e2db2cbd2dbebaaf2d46a151da29ce0885 Detailed log of new commits: Author: mkv Date: Tue Jun 30 15:07:09 2015 +0300 Small correction of test case for issue CR26339 |
|
Dear BugMaster, following test case bugs moddata_1 bug22759 is corrected, retested and pushed to branch CR26339 of occt git-repository. Now it is OK. |
|
Dear BugMaster, Branch CR26339 is TESTED. |
|
Branch CR26339 has been deleted by inv. SHA-1: c8aed7e2db2cbd2dbebaaf2d46a151da29ce0885 |
occt: master ea3b6e72 2015-07-02 11:19:13
Committer: bugmaster Details Diff |
0026339: [Regression in 6.9.0] Projecting a curve hangs Changed computation of point projection to more correct. Calculation periodicity information added to cache. Test case for issue CR26339 Small correction of test case for issue CR26339 |
Affected Issues 0026339 |
|
mod - src/ProjLib/ProjLib_ComputeApproxOnPolarSurface.cxx | Diff File | ||
mod - tests/bugs/moddata_1/bug22759 | Diff File | ||
add - tests/bugs/moddata_3/bug26339 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2015-06-13 19:56 | Roman Lygin | New Issue | |
2015-06-13 19:56 | Roman Lygin | Assigned To | => msv |
2015-06-13 19:56 | Roman Lygin | File Added: a_1886.zip | |
2015-06-13 19:56 | Roman Lygin | Severity | minor => major |
2015-06-13 20:06 | Roman Lygin | Note Added: 0042122 | |
2015-06-14 00:52 |
|
Assigned To | msv => aml |
2015-06-14 00:52 |
|
Status | new => assigned |
2015-06-15 09:36 |
|
File Added: c2d_new | |
2015-06-15 09:36 |
|
File Added: c2d_old | |
2015-06-15 09:36 |
|
File Added: c3d | |
2015-06-15 09:37 |
|
File Deleted: c3d | |
2015-06-15 09:39 |
|
File Added: c3d | |
2015-06-15 09:39 |
|
File Added: s | |
2015-06-15 09:40 |
|
File Added: 2dcurve_difference_beginning.png | |
2015-06-15 09:40 |
|
File Added: 2dcurve_difference_ending.png | |
2015-06-15 09:47 |
|
Note Added: 0042129 | |
2015-06-15 09:47 |
|
Assigned To | aml => msv |
2015-06-15 09:47 |
|
Status | assigned => feedback |
2015-06-15 10:57 | Roman Lygin | Note Added: 0042130 | |
2015-06-15 12:18 |
|
Note Added: 0042132 | |
2015-06-15 12:18 |
|
Assigned To | msv => aml |
2015-06-15 12:18 |
|
Status | feedback => assigned |
2015-06-17 13:38 | git | Note Added: 0042187 | |
2015-06-17 13:40 |
|
Note Added: 0042188 | |
2015-06-17 13:41 |
|
Note Edited: 0042188 | |
2015-06-17 14:20 | Roman Lygin | Note Added: 0042194 | |
2015-06-17 16:08 |
|
Note Added: 0042198 | |
2015-06-17 16:21 | Roman Lygin | Note Added: 0042199 | |
2015-06-17 17:24 |
|
Note Added: 0042203 | |
2015-06-17 17:45 | Roman Lygin | Note Added: 0042208 | |
2015-06-17 22:16 | git | Note Added: 0042217 | |
2015-06-17 22:19 |
|
Note Added: 0042218 | |
2015-06-18 08:12 |
|
Relationship added | parent of 0026350 |
2015-06-18 08:13 |
|
Note Added: 0042219 | |
2015-06-18 08:13 |
|
Assigned To | aml => msv |
2015-06-18 08:13 |
|
Status | assigned => resolved |
2015-06-18 11:32 |
|
Note Added: 0042224 | |
2015-06-18 11:32 |
|
Assigned To | msv => bugmaster |
2015-06-18 11:32 |
|
Status | resolved => reviewed |
2015-06-18 12:45 |
|
Assigned To | bugmaster => mkv |
2015-06-18 17:49 | git | Note Added: 0042241 | |
2015-06-19 14:10 |
|
Note Added: 0042267 | |
2015-06-19 14:12 |
|
Note Added: 0042268 | |
2015-06-19 14:12 |
|
Assigned To | mkv => aml |
2015-06-19 14:12 |
|
Status | reviewed => feedback |
2015-06-19 14:12 | git | Note Added: 0042269 | |
2015-06-19 14:13 |
|
Note Added: 0042270 | |
2015-06-19 14:13 |
|
Test case number | => bugs moddata_3 bug26339 |
2015-06-24 07:21 | git | Note Added: 0042392 | |
2015-06-24 07:22 |
|
Note Added: 0042393 | |
2015-06-24 07:22 |
|
Assigned To | aml => mkv |
2015-06-24 07:22 |
|
Status | feedback => reviewed |
2015-06-24 14:29 | git | Note Added: 0042411 | |
2015-06-25 13:15 |
|
Note Added: 0042442 | |
2015-06-25 13:16 |
|
Note Added: 0042443 | |
2015-06-25 13:16 |
|
Assigned To | mkv => aml |
2015-06-25 13:16 |
|
Status | reviewed => feedback |
2015-06-26 11:57 |
|
Note Added: 0042485 | |
2015-06-26 12:07 |
|
Assigned To | aml => bugmaster |
2015-06-30 15:07 | git | Note Added: 0042598 | |
2015-06-30 15:08 |
|
Note Added: 0042599 | |
2015-06-30 15:08 |
|
Status | feedback => tested |
2015-06-30 15:08 |
|
Note Added: 0042600 | |
2015-07-03 15:27 | bugmaster | Changeset attached | => occt master ea3b6e72 |
2015-07-03 15:27 | bugmaster | Status | tested => verified |
2015-07-03 15:27 | bugmaster | Resolution | open => fixed |
2015-08-14 10:57 | git | Note Added: 0044212 | |
2015-08-26 11:08 |
|
Target Version | 7.0.0 => 6.9.1 |
2015-10-16 14:56 |
|
Status | verified => closed |
2015-10-23 20:51 |
|
Fixed in Version | => 6.9.1 |
2016-06-05 13:30 | Roman Lygin | Relationship added | related to 0027569 |