View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0025086Open CASCADEOCCT:Modeling Algorithmspublic2014-07-16 09:392014-11-11 12:58
ReporterifvAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version6.8.0Fixed in Version6.8.0 
Summary0025086: Implementation PSO in package math
DescriptionPSO method is succesfully used in Extrema CS algorithm, but it is implementented directly in source of Extrema. In order to use PSO in other algoritms (ex. Extrema SS) it is necessary to implement it in package math as separate optimisation method.
Steps To ReproduceNo
Additional information
and documentation updates		See description for 0024979
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0024979 closedbugmaster Optimize Extrema_GenExtCS 

Activities

git

2014-07-23 16:01

administrator   ~0030389

Branch CR25086 has been created by aml.

SHA-1: b21fa6a74a3fa007eb1cdc94fa7af0267f8aeb41

aml

2014-07-23 16:07

developer   ~0030390

Dear ifv,
please check current state of branch CR25086.

Performance loss obtained(may be 2 times) due to usage of NCollection_Vector instead of fixed dimensions vector.

ifv

2014-07-25 17:22

developer   ~0030428

I think, it is possible to find solution without performance lost.

git

2014-07-29 16:30

administrator   ~0030472

Branch CR25086 has been updated forcibly by aml.

SHA-1: f14ad8d4fd01050c38bda9df8ec8313eb724caa0

aml

2014-07-29 16:34

developer   ~0030473

Dear jgv,
Please check current state of branch CR25086.

Performance problems fixed.

git

2014-07-30 14:05

administrator   ~0030491

Branch CR25086 has been updated forcibly by aml.

SHA-1: 3fa0ef6cfaf9829dbc239713912b0c94db0836d5

jgv

2014-07-31 13:23

developer   ~0030506

Ok.

git

2014-08-01 16:58

administrator   ~0030532

Branch CR25086 has been updated forcibly by apv.

SHA-1: 6cf064222d57a8281e21e3120b5d8f4c9fbb23ba

git

2014-08-01 17:18

administrator   ~0030535

Branch CR25086 has been updated forcibly by apv.

SHA-1: e3f658ed7957a5b664065ac3f083cd949784799b

apv

2014-08-05 10:51

tester   ~0030566

Last edited: 2014-08-05 10:53

 Dear BugMaster,

Branch CR25086 (and products from GIT master) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: e3f658ed7957a5b664065ac3f083cd949784799b

Number of compiler warnings:
occt component:
   Linux: 15 (15 on master)
   Windows: 0 (0 on master)
   MacOS: 196 (196 on master)
products component :
   Linux: 11 (11 on master)
   Windows: 1 (1 on master)

Regressions/Differences:
http://occt-tests/CR25086-master-occt/Windows-32-VC10/summary.html
bugs moddata_2 bug277

Testing cases:
Absent

Testing on Linux:
Total MEMORY difference: 352779188 / 351924220
Total CPU difference: 45075.200000000106 / 46365.03000000032

Testing on Windows:
Total MEMORY difference: 239759000 / 239974256
Total CPU difference: 27693.359375 / 30887.390625

There are differences in images found by testdiff:
http://occt-tests/CR25086-master-occt/Debian60-64/diff-Debian60-64.html
http://occt-tests/CR25086-master-occt/Windows-32-VC10/diff-Windows-32-VC10.html
Pay attention: bugs moddata_3 bug23830

git

2014-08-05 12:46

administrator   ~0030574

Branch CR25086 has been updated forcibly by aml.

SHA-1: b67d6bf4174f8f0a2ccc1c438c0d38f2d0fc98ba

aml

2014-08-05 12:54

developer   ~0030575

Dear abv,
please check current state of branch CR25086.

bugs moddata_2 bug277 - fixed.

bugs moddata_3 bug23830 - is ok, due to slightly changing extrema points (distance still ok).

abv

2014-08-05 18:50

manager   ~0030584

Last edited: 2014-08-05 18:52

 Some remarks:

1. Extrema_GenExtCS.cxx, L249:

+ Standard_Address CopyC = (Standard_Address)&C;
+ Adaptor3d_Curve& aC = *(Adaptor3d_Curve*)CopyC;

