Benjamin Root <ben.root@...83...> writes:
<snip>
Essentially, you make a Path object using the vertices, and then use its
contains_point() method.
Ben Root
OK, but given that contains_point works with a *single* point at a time, I have
to call it for all my points which is a bit more cumbersome, or am I missing
something?
Jorge
dharhas
>>> Jorge Scandaliaris <jorgesmbox-ml@...1664...> 3/8/2012 3:33 AM >>>
Benjamin Root <ben.root@…83…> writes:
> > Essentially, you make a Path object using the vertices, and then use its > contains_point() method. > Ben Root >OK, but given that contains_point works with a single point at a time, I have
to call it for all my points which is a bit more cumbersome, or am I missing
something?
Jorge
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
Matplotlib-users mailing list
Matplotlib-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-users
One could modify src/_path.cpp to expose a “points_in_path” method so the looping would be done in C++. The underlying work is being done in point_in_path_impl and exposed to python as point_in_path. It would not be too much work to expose a points_in_path method, but we would have to think about what arguments to take and to return. _path.cpp does not currently depend on numpy, and doing things via python list would be slower than what we currently have in nxutils which is array aware.
JDH
On Thu, Mar 8, 2012 at 10:21 AM, Dharhas Pothina <Dharhas.Pothina@…4014…> wrote:
- one on this issue. One of the big advantages of the nxutils points in poly is that you could pass it a large numpy array of points and get back a mask. We found this to be significantly faster than using looping through the single point in poly algorithms from packages like shapely. Echoing Jorge’s question how would we do this using contains_point().
+1 as well. I just took another look at the Path object and I see no such function. The lack of this function is a problem for me as well in my existing apps. In order to deprecate nxutils, this functionality needs to be added to Path. Otherwise, nxutils must be reinstated before the next release.
Ben Root
On Thu, Mar 8, 2012 at 10:21 AM, Dharhas Pothina <Dharhas.Pothina@…4014…> wrote:
- one on this issue. One of the big advantages of the nxutils points in poly is that you could pass it a large numpy array of points and get back a mask. We found this to be significantly faster than using looping through the single point in poly algorithms from packages like shapely. Echoing Jorge’s question how would we do this using contains_point().
- dharhas
>>> Jorge Scandaliaris <jorgesmbox-ml@...1664...> 3/8/2012 3:33 AM >>>
Benjamin Root <ben.root@…120…83…> writes:
> > Essentially, you make a Path object using the vertices, and then use its > contains_point() method. > Ben Root >OK, but given that contains_point works with a single point at a time, I have
to call it for all my points which is a bit more cumbersome, or am I missing
something?Jorge
Michael has already agreed to make a nxutils compatibility layer that would have the same interface as the old nxutils. So we are talking about performance, not core functionality.
We should remember that Michael did the lion’s share of the work on porting mpl to python 3 (https://github.com/matplotlib/matplotlib/pull/565/commits). He elected not to port all of the C++ if he could replace some of the functionality with the core. So those who rely on bare metal speed the you are getting in nxutils should step up to either :
help with the port of nxutils to python 3
help with exposing methods in _path.cpp that are almost as fast or faster
live with slower speeds in the compatibility layer he has agreed to write
ask (nicely) for someone to help you
I prefer option 2 because this is fairly easy and avoids code redundancy. It would take just a few lines of extra code to do this with the python sequence protocol as inputs and python lists as return values. It would take a bit more to support numpy arrays as input and output, and we should get input from Michael about the desirability of making _path.cpp depend on numpy. I don’t see the harm, but I’d like to verify.
In my opinion, a slower implementation in a nxutils.py compatibility module is not a release stopper, even if it is undesirable.
JDH
On Thu, Mar 8, 2012 at 10:32 AM, Benjamin Root <ben.root@…1304…> wrote:
+1 as well. I just took another look at the Path object and I see no such function. The lack of this function is a problem for me as well in my existing apps. In order to deprecate nxutils, this functionality needs to be added to Path. Otherwise, nxutils must be reinstated before the next release.
+1
Python 3 support is more important and, quite frankly,
overdue--especially given how long it's been sitting in the tree.
Ryan
On Thu, Mar 8, 2012 at 10:47 AM, John Hunter <jdh2358@...287...> wrote:
I prefer option 2 because this is fairly easy and avoids code redundancy.
It would take just a few lines of extra code to do this with the python
sequence protocol as inputs and python lists as return values. It would
take a bit more to support numpy arrays as input and output, and we should
get input from Michael about the desirability of making _path.cpp depend on
numpy. I don't see the harm, but I'd like to verify.In my opinion, a slower implementation in a nxutils.py compatibility module
is not a release stopper, even if it is undesirable.
--
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma
Don’t get me wrong. If help is needed, I can certainly provide it since it is my itch. I am just a little exasperated with how this issue has been handled up to now. The shim is a very good idea and it should have been done back when the py3k merge happened. I didn’t press the issue then because I was traveling and didn’t have time to examine the issue closely, and having _nxutils.so still in my build hide the problem from me (my own fault).
As for shim implementation, I would be willing to accept a slightly slower function now (with the promise of improvements later), but if the implementation is too much slower, then effort will need to be made to get it up to acceptable levels. I know of several users coughmy future employercough who uses functionality such as this, and they would not be happy if their products are dragged down by such a bottleneck.
Probably about time I dug more into CXX wrapped stuff…
Ben Root
On Thu, Mar 8, 2012 at 10:47 AM, John Hunter <jdh2358@…83…287…> wrote:
On Thu, Mar 8, 2012 at 10:32 AM, Benjamin Root <ben.root@…1304…> wrote:
+1 as well. I just took another look at the Path object and I see no such function. The lack of this function is a problem for me as well in my existing apps. In order to deprecate nxutils, this functionality needs to be added to Path. Otherwise, nxutils must be reinstated before the next release.
Michael has already agreed to make a nxutils compatibility layer that would have the same interface as the old nxutils. So we are talking about performance, not core functionality.
We should remember that Michael did the lion’s share of the work on porting mpl to python 3 (https://github.com/matplotlib/matplotlib/pull/565/commits). He elected not to port all of the C++ if he could replace some of the functionality with the core. So those who rely on bare metal speed the you are getting in nxutils should step up to either :
- help with the port of nxutils to python 3
- help with exposing methods in _path.cpp that are almost as fast or faster
- live with slower speeds in the compatibility layer he has agreed to write
- ask (nicely) for someone to help you
I prefer option 2 because this is fairly easy and avoids code redundancy. It would take just a few lines of extra code to do this with the python sequence protocol as inputs and python lists as return values. It would take a bit more to support numpy arrays as input and output, and we should get input from Michael about the desirability of making _path.cpp depend on numpy. I don’t see the harm, but I’d like to verify.
In my opinion, a slower implementation in a nxutils.py compatibility module is not a release stopper, even if it is undesirable.
JDH
_path.cpp already has a number of methods that use Numpy arrays
(John is mistaken that it doesn’t depend on Numpy), so adding
another is no problem.
It was my understanding that nxutils was an internal usage module
and not part of the public API, and therefore could be removed as
long as all of the internal references were updated. It doesn’t
have a ‘_’ prefix, so of course it being private wasn’t explicit,
but there are number of things in matplotlib that fall into that
category. Apparently, I was mistaken and there are a lot of users
using it from outside. It shouldn’t be a problem to build a
compatibility shim – I’d much rather do that than have multiple
functions – and adding a points_in_poly to _path.cpp should be
pretty straightforward.
Mike
Looking over the code, it looks like we could generalize point_in_path_impl() into points_in_path_impl(). The current code iterates through the path vertices to test a single point. Putting this action inside a loop (for each point that we want to test) would mean that this iterator has to be processed each time, which I suspect would kill performance when the number of vertices is far greater than the number of test points.
Tinkering…
Ben Root
On Thu, Mar 8, 2012 at 11:16 AM, Benjamin Root <ben.root@…1304…> wrote:
On Thu, Mar 8, 2012 at 10:47 AM, John Hunter <jdh2358@…287…> wrote:
On Thu, Mar 8, 2012 at 10:32 AM, Benjamin Root <ben.root@…1304…> wrote:
+1 as well. I just took another look at the Path object and I see no such function. The lack of this function is a problem for me as well in my existing apps. In order to deprecate nxutils, this functionality needs to be added to Path. Otherwise, nxutils must be reinstated before the next release.
Michael has already agreed to make a nxutils compatibility layer that would have the same interface as the old nxutils. So we are talking about performance, not core functionality.
We should remember that Michael did the lion’s share of the work on porting mpl to python 3 (https://github.com/matplotlib/matplotlib/pull/565/commits). He elected not to port all of the C++ if he could replace some of the functionality with the core. So those who rely on bare metal speed the you are getting in nxutils should step up to either :
- help with the port of nxutils to python 3
- help with exposing methods in _path.cpp that are almost as fast or faster
- live with slower speeds in the compatibility layer he has agreed to write
- ask (nicely) for someone to help you
I prefer option 2 because this is fairly easy and avoids code redundancy. It would take just a few lines of extra code to do this with the python sequence protocol as inputs and python lists as return values. It would take a bit more to support numpy arrays as input and output, and we should get input from Michael about the desirability of making _path.cpp depend on numpy. I don’t see the harm, but I’d like to verify.
In my opinion, a slower implementation in a nxutils.py compatibility module is not a release stopper, even if it is undesirable.
JDH
Don’t get me wrong. If help is needed, I can certainly provide it since it is my itch. I am just a little exasperated with how this issue has been handled up to now. The shim is a very good idea and it should have been done back when the py3k merge happened. I didn’t press the issue then because I was traveling and didn’t have time to examine the issue closely, and having _nxutils.so still in my build hide the problem from me (my own fault).
As for shim implementation, I would be willing to accept a slightly slower function now (with the promise of improvements later), but if the implementation is too much slower, then effort will need to be made to get it up to acceptable levels. I know of several users coughmy future employercough who uses functionality such as this, and they would not be happy if their products are dragged down by such a bottleneck.
Probably about time I dug more into CXX wrapped stuff…
Ben Root
+1 as well. I just took another look at the Path object and I
see no such function. The lack of this function is a problem
for me as well in my existing apps. In order to deprecate
nxutils, this functionality needs to be added to Path.
Otherwise, nxutils *must* be reinstated before the next release.Michael has already agreed to make a nxutils compatibility layer
that would have the same interface as the old nxutils. So we are
talking about performance, not core functionality.We should remember that Michael did the lion's share of the work on
porting mpl to python 3
(https://github.com/matplotlib/matplotlib/pull/565/commits). He
elected not to port all of the C++ if he could replace some of the
functionality with the core. So those who rely on bare metal speed
the you are getting in nxutils should step up to either :1) help with the port of nxutils to python 3
2) help with exposing methods in _path.cpp that are almost as fast
or faster3) live with slower speeds in the compatibility layer he has agreed
to write4) ask (nicely) for someone to help you
I prefer option 2 because this is fairly easy and avoids code
redundancy. It would take just a few lines of extra code to do this
with the python sequence protocol as inputs and python lists as
return values. It would take a bit more to support numpy arrays as
input and output, and we should get input from Michael about the
desirability of making _path.cpp depend on numpy. I don't see the
harm, but I'd like to verify.In my opinion, a slower implementation in a
nxutils.py compatibility module is not a release stopper, even if it
is undesirable.JDH
Don't get me wrong. If help is needed, I can certainly provide it since
it is my itch. I am just a little exasperated with how this issue has
been handled up to now. The shim is a very good idea and it should have
been done back when the py3k merge happened. I didn't press the issue
then because I was traveling and didn't have time to examine the issue
closely, and having _nxutils.so still in my build hide the problem from
me (my own fault).As for shim implementation, I would be willing to accept a slightly
slower function now (with the promise of improvements later), but if the
implementation is too much slower, then effort will need to be made to
get it up to acceptable levels. I know of several users **cough**my
future employer**cough** who uses functionality such as this, and they
would not be happy if their products are dragged down by such a bottleneck.
For my own purposes I decided to simply copy the pnpoly core and write a cython wrapper, since nxutils did not seem to have a secure future in mpl. If anyone is interested in this approach, see the files _pnpoly.c
and utility.pyx (last function therein) in the num/src subdirectory of my "pycurrents" repo: http://currents.soest.hawaii.edu/hgstage/pycurrents/shortlog/tip. The wrapper is brand-new and very lightly tested; I suspect it could be improved considerably. (Also, at present it has only the points_in_poly function.)
Probably about time I dug more into CXX wrapped stuff...
mpl strategy question: stick with CXX and hand-wrapping exclusively, or start using cython?
Eric
On 03/08/2012 07:16 AM, Benjamin Root wrote:
On Thu, Mar 8, 2012 at 10:47 AM, John Hunter <jdh2358@...287... > <mailto:jdh2358@…287…>> wrote:
On Thu, Mar 8, 2012 at 10:32 AM, Benjamin Root <ben.root@...1304... > <mailto:ben.root@…1304…>> wrote:
Ben Root
------------------------------------------------------------------------------
Virtualization& Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/_______________________________________________
Matplotlib-users mailing list
Matplotlib-users@lists.sourceforge.net
matplotlib-users List Signup and Options
It shouldn't be a problem to build a compatibility shim -- I'dmuch rather do that than have multiple functions
Oops -- hit send too soon. I meant to say "I'd much rather do that
than have multiple functions that do virtually the same thing"
Mike
There is a proposed solution to all of this here:
https://github.com/matplotlib/matplotlib/pull/746
Please test -- I don't have any nxutils-using code myself, and
matplotlib itself has none. We should probably convert some of the
nxutils code in the wild into some unit tests.
Mike
As someone who has never contributed to matplotlib before, are there any instructions on how to contribute to writing tests. We have some data and scripts we could probably convert to tests for the nxutils points in poly functions. Should I just do i pull the branch nxutilsbackwards branch and make a pull request after adding tests on github?
Michael Droettboom <mdroe@…86…> 3/8/2012 5:51 PM >>>
There is a proposed solution to all of this here:
https://github.com/matplotlib/matplotlib/pull/746
Please test – I don’t have any nxutils-using code myself, and matplotlib itself has none. We should probably convert some of the nxutils code in the wild into some unit tests.
Mike