MantisBT - Open CASCADE
View Issue Details
0030908Open CASCADE[OCCT] PRODUCTS:BestFitpublic2019-08-21 14:112019-09-07 16:31
avn 
bugmaster 
normalmajor 
verifiedfixed 
WindowsVC++ 201364 bit
[OCCT] 7.4.0 
[OCCT] 7.3.0 
bfit loc_opt bug30908
0030908: BestFit - Incorrect convergence of local optimization.
Using BFGS may give the wrong result. It requires change of the local optimizer.
test bfit loc_opt bug30908
No tags attached.
? teapot.brep (2,374,465) 2019-08-23 14:59
https://tracker.dev.opencascade.org/
? teapot1.xyz (1,427,679) 2019-08-23 14:59
https://tracker.dev.opencascade.org/
Issue History
2019-08-21 14:11avnNew Issue
2019-08-21 14:11avnAssigned To => avn
2019-08-23 14:59avnFile Added: teapot.brep
2019-08-23 14:59avnFile Added: teapot1.xyz
2019-08-26 08:50avnNote Added: 0086444
2019-08-26 08:50avnAssigned Toavn => aml
2019-08-26 08:50avnStatusnew => resolved
2019-08-26 08:53avnNote Edited: 0086444bug_revision_view_page.php?bugnote_id=86444#r21663
2019-08-27 07:40amlNote Added: 0086456
2019-08-27 07:40amlAssigned Toaml => avn
2019-08-27 07:40amlStatusresolved => assigned
2019-08-28 15:12avnNote Added: 0086485
2019-08-28 15:12avnAssigned Toavn => aml
2019-08-28 15:12avnStatusassigned => resolved
2019-08-29 07:17amlNote Added: 0086496
2019-08-29 07:17amlAssigned Toaml => avn
2019-08-29 07:17amlStatusresolved => feedback
2019-08-29 09:05avnAssigned Toavn => aml
2019-08-29 09:06avnNote Added: 0086500
2019-08-29 09:17amlSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=21671#r21671
2019-08-29 09:20amlNote Added: 0086501
2019-08-29 09:20amlAssigned Toaml => bugmaster
2019-08-29 09:20amlStatusfeedback => reviewed
2019-08-29 17:49apnTest case number => bfit loc_opt bug30908
2019-08-29 18:45apnNote Added: 0086537
2019-08-29 18:45apnStatusreviewed => tested
2019-08-30 19:06apnNote Added: 0086557
2019-08-30 19:06apnAssigned Tobugmaster => avn
2019-08-30 19:06apnStatustested => assigned
2019-09-02 09:14avnAssigned Toavn =>
2019-09-02 09:15avnAssigned To => aml
2019-09-02 09:15avnNote Added: 0086607
2019-09-02 09:17avnStatusassigned => resolved
2019-09-02 09:19amlNote Added: 0086608
2019-09-02 09:19amlAssigned Toaml => bugmaster
2019-09-02 09:19amlStatusresolved => reviewed
2019-09-02 17:16apnStatusreviewed => tested
2019-09-02 17:19kgvNote Added: 0086623
2019-09-02 17:20kgvAssigned Tobugmaster => avn
2019-09-02 17:20kgvStatustested => assigned
2019-09-02 17:21kgvNote Edited: 0086623bug_revision_view_page.php?bugnote_id=86623#r21695
2019-09-02 17:21kgvNote Edited: 0086623bug_revision_view_page.php?bugnote_id=86623#r21696
2019-09-03 16:30amlAssigned Toavn => aml
2019-09-03 16:33amlNote Added: 0086693
2019-09-05 06:51amlNote Added: 0086769
2019-09-05 06:51amlAssigned Toaml => kgv
2019-09-05 06:51amlStatusassigned => resolved
2019-09-05 07:01kgvAssigned Tokgv => bugmaster
2019-09-05 07:01kgvStatusresolved => reviewed
2019-09-05 18:23bugmasterNote Added: 0086812
2019-09-05 18:23bugmasterStatusreviewed => tested
2019-09-07 11:48bugmasterNote Added: 0086920
2019-09-07 11:48bugmasterStatustested => verified
2019-09-07 11:48bugmasterResolutionopen => fixed

Notes
(0086444)
avn   
2019-08-26 08:50   
(edited on: 2019-08-26 08:53)
http://occt-tests/master-CR30908-avn-Products/Windows-64-VC14/diff_summary.html [^]
branch - CR30908

(0086456)
aml   
2019-08-27 07:40   
1) tests/bfit/end
Misprint: &optimizer -> $optimizer

2) tests/bfit/pn100/begin
set shape_file [locate_data_file Propeller.rle]

set optimizer "BFGS"

Remove the extra empty line.

3) tests/bfit/pn1000/begin
set shape_file [locate_data_file Propeller.rle]

set optimizer "BFGS"

Remove the extra empty line.

4) tests/bfit/pn10000/begin
set shape_file [locate_data_file Propeller.rle]

set optimizer "BFGS"

Remove the extra empty line.

5) tests/bfit/teapot
Please rename group to the "loc_opt".

6) tests/bfit/teapot/A1
In the OCCT regression tests are named by the corresponding issue name. Please rename test to the bug30908.

7) src/BestFitTest/BestFitTest.cxx::975-981:
  theDI.Add("bfRunFit", " [approxToFirstProj(0|1) [freedom(XYZABG) [nbIter [optimizer]]]]:\n \
                          \tRun best fit algoritm with following optional arguments:\n \
                          \tapproximation to first projection ( bool value 0 or 1 - default is 0 )\n \
                          \tfreedom of degree value ( X Y Z ALFA BETA GAMMA ) - default XYZABG - all degrees\n \
                          \tmax nb of iterations - default is 200\n \
                          \tlocal optimizer - default is BFGS",
                          __FILE__, bestfit, g);