Why this cast is necessary? If really unavoidable, use const_cast<> instead, and add explanatory comment.

2. Please try to minimize irrelevant changes in formatting of existing code.

This both causes problems in code review, and can make the code worse for the people who have different feeling of good coding rules.
For instance, removing {} around one-line body of "for" cycle is rather controversial change: these braces are still useful due to clearly limiting the body scope.

3. In Extrema_GenExtCS, variable names like

+ math_Vector aMaxUV(1,3);

suggest that such variable contains U and V parameters (on surface), while actually it contains also parameter on curve. I suggest these should be named like

+ math_Vector aMaxTUV(1,3);

where T refers to parameter on curve.

4. Please avoid unnecessary dynamic allocation of memory and use of plain C pointers:

+ math_MultipleVarFunction *aFunc = new Extrema_GlobOptFuncCSC2(aC, *myS);
+ math_PSO aPSO(*aFunc, UVinf, UVsup, aStep);
+ aPSO.Perform(aParticles, aNbParticles, aValue, UV);
+
     math_FunctionSetRoot anA (myF, UV, Tol, UVinf, UVsup, 100, Standard_False);
+ delete aFunc;

why not makeing aFunc a local var?

5. Class Extrema_GlobOptFuncCS:
- is it really a new class? have you checked that the same tool did not existed?
- if it is used only in Extrema_GenExtCS, it can be defined as local class in Extrema_GenExtCS.cxx (within anonymous namespace)
- in the comment to class(es), do words "which operates with Euclidean distance between curve and surface" mean "calculating square Euclidean distance between point on curve and point on surface"? Note that "br" is not needed, this is artifact of CDL extractor
- these classes remember addresses of constructor arguments in their fields, and it would be safer if constructor accepted pointer rather than reference: this would hint the programmer that this pointer may be still in use after construction. And, please comment this! (The same remark for math_PSO::myFunc)
- why defining three classes rather than single one (with Hessian)?
- perhaps we can define some adapter to convert multiple-valued function to its scalar product, this would allow using existing Extrema_FuncExtCS function instead of defining a new one

6. Class math_PSOParticlesPool:
- have you tried using NCollection_Array instead of C array manipulated by pointers? please consider!
- note that you can use STL-style iteration by arrays, this can be faster than addressing by index.
- instead of method GetParticleData() returning pointers to internal arrays, please consider providing separate access methods to relevant fields

----

A side note: In Extrema_GenExtCS, now you use math_Vector where previously NCollection_Vec3 was used. This was clearly necessary to generalize the code, but may have some performance penalty. We may think of how to improve this...

git

2014-08-06 09:20

administrator   ~0030594

Branch CR25086 has been updated forcibly by aml.

SHA-1: 226ecc79104c49628cd3e11f260dfbfcb1af4b4a

aml

2014-08-06 09:22

developer   ~0030595

1. Extrema_GenExtCS.cxx, L249:
Fixed.

2. Please try to minimize irrelevant changes in formatting of existing code.
Ok.

3. In Extrema_GenExtCS, variable names like...
Fixed.

4. Please avoid unnecessary dynamic allocation of memory and use of plain C pointers:
Ok. Fixed.

5. Class Extrema_GlobOptFuncCS:
- is it really a new class? have you checked that the same tool did not existed?
  In mathematics there are usually more than one way to describe something. Old class FuncExtCS represents function set to solve minimization problem, new class represents one function for solving the same problem.
  We can use FuncExtCS to our purposes, but it will significantly increase arithmetic operations number, with factor 2-5.
  
- if it is used only in Extrema_GenExtCS, it can be defined as local class in Extrema_GenExtCS.cxx (within anonymous namespace)
  What reason to use anonymous namespace?
- in the comment to class(es), do words "which operates with Euclidean distance between curve and surface" mean "calculating square Euclidean distance between point on curve and point on surface"? Note that "br" is not needed, this is artifact of CDL extractor
  Comments fixed. "Br" is doxygen command which forces a line break.
  
- these classes remember addresses of constructor arguments in their fields, and it would be safer if constructor accepted pointer rather than reference: this would hint the programmer that this pointer may be still in use after construction. And, please comment this! (The same remark for math_PSO::myFunc)
  fixed.
  
