octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: GSoC 16: Sample implementation for ispolycw function


From: amr mohamed
Subject: RE: GSoC 16: Sample implementation for ispolycw function
Date: Wed, 23 Mar 2016 22:50:53 +0200

Subject: Re: GSoC 16: Sample implementation for ispolycw function
From: address@hidden
Date: Wed, 23 Mar 2016 10:56:25 -0700
CC: address@hidden; address@hidden; address@hidden
To: address@hidden


On Mar 23, 2016, at 10:28 AM, amr mohamed <address@hidden> wrote:

Dear all,
I have made a sample cc file for the ispolycw function using Boost/Geometry library.
I have tested it and it is working well.
Could you please review it so that i can finalize it? 
https://ideone.com/w6WFzB

Regards, 
Amr Mohamed

This looks like a good start of getting at least one function implemented using the Octave C++ API and including Boost::Geometry. 

Just two quick comments:

1) You might also want to work on making this handle self-intersections in the same way that the Matlab documentation indicates they handle self-intersections

2) To show that testing works (and correctly), you can include tests written in the Octave language as comments directly in the C++ code.
See http://hg.octave.org/octave/file/tip/libinterp/dldfcn/chol.cc where there is the following example that checks whether the inverse from the Cholesky decomposition gives the same result as the standard matrix inverse computation.
/*
%!shared A, Ainv
%! A = [2,0.2;0.2,1];
%! Ainv = inv (A);
%!test
%! Ainv1 = cholinv (A);
%! assert (norm (Ainv-Ainv1), 0, 1e-10);
%!testif HAVE_CHOLMOD
%! Ainv2 = inv (sparse (A));
%! assert (norm (Ainv-Ainv2), 0, 1e-10);
%!testif HAVE_CHOLMOD
%! Ainv3 = cholinv (sparse (A));
%! assert (norm (Ainv-Ainv3), 0, 1e-10);
*/

If you could include tests like this for at least one clockwise and one counterclockwise polygon, that would help show it works and allow there to be regression testing going forward in the case that something changes in Boost::Geometry or a bugfix causes a different error.

Let me know if you have any questions.

John S.


Dear Mr John,

I have included some test cases to the cc file as mentioned https://ideone.com/oAiVFP .
I haven't handled the self-intersecting polygons case yet.

I have two questions:
1)Should i recommend a library for each function in the proposal? and if so , how can i choose between two libraries if both can be used to implement the function?

2)Should i mention that i have made a sample for the ispolycw function ? and will it be better if a create a bitbucket repository instead of sharing my code through links?

Regards, 
Amr Mohamed

reply via email to

[Prev in Thread] Current Thread [Next in Thread]