\tlocal optimizer it should be \toptimizer. Also, add available options.

8) src/BestFitAPI/BestFitAPI_Algo.hxx::366:
Add doxygen comment for the new variable myOptimizer.

9) src/BestFitAPI/BestFitAPI_Algo.cxx::684-714:
Code duplication. It is possible to extract the following block:

    if ( aStatus )
      OUT_LOG ( "\nNumber of really done iterations:\t" << aTool.NbIterations() );
    else {
      // If algorithm failed -- well, try at least to take minimal value found during its execution...
      OUT_LOG ( "\nNo convergency; taking best found solution\t" );
      double val;
      aFunct.Value ( aVec, val );
      
The actual number of iterations should be extracted for that. Please retest after that.
(0086485)
avn   
2019-08-28 15:12   
http://occt-tests/CR30908-master-avn-Products/Debian80-64/diff_summary.html [^]
http://occt-tests/CR30908-master-avn-Products/Windows-64-VC14/diff_summary.html [^]
(0086496)
aml   
2019-08-29 07:17   
Reviewed with minor cosmetic changed. Dear Andrey, could you please launch testing again?
(0086500)
avn   
2019-08-29 09:06   
http://occt-tests/CR30908-master-avn-Products/Windows-64-VC14/summary.html [^]
http://occt-tests/CR30908-master-avn-Products/Debian80-64/summary.html [^]
(0086501)
aml   
2019-08-29 09:20   
Reviewed. Please add attached shapes for successful testing.
(0086537)
apn   
2019-08-29 18:45   
Combination -
OCCT branch : master
Products branch : CR30908 SHA - a1d02c2d4e601da8c8993b3bc1fd6f1b67a5e50c
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
---
Products
Total CPU difference: 10489.820000000056 / 10489.71000000005 [+0.00%]
Windows-64-VC14:
OCCT
---
Products
Total CPU difference: 12078.46875 / 12068.71875 [+0.08%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0086557)
apn   
2019-08-30 19:06   
There are following warnings on Windows-win32-vc10 platform:

BestFitTest.cxx:569, MSBuild, Priority: Normal
nonstandard extension used: enum 'BestFitAPI_Algo::LocalOptimization' used in qualified name

BestFitTest.cxx:604, MSBuild, Priority: Normal
nonstandard extension used: enum 'BestFitAPI_Algo::LocalOptimization' used in qualified name

BestFitTest.cxx:606, MSBuild, Priority: Normal
nonstandard extension used: enum 'BestFitAPI_Algo::LocalOptimization' used in qualified name

BestFitAPI_Algo.cxx:102, MSBuild, Priority: Normal
nonstandard extension used: enum 'BestFitAPI_Algo::LocalOptimization' used in qualified name

BestFitAPI_Algo.cxx:685, MSBuild, Priority: Normal
nonstandard extension used: enum 'BestFitAPI_Algo::LocalOptimization' used in qualified name

BestFitAPI_Algo.cxx:691, MSBuild, Priority: Normal
nonstandard extension used: enum 'BestFitAPI_Algo::LocalOptimization' used in qualified name
(0086607)
avn   
2019-09-02 09:15   
http://occt-tests/CR30908_2-master-avn-Products/Windows-64-VC14/diff_summary.html [^]
http://occt-tests/CR30908_2-master-avn-Products/Debian80-64/diff_summary.html [^]
(0086608)
aml   
2019-09-02 09:19   
Reviewed.
(0086623)
kgv   
2019-09-02 17:19   
(edited on: 2019-09-02 17:21)
+  if ( myOptimizer == OptimizationAlgo_BFGS )
+  else if( myOptimizer == OptimizationAlgo_FRPR )

tip: if this is enumeration, then it is peferrable using switch() without default statement.
Compiler emits warning in switch() statements for new enumeration values (when enum is extended and some use cases have been forgotten to be updated).

+  enum OptimizationAlgo {
+    OptimizationAlgo_BFGS = 0,
+    OptimizationAlgo_FRPR
+  };

Looks like documentation is missing describing what these magic letters mean.

+void BestFitAPI_Algo::SetOptimizer (const OptimizationAlgo theOptimizer)
+{
+  myOptimizer = theOptimizer;

nit: seems to be can be inlined.

+  //! Returns used optimizer
+  Standard_EXPORT OptimizationAlgo GetOptimizer () const { return myOptimizer; 

Standard_EXPORT should NOT be used for inline method (leads to exports pollution).

(0086693)
aml   
2019-09-03 16:33   
I will take care of it.
(0086769)
aml   
2019-09-05 06:51   
Dear Kirill,
Your remarks are corrected. Could you please take a look at the CR30908_2 branch?

Testing results:
http://occt-tests/master-CR30908_2-aml-Products/Windows-64-VC14/diff_summary.html [^]
http://occt-tests/master-CR30908_2-aml-Products/Debian80-64/diff_summary.html [^]
(0086812)
bugmaster   
2019-09-05 18:23   
Combination -
OCCT branch : master
Products branch : CR30908_2 SHA - 439d4da35266bbb06a635b9efd1d06c025ed22ad
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
---
Products
Total CPU difference: 10497.040000000057 / 10484.930000000048 [+0.12%]
Windows-64-VC14:
OCCT
---
Products
Total CPU difference: 12069.3125 / 12168.265625 [-0.81%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0086920)
bugmaster   
2019-09-07 11:48   
Fix has been integrated into master of occt-products repository