- why defining three classes rather than single one (with Hessian)?
  fixed.

- perhaps we can define some adapter to convert multiple-valued function to its scalar product, this would allow using existing Extrema_FuncExtCS function instead of defining a new one
  Answered earlier, this seems to be possible, but ineffective.

6. Class math_PSOParticlesPool:
- have you tried using NCollection_Array instead of C array manipulated by pointers? please consider!
  Didn't tried yet.
 
- note that you can use STL-style iteration by arrays, this can be faster than addressing by index.
  Both algorithms(my and std::max_element) have linear complexity, not faster in this case.
  
- instead of method GetParticleData() returning pointers to internal arrays, please consider providing separate access methods to relevant fields
  fixed.

git

2014-08-06 12:04

administrator   ~0030600

Branch CR25086 has been updated forcibly by aml.

SHA-1: f64df593da9dfe2c36578839f3a3ea299bffd3cf

git

2014-08-08 10:42

administrator   ~0030629

Branch CR25086_1 has been created by aml.

SHA-1: ce0cb51a30a2fc2d3495be7cfb9130a4f21bb8e4


This branch includes the following new commits:

       new ce0cb51 0025086: Implementation PSO in package math


Detailed log of new commits:

commit ce0cb51a30a2fc2d3495be7cfb9130a4f21bb8e4
Author: aml
Date: Mon Jul 21 12:52:15 2014 +0400

    0025086: Implementation PSO in package math
    
    Porting Particle Swarm Optimization (PSO) to math package.

abv

2014-08-13 09:51

manager   ~0030700

No remarks, please test

git

2014-08-13 15:48

administrator   ~0030722

Branch CR25086_1 has been updated forcibly by apv.

SHA-1: 4737f92b4a793138d15b05175105034e2d603ca5

apv

2014-08-13 17:13

tester   ~0030725

Last edited: 2014-08-13 18:39

 Dear BugMaster,

Branch CR25086_1 was compiled on Linux platform.
There are following compilation error:
http://jenkins-test-02.nnov.opencascade.com/user/mnt/my-views/view/CR25086_1/job/mnt-CR25086_1-master_build_occt_linux/1/parsed_console/

1. /usr/include/c++/4.4/bits/stl_algo.h:5998: error: no match for ‘operator<’ in ‘__first.NCollection_StlIterator<Category, BaseIterator, ItemType, IsConstant>::operator* [with Category = std::random_access_iterator_tag, BaseIterator = NCollection_Array1<PSO_Particle>::Iterator, ItemType = PSO_Particle, bool IsConstant = false]() < __result.NCollection_StlIterator<Category, BaseIterator, ItemType, IsConstant>::operator* [with Category = std::random_access_iterator_tag, BaseIterator = NCollection_Array1<PSO_Particle>::Iterator, ItemType = PSO_Particle, bool IsConstant = false]()’

2. /usr/include/c++/4.4/bits/stl_algo.h:6054: error: no match for ‘operator<’ in ‘__result.NCollection_StlIterator<Category, BaseIterator, ItemType, IsConstant>::operator* [with Category = std::random_access_iterator_tag, BaseIterator = NCollection_Array1<PSO_Particle>::Iterator, ItemType = PSO_Particle, bool IsConstant = false]() < __first.NCollection_StlIterator<Category, BaseIterator, ItemType, IsConstant>::operator* [with Category = std::random_access_iterator_tag, BaseIterator = NCollection_Array1<PSO_Particle>::Iterator, ItemType = PSO_Particle, bool IsConstant = false]()’

The same problems detected on Windows:
http://jenkins-test-02.nnov.opencascade.com/user/mnt/my-views/view/CR25086_1/job/mnt-CR25086_1-master_build_occt_windows/2/consoleFull

git

2014-08-14 07:49

administrator   ~0030734

Branch CR25086_1 has been updated forcibly by aml.

SHA-1: f92ba4374719655168d45d5d47a8970351c1ec3c

aml

2014-08-14 10:27

developer   ~0030738

Fixed, please re-test.

git

2014-08-15 14:03

administrator   ~0030782

Branch CR25086_1 has been updated forcibly by apv.

