Wrong result of 2D-extrema between two ellipsis

Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: aml
Date: Mon Dec 22 14:48:58 2014 +0300

    0025635: Wrong result of 2D-extrema between two ellipsis
    
    Fixed Lipschitz constant evaluation in case co-parametrized objects.
    Fixed 2dextrema output.
    Testcase update to new behaviour.

Dear abv,
Please check current state of branch CR25635.

Minor remarks on performance and robustness in math_GlobOptMin::computeInitialValues():

- there seem to be no protection against situation if myA == myB, in that case aStep would become 0 and it still serves as divisor

- aPrevVal is not reset before second cycle, should not it be?

- second cycle is not symmetric w.r.t. the order of variables, should not it be?

- division by aStep seems to be not really necessary in cycles, this may be done once at the end

- Value (myB - myA) / (aPntNb - 1) could be calculated once and then reused in each cycle. It seems that '- 1' is not needed here, and that value should be consistent with how aStep is calculated.

- arguments calculated in condition of "if"s in "for" cycles at lines 317 and 332 are used in their bodies; it is better to calculate the value once.
For both performance and clarity it might be better to write something like

aLipConst = Max (aLipConst, Abs(aCurrVal - aPrevVal))

instead of "if" (operator "?" works faster than "if" from my experience)

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear abv,

"second cycle is not symmetric w.r.t. the order of variables, should not it be?"
Yes, cycle is not symmetric and it is correct.

Another remarks are fixed.

Please check updated branch CR25635.

Alexander, please explain the formula used to calculate aCurrPnt:

  for(i = 1; i <= aPntNb; i++)
  {
    aCurrPnt = myA + (myB - myA) * i / (aPntNb - 1);
    ...
  }

At the last iteration i = aPntNb and aCurrPnt is NOT equal to myB, while it should be, if my understanding is correct. See relevant point in my previous comment.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear abv,
I forgot to add this part of fix. As we discussed "(myB - myA) / aPntNb" computation moved out of cycle.

Please check branch CR25635.

Reviewed, please test.

P.S. Though, "aStep = aParamStep.Norm();" :)

Dear Commenter 1,
Branch CR25635 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: 18 (18 on master)
Windows: 0 (0 on master)

products component :
Linux: 11 (11 on master)
Windows: 1 (1 on master)

Regressions/Differences:
No regressions/differences

Testing cases:
http://occt-tests.nnov.opencascade.com/CR25635-master-occt/Windows-32-VC10/bugs/fclasses/bug25635_1.html
bugs fclasses(002) bug25635_1: OK
http://occt-tests.nnov.opencascade.com/CR25635-master-occt/Windows-32-VC10/bugs/fclasses/bug25635_2.html
bugs fclasses(002) bug25635_2: OK

Testing on Linux:
occt component :
Total MEMORY difference: 367337528 / 367924740
Total CPU difference: 50018.94999999995 / 51486.81000000016
products component :
Total MEMORY difference: 113173680 / 113199648
Total CPU difference: 16686.53999999996 / 17027.049999999977

Testing on Windows:
occt component :
Total MEMORY difference: 277698124 / 277270364
Total CPU difference: 38541.03125 / 37471.9842007
products component :
Total MEMORY difference: 72394772 / 71382268
Total CPU difference: 10750.578125 / 10342.656534399997

There are no differences in images found by testdiff.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkv
Date: Tue Dec 30 17:43:46 2014 +0300

    Test cases for issue CR25635

Dear aml,
could you please review test case bugs/fclasses(002)/bug25635_1 and bugs/fclasses(002)/bug25635_2

Dear mkv,

bug25635_1
Please follow nbv comments.

bug25635_2
Second test should check output order:

master:
"dist 1: 4
ext_1 dist 2: 0
dist 3: 8
ext_3 dist 4: 4
ext_4"

after fix:

"dist 1: 4 ext_1
 dist 2: 0 ext_2
 dist 3: 8 ext_3
 dist 4: 4 ext_4"

ext_2 added to output and "dist i / ext_i" now on one line.

1) Check existence of each ext_i in "2dextrema" command output.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkv
Date: Wed Feb 4 17:08:26 2015 +0300

    Correction of test cases for issue CR25635

Dear aml,
could you please review test case bugs/fclasses(002)/bug25635_1 and bugs/fclasses(002)/bug25635_2

http://occt-tests/CR25635-master-occt/Windows-32-VC10/bugs/fclasses/bug25635_1.html
http://occt-tests/CR25635-master-occt/Windows-32-VC10/bugs/fclasses/bug25635_2.html

Dear mkv,

I
First test case is OK.

II
In the second test case distance value strictly associated with extrema. It's not good since output order may change from time to time. For example if swap first and second extrema in output, testcase will fail:

"dist 1: 0 ext_1
 dist 2: 4 ext_2
 dist 3: 8 ext_3
 dist 4: 4 ext_4"

It is better to make regular expressions which check existence of each ext_i. For example:

if { [regexp {ext_1}} ${info}] } {
  puts "OK: Good ext_1"
} else {
  puts "Error: Wrong ext_1"
}

This will be enough.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkv
Date: Thu Feb 5 14:28:01 2015 +0300

    Correction of test case for issueCR25635

Branch CR25635 is TESTED.

Branch [archived branch] has been deleted by Participant.

[revision removed]