Configuration, NCollection_UBTreeFiller - do not use _REENTRANT in a header file

Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: msv
Date: Mon Dec 28 12:35:34 2015 +0300

    0026939: Configuration, NCollection_UBTreeFiller - do not use _REENTRANT in a header file
    
    Make the code always using the thread-safe function rand_r.

Please review.

Dear Mikhail,

rand_r is not part of Android NDK - and this is the reason for this bug (sorry for incomplete description).
Maybe some OCCT generator can be used instead (like math_BullardGenerator, but it is located in another package)?

Also, rand_r is not available on Windows (at least, MSDN does not mention it, and patch does not build). Note that modern C++ provides a set of standard random generators in <random> header, I suggest one of these should be used instead of C function. I guess that std::mt19937 should work just fine.

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Jan 22 14:28:18 2016 +0300

    0026939: Configuration, NCollection_UBTreeFiller - do not use _REENTRANT in a header file
    
    Use std::mt19937 random number generator instead of rand() in NCollection_UBTreeFiller.

Dear Mikhail,

please take a look on the patch in branch CR26939_1 (based on suggestion from Andrey).

Remarks:

1) It seems RAND_MAX is no more needed. Please, remove corresponding code.
2) I think the following includes can be removed:
#include <stdlib.h>
#include <stdio.h>

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Jan 22 15:55:07 2016 +0300

    remove redundant code

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Jan 22 15:55:55 2016 +0300

    0026939: Configuration, NCollection_UBTreeFiller - do not use _REENTRANT in a header file
    
    Use std::mt19937 random number generator instead of rand() in NCollection_UBTreeFiller.

Remarks have been applied in updated patch.

Reviewed.

Dear Commenter 3,
Branch CR26939_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 1 (1 on master)

products component :
Linux: 36 (36 on master)
Windows: 0 (0 on master)

Regressions/Differences/Improvements:

http://occt-tests/CR26939-2-master-occt-64/Debian70-64/summary.html
Improvements:
  boolean volumemaker D5
Failed:
  boolean volumemaker B6, C9, D2, H4
  bugs modalg_1 buc60462_2

http://occt-tests/CR26939-2-master-occt-64/Windows-64-VC10/summary.html
Improvements:
  boolean gdml_private ZI7, ZJ7
Failed:
  boolean volumemaker B6, C9, D2
  bugs modalg_1 buc60462_2

http://occt-tests/CR26939-2-master-products-64/Debian70-64/summary.html
Failed:
  bfit pnt100 A4
  bfit pnt1000 A4, A7
  bfit pnt10000 A4

http://occt-tests/CR26939-2-master-products-64/Windows-64-VC10/summary.html
Failed:
  bfit pnt100 A4

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 89373411 / 89608145 [-0.26%]
Total CPU difference: 19227.640000000167 / 19150.610000000095 [+0.40%]
products component :

Testing on Windows:
occt component :
Total MEMORY difference: 57221763 / 57247969 [-0.05%]
Total CPU difference: 17947.930650098806 / 18414.139638599045 [-2.53%]
products component :
Total MEMORY difference: 17239751 / 17252669 [-0.07%]
Total CPU difference: 5772.505002999976 / 6200.821348599963 [-6.91%]

There are no differences in images found by testdiff.

Dear kgv,
Branch [archived branch] has been rejected due to:
- regressions/differences/improvements

The changes are caused by variation of behavior of UBTree due to changed sequence of random numbers (note that UBTree is rather widely used in BOP now). The dependency of result on random sequence is indication of flawed logic of the algorithms, however this should be treated separately.

As soon as the new result is stable (only the algorithm has changed), I propose we can accept this as current state of the code.

By logs, regressions are compensated by improvements:

boolean volumemaker D5 on Linux: is actual improvement (behavior is correct, like on Windows)

boolean volumemaker B6, C9, D2: formal regression (faulties), but visually result is the same (invalid)