SHA-1: d7de6a9870bd6019de639aa9744579354417f115

apv

2014-08-18 13:16

tester   ~0030833

Dear BugMaster,

Branch CR25086_1 (and products from GIT master) was compiled on Linux and Windows platforms and tested.
SHA-1: d7de6a9870bd6019de639aa9744579354417f115

Number of compiler warnings:
occt component:
   Linux: 18 (15 on master)
   Windows: 0 (0 on master)
products component:
   Linux: 11 (11 on master)
   Windows: 1 (1 on master)

There are new additional compilation warnings:
on Linux platform:
math_PSOParticlesPool.hxx:67, GNU C Compiler 4 (gcc), Priority: Normal
   ‘NCollection_Array1<double> math_PSOParticlesPool::myMemory’
math_PSOParticlesPool.hxx:68, GNU C Compiler 4 (gcc), Priority: Normal
   ‘math_PSOParticlesPool::myParticlesPool’ will be initialized after
math_PSOParticlesPool.cxx:24, GNU C Compiler 4 (gcc), Priority: Normal
   when initialized here

Regressions/Differences:
Not detected

Testing cases:
Absent

Testing on Linux:
Total MEMORY difference: 352512548 / 351903180
Total CPU difference: 45743.72000000025 / 45953.950000000135

Testing on Windows:
Total MEMORY difference: 239348920 / 239452172
Total CPU difference: 41600.875 / 32460.53125

git

2014-08-19 11:25

administrator   ~0030871

Branch CR25086_1 has been updated forcibly by aml.

SHA-1: 719229aeb3cfd3345133589336f3f406cbd75b1d

aml

2014-08-19 11:30

developer   ~0030872

Fixed. Changed order of class members to avoid gcc warnings.

apv

2014-08-20 14:00

tester   ~0030933

Dear BugMaster,

Branch CR25086_1 (and products from GIT master) was compiled on Linux and Windows platforms and tested.
SHA-1: 719229aeb3cfd3345133589336f3f406cbd75b1d

Number of compiler warnings:
occt component:
   Linux: 15 (15 on master)
   Windows: 0 (0 on master)
products component:
   Linux: 11 (11 on master)
   Windows: 1 (1 on master)

Regressions/Differences:
Not detected

Testing case:
Not needed

Testing on Linux:
Total MEMORY difference: 352277024 / 351903180
Total CPU difference: 44277.99000000018 / 45953.950000000135

Testing on Windows:
Total MEMORY difference: 239925600 / 239619136
Total CPU difference: 32506.9375 / 32608.984375

git

2014-09-08 15:51

administrator   ~0031464

Branch CR25086 has been deleted by inv.

SHA-1: f64df593da9dfe2c36578839f3a3ea299bffd3cf

git

2014-09-08 15:51

administrator   ~0031465

Branch CR25086_1 has been deleted by inv.

SHA-1: 719229aeb3cfd3345133589336f3f406cbd75b1d

Related Changesets

occt: master 1d33d22b

2014-07-21 08:52:15

aml


Committer: bugmaster Details Diff		 0025086: Implementation PSO in package math

Porting Particle Swarm Optimization (PSO) to math package.		 Affected Issues
0025086
mod - src/Extrema/Extrema_GenExtCS.cdl Diff File
mod - src/Extrema/Extrema_GenExtCS.cxx Diff File
add - src/Extrema/Extrema_GlobOptFuncCS.cxx Diff File
add - src/Extrema/Extrema_GlobOptFuncCS.hxx Diff File
mod - src/Extrema/FILES Diff File
mod - src/math/FILES Diff File
mod - src/math/math.cdl Diff File
add - src/math/math_BullardGenerator.hxx Diff File
add - src/math/math_PSO.cxx Diff File
add - src/math/math_PSO.hxx Diff File
add - src/math/math_PSOParticlesPool.cxx Diff File
add - src/math/math_PSOParticlesPool.hxx Diff File

Issue History

