Hi, Chris and David.
>> Ok, although it is usually better to write some perl
>> code to call pp_def() to generated both routines
>> from the same code---avoids the case where one
>> routine is modified but not the other.
Agreed. Updated (see below).
>> The proposed route is usually good to allow for things to
>> be fully tested and incrementally developed. In the case
>> where one has a "developer on fire" (TM) then a lot of the
>> stages can be skipped. :-)
Aah, I see now. I tried to code it so that it would be as immediately
beneficial as possible while not breaking the current codebase at the
expense of increasing the Image2D::poly* namespace. I definitely understand
slow integration steps to avoid breaking things on a project this big with
such a diverse user base. :)
>> Since the PP and non-PP implementations do the
>> same thing---just better, the pnpoly routine should
>> be migrated to use pnpoly_pp under the hood.
That will be a bit of a complication. Currently, the PDL::pnpoly interface
is (in PP jargon) 'x(m,n); y(m,n); px(k); py(k); int [o] msk(m,n)' and the
interface for pnpoly_pp is 'a(m,n); ps(k,l); int [o] msk(m,n)'. Certainly,
the inputs to PDL::pnpoly could be manipulated and sent off to pnpoly_pp,
but the expensive xvals/yvals calls still exist in userland. This is why I
was thinking that PDL::pnpoly could be put on the path to deprecation (not
for a long while, though!) and pnpoly_pp could be exported now to allow
people to move over to the PP version. However, I shall leave that
executive decision in your hands.
>> Once we have some more user experience
>> with the new routine, we could prepare to make polyfill and
>> polyfillv call the pnpoly routines under the hood with a flag
>> to control the option.
Agreed. Having two methods of solving the points-in-a-plane problem isn't
terribly beneficial in the long run.
>> Our PDL standard test module is Test::More but not all
>> existing tests have been migrated to use that. All new
>> tests should start with that.
The Image2D suite is still using just Test, so I went with that.
@David,
>> I wonder why you don't use loop %{ %} blocks?
Those are very reasonable nits to pick. I wasn't using them originally
because I couldn't figure out how to get at a single element of a 2D
piddle, but I figured that out today and added them in. (See below)
>> However, I quite like your use of the #define statements and may add a
note about that in PDL::Book::PP. :-)
Thanks! I got the idea from how polyfill was setup and I needed to be able
to keep the vertx/verty in the algorithm straight since there are so many
different invocations of them.
>> Do you have a fork with this work on the web anywhere? I know, you're
almost done, but if you want, you can easily fork the PDL project at
github: https://github.com/PDLPorters/pdl.
I just did a 'git clone git://pdl.git.sourceforge.net/gitroot/pdl/pdl PDL'.
I usually submit my patches through the mailing list and someone else will
commit them for me. Would I be able to do commits with my own fork?
Here is what I decided on for dynamically creating the pnpoly_pp and
pnpolyfill_pp routines.
my %pnpolyFields = (
'pnpoly_pp' => {'pars' => 'a(m,n); ps(k,l); int [o] msk(m,n)',
'special' => '$msk() = c;'},
'pnpolyfill_pp' => {'pars' => '[o,nc] a(m,n); ps(k,l); int col()',
'special' => 'if(c) { $a() = $col(); }'}
);
for my $name (keys %pnpolyFields) {
pp_def($name,
HandleBad => 0,
Pars => $pnpolyFields{$name}->{'pars'},
Code => '
int i, j, c, nvert;
nvert = $SIZE(l);
#define VERTX(q) $ps(k=>0,l=>q)
#define VERTY(q) $ps(k=>1,l=>q)
threadloop %{
loop(n) %{
loop(m) %{
c = 0;
for(i=0,j=nvert-1;i<nvert;j=i++) {
if( ((VERTY(i)>n) != (VERTY(j)>n)) &&
(m < (VERTX(j)-VERTX(i)) * (n-VERTY(i)) /
(VERTY(j)-VERTY(i)) + VERTX(i)) )
c = !c;
}
' . $pnpolyFields{$name}->{'special'} .'
%}
%}
%}
#undef VERTX
#undef VERTY
'
);
}
Everything is building/testing fine on my Perl 5.14.2 and PDL 2.4.10. I
will give the documentation a thorough inspection, then get a patch out.
Many thanks.
- Tim
On Mon, Jun 18, 2012 at 1:55 PM, Chris Marshall <[email protected]>wrote:
> On Mon, Jun 18, 2012 at 12:30 PM, Tim Haines
> <[email protected]> wrote:
> > Hi, Chris.
> >
> >
> >>> I'm assuming that pnpoly_pp is actually pnpolyfill_pp?
> >
> > pnpoly_pp is just a pure PP version of the C code from
> > http://www.ecse.rpi.edu/Homepages/wrf/Research/Short_Notes/pnpoly.html.
> I
> > did this so that it would allow users to have access to a very fast
> pnpoly
> > routine if they are just looking for the mask and do not need the
> > pnpolyfill[v] functionality. pnpolyfill_pp was implemented separately,
> but
> > using essentially the copy/paste of pnpoly_pp.
>
> Ok, although it is usually better to write some perl
> code to call pp_def() to generated both routines
> from the same code---avoids the case where one
> routine is modified but not the other.
>
> >>> It would be better to have the pnpolyfill/pnpolyfillv accept an
> >>> option to use the PP version or the plain perl/PDL level code.
> >
> > My apologies- I did not explain myself well enough. My thinking was that
> the
> > pnpolyfill[v] functions would only be using the pnpoly_pp functionality
> > since it has significant advantages over the PDL::pnpoly version. Aside
> from
> > simplifying the testing syntax, I struggle to understand why a user would
> > willfully use PDL::pnpoly instead of pnpoly_pp. Perhaps I am too myopic.
>
> This was more a matter of how to stage the development.
> The proposed route is usually good to allow for things to
> be fully tested and incrementally developed. In the case
> where one has a "developer on fire" (TM) then a lot of the
> stages can be skipped. :-)
>
> > Here is a quick overview of what I was envisioning for the
> > PDL::Image2D::poly* namespace:
> >
> > # Leave these alone for backwards compatibility (but update the docs to
> let
> > people know of the pnpoly* versions)
> > PDL::pnpoly
>
> Since the PP and non-PP implementations do the
> same thing---just better, the pnpoly routine should
> be migrated to use pnpoly_pp under the hood.
>
> > polyfill
> > PDL::polyfillv
>
> Yes
>
> > # Add these
> > PDL::pnpolyfill
> > PDL::pnpolyfillv
>
> Yes
>
> > pnpolyfill_pp
> > pnpoly_pp
>
> I recommend these be internal routines only
> since all the functionality should be exposed
> in pnpolyfill and pnpolyfillv.
>
> > with
> >
> >
> > sub PDL::pnpolyfill {
> > my ($im, $ps, $col) = @_;
> > barf("ps must contain pairwise points.\n") unless $ps->getdim(0) == 2;
> > barf("color not specified.\n") unless $col;
> > pnpolyfill_pp($im,$ps,$col);
> > return $im;
> > }
> >
> > and
> >
> >
> > sub PDL::pnpolyfillv :lvalue {
> > my ($im, $ps) = @_;
> > barf("ps must contain pairwise points.\n") unless $ps->getdim(0) == 2;
> >
> > # Here, we have the choice of creating the mask
> > # in the current space and calling pnpolyfill_pp like
> > # what is done in PDL::polyfillv, or we can just have
> > # pnpoly_pp create the intermediate mask for us
> > # chose the latter to keep this as small as possible.
> > return $im->where(pnpoly_pp($im, $ps));
> > }
> >
> > still being Perl/PDL functions so that high-level checks on the inputs
> can
> > be made and still have access to a fast pnpoly implementation.
> >
> > I have written some tests to validate that the results of these pnpoly*
> > routines provide the same results as using PDL::pnpoly and masking
> > techniques, and have found complete agreement with much improved speed. I
> > will include these tests in the t/image2d.t test suite.
> >
> > I apologize for the protracted discussion on these enhancements. This
> being
> > my first true interlude into PP, I tried to proceed with trepidation, but
> > inevitably blundered along the way. I have learned a quite a bit about
> the
> > PP architecture from this exercise, and it has greatly increased my
> > enthusiasm for using PDL.
> >
> > I shall await approval on these proposed changes before proceeding.
>
> Look good to me. Once we have some more user experience
> with the new routine, we could prepare to make polyfill and
> polyfillv call the pnpoly routines under the hood with a flag
> to control the option. Followed by deprecation of the old
> implementation.... Yes, a bit tedious but we're trying not
> to break PDL out from under users.
>
> Thanks for the hard work and coding,
> Chris
>
> > Thank you much for your patience.
> >
> > - Tim
>
_______________________________________________
Perldl mailing list
[email protected]
http://mailman.jach.hawaii.edu/mailman/listinfo/perldl
