>

>> On Apr 17, 2016, at 10:42 AM, PhilipNienhuis <

[hidden email]> wrote:

>>

>> John Swensen-3 wrote

>>>> On Mar 23, 2016, at 1:50 PM, amr mohamed <

>>

>>> amr_mohamed@

>>

>>> > wrote:

>>>>

>>>> Subject: Re: GSoC 16: Sample implementation for ispolycw function

>>>> From:

>>

>>> jpswensen@

>>

>>> <mailto:

>>

>>> jpswensen@

>>

>>> >

>>>> Date: Wed, 23 Mar 2016 10:56:25 -0700

>>>> CC:

>>

>>> pr.nienhuis@

>>

>>> <mailto:

>>

>>> pr.nienhuis@

>>

>>> >;

>>

>>> nirkrakauer@

>>

>>> <mailto:

>>

>>> nirkrakauer@

>>

>>> >;

>>

>>> octave-maintainers@

>>

>>> <mailto:

>>

>>> octave-maintainers@

>>

>>> >

>>>> To:

>>

>>> amr_mohamed@

>>

>>> <mailto:

>>

>>> amr_mohamed@

>>

>>> >

>>>>

>>>>

>>>> On Mar 23, 2016, at 10:28 AM, amr mohamed <

>>

>>> amr_mohamed@

>>

>>> <mailto:

>>

>>> amr_mohamed@

>>

>>> >> 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 <

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>>>> <

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 <

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

>>>

>>>

>>> 1) I would think that we would only want to include one library dependency

>>> to implement all the missing polygon functionality. Otherwise, you would

>>> have to spend a lot of effort learning two different libraries and how to

>>> convert between their vertex representations.

>>>

>>> 2) I think the best way to contribute code is to make sure it is complete.

>>> For example don’t include things like “TODO: Check if arguments are valid”

>>> when it is a pretty easy task to go ahead and perform those checks. Once

>>> you think it is ready, I would submit a Bug Report marked as a Feature

>>> Request to the GNU Octave Savannah project and then attach the file to the

>>> bug report.

>>>

>>> Two other comments:

>>> I would definitely mention in your proposal that you are at the point when

>>> you can compile external functions against the Octave libraries and the

>>> Boost libraries and implemented one of the more simple polygon functions.

>>>

>>> Also, you should look more carefully at how the tests were written in the

>>> link I gave you. You should follow the pattern they used to include

>>> “assert” statements that will let the automated test system know whether

>>> the tests succeeded or failed.

>>>

>>> John S.

>>

>> PMFJI (very) late.

>>

>> We seem to overlook functions already present in other OF packages.

>>

>> The geometry package contains polygonArea.m that returns the area of a

>> polygon, or a set of polygons, which is negative for clockwise and positive

>> for counterclockwise polygons.

>> So we could have a very simple function ispolycw.m looking along the lines

>> of:

>>

>> function cw = ispolycw (<Nx2_POINTS>)

>>

>> cw = polygonArea (<Nx2_POINTS>) < 0;

>>

>> endfunction

>>

>> that has no external dependencies whatsoever (apart from maybe an OF

>> package).

>>

>> Now, CPU performance may be a point. But for the time being, a function

>> ispolycw.m w/o dependencies (the latter an aim of me) is in close reach.

>> And I think it would fit nicely and naturally in the geometry package. (Yeah

>> I'm stirring up again, sorry for that)

>>

>> Philip

>>

>>

>>

>>

>> --

>> View this message in context:

http://octave.1599824.n4.nabble.com/GSoC-16-Sample-implementation-for-ispolycw-function-tp4675763p4676344.html>> Sent from the Octave - Maintainers mailing list archive at Nabble.com.

>>

>

> I think that doing a quick performance check on the existing polygon functionality in Octave compared to the algorithms in Boost::Geometry and Boost::Polygon would be of merit. I think that one of the websites that did a comparison test between some other libraries gave the sets of points they used for their benchmarks. I know from interacting a bit on the mailing list that Boost::Geometry is very actively maintained by funded contributors from companies that do mapping and GIS. This alone makes it a somewhat attracted alternative. You are right, however, that it adds another dependency to the package(s) of geometry and mapping.

>

> John S.

>

>

>

>

Geometry is a fork of matgeom, a m-file only geometry library. I am