Date Modified Username Field Change
2014-07-16 09:39 ifv New Issue
2014-07-16 09:39 ifv Assigned To => aml
2014-07-16 09:53 abv Relationship added related to 0024979
2014-07-23 16:01 git Note Added: 0030389
2014-07-23 16:07 aml Note Added: 0030390
2014-07-23 16:07 aml Assigned To aml => ifv
2014-07-23 16:07 aml Status new => resolved
2014-07-25 17:22 ifv Note Added: 0030428
2014-07-25 17:22 ifv Assigned To ifv => aml
2014-07-25 17:22 ifv Status resolved => assigned
2014-07-29 16:30 git Note Added: 0030472
2014-07-29 16:34 aml Note Added: 0030473
2014-07-29 16:34 aml Assigned To aml => jgv
2014-07-29 16:34 aml Status assigned => resolved
2014-07-30 14:05 git Note Added: 0030491
2014-07-31 13:23 jgv Note Added: 0030506
2014-07-31 13:23 jgv Status resolved => reviewed
2014-08-01 16:58 git Note Added: 0030532
2014-08-01 17:18 git Note Added: 0030535
2014-08-05 10:51 apv Note Added: 0030566
2014-08-05 10:53 apv Note Edited: 0030566
2014-08-05 10:54 apv Assigned To jgv => aml
2014-08-05 10:54 apv Status reviewed => assigned
2014-08-05 12:46 git Note Added: 0030574
2014-08-05 12:54 aml Note Added: 0030575
2014-08-05 12:54 aml Assigned To aml => abv
2014-08-05 12:54 aml Status assigned => resolved
2014-08-05 18:50 abv Note Added: 0030584
2014-08-05 18:50 abv Assigned To abv => aml
2014-08-05 18:50 abv Status resolved => assigned
2014-08-05 18:51 abv Note Edited: 0030584
2014-08-05 18:52 abv Note Edited: 0030584
2014-08-06 09:20 git Note Added: 0030594
2014-08-06 09:22 aml Note Added: 0030595
2014-08-06 09:22 aml Assigned To aml => abv
2014-08-06 09:22 aml Status assigned => resolved
2014-08-06 12:04 git Note Added: 0030600
2014-08-08 10:42 git Note Added: 0030629
2014-08-13 09:51 abv Note Added: 0030700
2014-08-13 09:51 abv Assigned To abv => bugmaster
2014-08-13 09:51 abv Status resolved => reviewed
2014-08-13 10:16 bugmaster Assigned To bugmaster => apv
2014-08-13 15:48 git Note Added: 0030722
2014-08-13 17:13 apv Note Added: 0030725
2014-08-13 17:14 apv Assigned To apv => aml
2014-08-13 17:14 apv Status reviewed => assigned
2014-08-13 18:39 apv Note Edited: 0030725
2014-08-14 07:49 git Note Added: 0030734
2014-08-14 10:27 aml Status assigned => resolved
2014-08-14 10:27 aml Note Added: 0030738
2014-08-14 10:27 aml Status resolved => reviewed
2014-08-14 10:29 aml Assigned To aml => bugmaster
2014-08-15 12:07 mkv Assigned To bugmaster => apv
2014-08-15 14:03 git Note Added: 0030782
2014-08-18 13:16 apv Note Added: 0030833
2014-08-18 13:17 apv Assigned To apv => aml
2014-08-18 13:17 apv Status reviewed => assigned
2014-08-19 11:25 git Note Added: 0030871
2014-08-19 11:30 aml Note Added: 0030872
2014-08-19 11:30 aml Assigned To aml => abv
2014-08-19 11:30 aml Status assigned => resolved
2014-08-19 11:45 abv Status resolved => reviewed
2014-08-19 12:16 apv Assigned To abv => apv
2014-08-20 12:33 apv Test case number => Not needed
2014-08-20 14:00 apv Note Added: 0030933
2014-08-20 14:01 apv Assigned To apv => bugmaster
2014-08-20 14:01 apv Status reviewed => tested
2014-08-22 15:42 bugmaster Changeset attached => occt master 1d33d22b
2014-08-22 15:42 bugmaster Status tested => verified
2014-08-22 15:42 bugmaster Resolution open => fixed
2014-09-08 15:51 git Note Added: 0031464
2014-09-08 15:51 git Note Added: 0031465
2014-11-11 12:45 aiv Fixed in Version => 6.8.0
2014-11-11 12:58 aiv Status verified => closed