MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0025086Open CASCADE[OCCT] OCCT:Modeling Algorithmspublic2014-07-16 09:392014-11-11 12:58
Reporterifv 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 6.8.0Fixed in Version[OCCT] 6.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
Attached Files

- Relationships
related to 0024979closedbugmaster Optimize Extrema_GenExtCS 

-  Notes
(0030389)
git (administrator)
2014-07-23 16:01

Branch CR25086 has been created by aml.

SHA-1: b21fa6a74a3fa007eb1cdc94fa7af0267f8aeb41
(0030390)
aml (developer)
2014-07-23 16:07

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.
(0030428)
ifv (developer)
2014-07-25 17:22

I think, it is possible to find solution without performance lost.
(0030472)
git (administrator)
2014-07-29 16:30

Branch CR25086 has been updated forcibly by aml.

SHA-1: f14ad8d4fd01050c38bda9df8ec8313eb724caa0
(0030473)
aml (developer)
2014-07-29 16:34

Dear jgv,
Please check current state of branch CR25086.

Performance problems fixed.
(0030491)
git (administrator)
2014-07-30 14:05

Branch CR25086 has been updated forcibly by aml.

SHA-1: 3fa0ef6cfaf9829dbc239713912b0c94db0836d5
(0030506)
jgv (developer)
2014-07-31 13:23

Ok.
(0030532)
git (administrator)
2014-08-01 16:58

Branch CR25086 has been updated forcibly by apv.

SHA-1: 6cf064222d57a8281e21e3120b5d8f4c9fbb23ba
(0030535)
git (administrator)
2014-08-01 17:18

Branch CR25086 has been updated forcibly by apv.

SHA-1: e3f658ed7957a5b664065ac3f083cd949784799b
(0030566)
apv (tester)
2014-08-05 10:51
edited on: 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

(0030574)
git (administrator)
2014-08-05 12:46

Branch CR25086 has been updated forcibly by aml.

SHA-1: b67d6bf4174f8f0a2ccc1c438c0d38f2d0fc98ba
(0030575)
aml (developer)
2014-08-05 12:54

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).
(0030584)
abv (manager)
2014-08-05 18:50
edited on: 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...

(0030594)
git (administrator)
2014-08-06 09:20

Branch CR25086 has been updated forcibly by aml.

SHA-1: 226ecc79104c49628cd3e11f260dfbfcb1af4b4a
(0030595)
aml (developer)
2014-08-06 09:22

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.
(0030600)
git (administrator)
2014-08-06 12:04

Branch CR25086 has been updated forcibly by aml.

SHA-1: f64df593da9dfe2c36578839f3a3ea299bffd3cf
(0030629)
git (administrator)
2014-08-08 10:42

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.
(0030700)
abv (manager)
2014-08-13 09:51

No remarks, please test
(0030722)
git (administrator)
2014-08-13 15:48

Branch CR25086_1 has been updated forcibly by apv.

SHA-1: 4737f92b4a793138d15b05175105034e2d603ca5
(0030725)
apv (tester)
2014-08-13 17:13
edited on: 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 [^]

(0030734)
git (administrator)
2014-08-14 07:49

Branch CR25086_1 has been updated forcibly by aml.

SHA-1: f92ba4374719655168d45d5d47a8970351c1ec3c
(0030738)
aml (developer)
2014-08-14 10:27

Fixed, please re-test.
(0030782)
git (administrator)
2014-08-15 14:03

Branch CR25086_1 has been updated forcibly by apv.

SHA-1: d7de6a9870bd6019de639aa9744579354417f115
(0030833)
apv (tester)
2014-08-18 13:16

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
(0030871)
git (administrator)
2014-08-19 11:25

Branch CR25086_1 has been updated forcibly by aml.

SHA-1: 719229aeb3cfd3345133589336f3f406cbd75b1d
(0030872)
aml (developer)
2014-08-19 11:30

Fixed. Changed order of class members to avoid gcc warnings.
(0030933)
apv (tester)
2014-08-20 14:00

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
(0031464)
git (administrator)
2014-09-08 15:51

Branch CR25086 has been deleted by inv.

SHA-1: f64df593da9dfe2c36578839f3a3ea299bffd3cf
(0031465)
git (administrator)
2014-09-08 15:51

Branch CR25086_1 has been deleted by inv.

SHA-1: 719229aeb3cfd3345133589336f3f406cbd75b1d

- Related Changesets
occt: master 1d33d22b
Timestamp: 2014-07-21 08:52:15
Author: aml
Committer: bugmaster
Details ] Diff ]
0025086: Implementation PSO in package math

Porting Particle Swarm Optimization (PSO) to math package.
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 View Revisions
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 View Revisions
2014-08-05 18:52 abv Note Edited: 0030584 View Revisions
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 View Revisions
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


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker