xjiang4 / ellipsoids

Automatically exported from code.google.com/p/ellipsoids
Other
0 stars 1 forks source link

Replace fCalcBodyTri nested function in minksum and other mink* functions with getRhoBoundary #116

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
We need to use fCalcBodyTri to create the following public methods of ellipsoid 
class (along with tests):
[bpMat, fMat, supVec] = getRhoBoundary(ellObj,nPoints)
[bpMat, fMat, supVec] = getRhoBoundaryByFactor(ellObj,nPoints)

They should be analogous to already existing methods

[bpMat, fMat] = getBoundary(ellObj,nPoints)
[bpMat, fMat] = getBoundaryByFactor(ellObj,nPoints)

Once this is done we need to replace fCalcBodyTri with getRhoBoundary in mink 
functions

Original issue reported on code.google.com by heartofm...@gmail.com on 29 Apr 2013 at 3:16

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 18 Sep 2013 at 12:13

GoogleCodeExporter commented 8 years ago

Original comment by SeregaDrozh on 28 Sep 2013 at 10:00

GoogleCodeExporter commented 8 years ago

Original comment by SeregaDrozh on 2 Oct 2013 at 10:31

GoogleCodeExporter commented 8 years ago
1) In any Matlab function/method all outputs are optional by default so please 
do not use "optional" marker for outputs in the methods help headers.

2) Instead of checking for a number of outputs like this:

if nargout==1
...
elseif nargout==2

etc

do this:

function varargout=outerMethod(self,...)

if nargout==0
varargout=cell(1,0);
else
[varargout{:}]=self.innerMethod(...);
end

i.e. you need to handle only 2 different cases (nargout==0 and nargout>0)

3) Why do you need to do vGridMat(vGridMat==0)=eps? This doesn't make sense to 
me...
4) Please replace fCalcBodyTri with getBoundaryByFactor in ellipsoid.plot
5) See the description of the rest of the problems in the comments for your 
last commit.

Original comment by heartofm...@gmail.com on 3 Oct 2013 at 6:51

GoogleCodeExporter commented 8 years ago

Original comment by SeregaDrozh on 3 Oct 2013 at 6:12

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 5 Oct 2013 at 3:51

GoogleCodeExporter commented 8 years ago

Original comment by SeregaDrozh on 6 Oct 2013 at 3:22

GoogleCodeExporter commented 8 years ago
So was it a bug in the help headers or incorrect implementation of the methods?

Also, can you please just write [nFaces,nDims] and put all the details like  
nFaces = nPoints/(vNum+eNum+1)]+1)*(vNum+eNum) + 1 into the description of the 
parameter. And to be honest, you do not need to be that specific - a user just 
need to know how the factor or the number of points influences the number of 
vertices, as for the number of faces - it is not important for a user as it 
purely depends on the triangulation algorithm.

You still write
 bpGridMat: double[nVertices,nDims]/
    21  +   % double[nDims, ([nPoints/(vNum+eNum+1)]+1)*(vNum+eNum) + 1]

and that confuses me - the format of the output should ALWAYS be 
[nVerties,nDims] but double[nDims, ([nPoints/(vNum+eNum+1)]+1)*(vNum+eNum) + 1] 
contradicts this requirement.

Original comment by heartofm...@gmail.com on 6 Oct 2013 at 4:38

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 6 Oct 2013 at 4:38

GoogleCodeExporter commented 8 years ago
Please run all the tests to make sure they pass

Original comment by heartofm...@gmail.com on 7 Oct 2013 at 8:58

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 12 Oct 2013 at 7:54

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 19 Oct 2013 at 6:50