boolean volumemaker D5: Linux-specific regression (bopcheck failed)

boolean gdml_private ZI7 ZJ7: looks like improvement (checkshape), Linux only

bugs modalg_1 buc60462_2: looks like real improvement (invalid result instead of complete failure)

I agree to accept the current state.

Dear Andrey, please update test cases.

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

Branch [archived branch] has been updated by Commenter 2.

[revision removed]


Detailed log of new commits:

Author: Commenter 2
Date: Wed Feb 3 15:00:44 2016 +0300

    boolean gdml_private ZI7 ZJ7 - TODO "bopcheck failed" is only for Linux now, checkshape faulty is unstable (issue #27052)
    boolean volumemaker B6 - Added TODO (bopcheck and checkshape faulties)
    boolean volumemaker C9 - Added TODO (checkprops and checkshape faulties)
    boolean volumemaker D2 - Added TODO (checkshape faulty)
    boolean volumemaker H4 - Added TODO (checkprops and checkshape faulties Linux only)
    boolean volumemaker D5 - IMVPROVEMENT, TODOs were deleted (bopcheck and checkshape faulties)
    bugs modalg_1 buc60462_2 - modified TODOs according to new behavior

Test cases were modified according to new behavior.
Dear msv, could you please review OCCT and PRODUCTS branches CR26939_2.

Remarks.

tests\boolean\gdml_private\ZI7
tests\boolean\gdml_private\ZJ7

The picture in Windows shows totally incorrect result. So, it is not improvement. Please add checking of sprops of the result in these cases. Take master value as reference. Also, it is incorrect to show input shapes when the result is empty or null. So, add the command "don result".

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

Branch [archived branch] has been updated by Commenter 2.

[revision removed]


Detailed log of new commits:

Author: Commenter 2
Date: Mon Feb 8 15:07:39 2016 +0300

    Corrections for boolean gdml_private ZI7 ZJ7

Dear msv, remarks were applied.
After integration in master checkview command (CR26939_2 was rebased) there are no input shapes in viewer when the result is empty or null. Checkprops was added with reference value from master.
http://occt-tests/gdml_private_ZI7_ZJ7_WNT/summary.html
http://occt-tests/gdml_private_ZI7_ZJ7_LIN/summary.html

Changes in tests look OK; please consider the patch as tested

I wonder why reference area is different for Linux and Windows. I am sure the property of the result must be the same when the test is passed.

Mikhail, as far as I understand, the whole result is different -- this is one of a few test cases which are still unstable.

I see but I think the reference must be the same in the test case, especially we need to insert TODO in any case.

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

Necessary TODOs were added.
http://occt-tests/gdml_private_ZI7_ZJ7_WNT/summary.html
http://occt-tests/gdml_private_ZI7_ZJ7_LIN/summary.html

Dear szv,
Please, validate modifications of following test cases in PRODUCTS branch CR26939_2:
bfit pnt100 A4
bfit pnt1000 A4, A7
bfit pnt10000 A4

Please, explain the big difference in test "bfit pnt1000 A4":

set RES_MAX_DIST 80.3708386
set RES_EXACT_MAX_DIST 80.3708386

The reference values:

set RES_MAX_DIST 5.4432757
set RES_EXACT_MAX_DIST 5.4325009

Sergey, these tests are very unstable; they have different results on Linux and Windows, which change after any change in used OCCT classes, see e.g. 0026368:0043770, #0026252. I propose this problem should be addressed separately.

A new issue 27153 has been created to report unstable test cases of the Best Fit component.

The regressions in the Best Fit test cases

bfit pnt100 A4
bfit pnt1000 A4, A7
bfit pnt10000 A4

caused by this fix are accepted, and the fix can be treated further.

Please consider as tested

Branch [archived branch] has been deleted by Author.

[revision removed]

Branch [archived branch] has been deleted by Author.

[revision removed]

Branch [archived branch] has been deleted by Author.

[revision removed]