Updating tests

[Mateusz Loskot]

Hi,

Currently, I'm making the current collection of tests compiling
and running on my Linux box using GCC 4.3.3 with Boost 1.3.5.

All relevant submissions start with SVN Revision 522 and include
appropriate log message.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Mateusz Loskot]

Mateusz Loskot wrote:
> Hi,
>
> Currently, I'm making the current collection of tests compiling and
> running on my Linux box using GCC 4.3.3 with Boost 1.3.5.
>
> All relevant submissions start with SVN Revision 522 and include
> appropriate log message.

After collection of today updates, my last tests run says that
33 tests passed:

mloskot@dog:~/dev/ggl/_svn/trunk/geometry/test$ bjam | grep passed
**passed** bin/access.test/gcc-4.3.3/debug/access.test
**passed** bin/append.test/gcc-4.3.3/debug/append.test
**passed** bin/area.test/gcc-4.3.3/debug/area.test
**passed** bin/arithmetic.test/gcc-4.3.3/debug/arithmetic.test
**passed** bin/assign.test/gcc-4.3.3/debug/assign.test
**passed** bin/box.test/gcc-4.3.3/debug/box.test
**passed** bin/circle.test/gcc-4.3.3/debug/circle.test
**passed** bin/combine.test/gcc-4.3.3/debug/combine.test
**passed** bin/convert.test/gcc-4.3.3/debug/convert.test
**passed** bin/convex_hull.test/gcc-4.3.3/debug/convex_hull.test
**passed** bin/copy.test/gcc-4.3.3/debug/copy.test
**passed** bin/core.test/gcc-4.3.3/debug/core.test
**passed** bin/custom_linestring.test/gcc-4.3.3/debug/custom_linestring.test
**passed** bin/distance.test/gcc-4.3.3/debug/distance.test
**passed** bin/dot_product.test/gcc-4.3.3/debug/dot_product.test
**passed** bin/envelope.test/gcc-4.3.3/debug/envelope.test
**passed**
bin/for_each_coordinate.test/gcc-4.3.3/debug/for_each_coordinate.test
**passed** bin/loop.test/gcc-4.3.3/debug/loop.test
**passed** bin/make.test/gcc-4.3.3/debug/make.test
**passed** bin/parse.test/gcc-4.3.3/debug/parse.test
**passed** bin/parse_dms.test/gcc-4.3.3/debug/parse_dms.test
**passed** bin/point_ll.test/gcc-4.3.3/debug/point_ll.test
**passed** bin/projection.test/gcc-4.3.3/debug/projection.test
**passed** bin/projection_epsg.test/gcc-4.3.3/debug/projection_epsg.test
**passed** bin/projections.test/gcc-4.3.3/debug/projections.test
**passed** bin/pythagoras.test/gcc-4.3.3/debug/pythagoras.test
**passed** bin/sectionalize.test/gcc-4.3.3/debug/sectionalize.test
**passed** bin/segment.test/gcc-4.3.3/debug/segment.test
**passed** bin/selected.test/gcc-4.3.3/debug/selected.test
**passed** bin/simplify.test/gcc-4.3.3/debug/simplify.test
**passed** bin/transform.test/gcc-4.3.3/debug/transform.test
**passed** bin/transformer.test/gcc-4.3.3/debug/transformer.test
**passed** bin/wkt.test/gcc-4.3.3/debug/wkt.test


Othere tests fail or do not compile.
In this regard I have a question, what's the normalize.hpp file
referenced from within_test.cpp ?

within_test.cpp:9:34: error: geometry/normalize.hpp: No such file or
directory

I can not find it anywhere.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Barend Gehrels]

Hi Mateusz,

Last message today. Perfect! Thanks for all your updates.


> After collection of today updates, my last tests run says that
> 33 tests passed:
>
>  
[snipped]

> Othere tests fail or do not compile.
> In this regard I have a question, what's the normalize.hpp file
> referenced from within_test.cpp ?
>
>  

Normalize.hpp is long ago been renamed to correct.hpp. The within test
should be revived indeed.


Barend

[Mateusz Loskot]

Barend Gehrels wrote:
> Normalize.hpp is long ago been renamed to correct.hpp. The within
> test should be revived indeed.

I've updated the within_test.cpp so it compiles and runs.
However, I had to disable multipolygon in circle test case because
it seem multis are not supported by within, so calculate function can
not be deduced. Perhaps, I'm wrong.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Barend Gehrels]

> I've updated the within_test.cpp so it compiles and runs.
> However, I had to disable multipolygon in circle test case because
> it seem multis are not supported by within, so calculate function can
> not be deduced. Perhaps, I'm wrong.
>
>  
A few pieces of history and comments here.
1) The within_test was one of the first tests I did, in january 2008. It
has been ignored for a long while. So good you took it up!
2) The other tests are usually revised, working for more points then the
default point. See e.g. simplify.cpp or wkt.cpp. They are also renamed,
the _test suffix is dropped (actually because they moved to the test folder)
3) The multi*geometries were present in the first preview but because of
the total rework in the design they were put aside for a while. They are
still not published in boost. I updated them sometimes slightly because
I'm using them myself (at least multipolygon). Last week(s) I revised
them and I think they should take part of the new preview. However, the
polygon-in-circle is still there but the file is moved to
multi/algorithms/within.hpp
4) After the first test we also introduced strategies. There are more
within algorithms implemented for point in polygon, actually they should
be tested as well
5) The default one for point in polygon is the winding counting one, I'm
quite satisified with it because it is "coordinate system agnostic",
meaning that as soon we have a side-strategy implemented for latlong,
the within-test will work in latlong as well. It also does not require a
lot of calculations
6) There has been a problem with the point in polygon (solved now). This
was the combination with the problem:
    from_wkt("POLYGON((146351 410597,146521 410659,147906 410363,148088
410420,148175 410296,148281 409750,148215 *409623*,148154 409666,148154
409666,148130 409625,148035 409626,148035 409626,148008 409544,147963
409510,147993 409457,147961 409352,147261 408687,147008 408586,145714
408840,145001 409033,144486 409066,144616 409308,145023 410286,145254
410488,145618 410612,145618 410612,146015 410565,146190 410545,146351
410597))", poly);
    from_wkt("POINT(146383 *409623*)", point);
    std::cout << within(point, poly) << std::endl;
(having the same y-coordinate and going back). This case should be
added. There are more differences in the "comparisons" of US counties,
they have to be checked, I think GEOS is wrong there and Terralib is
also wrong.

OK, long story but I think it is useful to let this know.

Barend




-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090415/7ab3ced1/attachment.html

[Barend Gehrels]

>
>> I've updated the within_test.cpp so it compiles and runs.
>> However, I had to disable multipolygon in circle test case because
>> it seem multis are not supported by within, so calculate function can
>> not be deduced. Perhaps, I'm wrong.
>>  
> A few pieces of history and comments here. [snipped]

One thing more about this, I forgot this morning:

7) we keep the multi completely separate from the rest. Most
Boost-people are not so interested in multi-polygons etc. So the
multi-polygon can be removed from the within test and a new
"test/multi/within.cpp" can be created

B.

[Mateusz Loskot]

Barend Gehrels wrote:

>
>> I've updated the within_test.cpp so it compiles and runs. However,
>> I had to disable multipolygon in circle test case because it seem
>> multis are not supported by within, so calculate function can not
>> be deduced. Perhaps, I'm wrong.
>
>
> A few pieces of history and comments here. 1) The within_test was one
> of the first tests I did, in january 2008. It has been ignored for a
> long while. So good you took it up!

I just strongly believe in unit tests usefulness :-)

> 2) The other tests are usually revised, working for more points then
> the default point. See e.g. simplify.cpp or wkt.cpp. They are also
> renamed, the _test suffix is dropped (actually because they moved to
> the test folder)

I see.

Are the tests structured in any way or how new tests are added is a
matter of individual programmer choice?

As I see, test files are named after algorithms (intersection) or types
(point_ll or segment) or (probably) namespace or directories
(i.e. core, wkt). Is there any rationale behind this structure?

> 3) The multi*geometries were present in the first preview but because
> of the total rework in the design they were put aside for a while.
> They are still not published in boost. I updated them sometimes
> slightly because I'm using them myself (at least multipolygon). Last
> week(s) I revised them and I think they should take part of the new
> preview. However, the polygon-in-circle is still there but the file
> is moved to multi/algorithms/within.hpp

I see. I've checked, after adding this header, within_test.cpp compiles
and runs well:

#include <geometry/multi/algorithms/within.hpp>

> 4) After the first test we
> also introduced strategies. There are more within algorithms
> implemented for point in polygon, actually they should be tested as
> well

Do you mean that all types and algorithms under geometry/strategies
need to be covered with tests?

5) The default one for point in polygon is the winding counting
> one, I'm quite satisified with it because it is "coordinate system
> agnostic", meaning that as soon we have a side-strategy implemented
> for latlong, the within-test will work in latlong as well. It also
> does not require a lot of calculations

I see.

> 6) There has been a problem
> with the point in polygon (solved now). This was the combination with
> the problem:
>
> fromwkt("POLYGON((146351 410597,146521 410659,147906
> 410363,148088 410420,148175 410296,148281 409750,148215
> *409623*,148154 409666,148154 409666,148130 409625,148035
> 409626,148035 409626,148008 409544,147963 409510,147993 409457,147961
> 409352,147261 408687,147008 408586,145714 408840,145001 409033,144486
> 409066,144616 409308,145023 410286,145254 410488,145618 410612,145618
> 410612,146015 410565,146190 410545,146351 410597))", poly);
> from_wkt("POINT(146383 *409623*)", point);
> std::cout << within(point, poly) << std::endl;
>
> (having the same y-coordinate and going back).
> This case should be added. There are more differences in the
> "comparisons" of US counties, they have to be checked, I think GEOS
> is wrong there and Terralib is also wrong.

That's interesting. I will see what I can find about that.

> OK, long story but I think it is useful to let this know.

Yes, indeed, it's very useful to learn some details of background.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Mateusz Loskot]

Barend Gehrels wrote:

>
>>
>>> I've updated the within_test.cpp so it compiles and runs.
>>> However, I had to disable multipolygon in circle test case because
>>> it seem multis are not supported by within, so calculate function can
>>> not be deduced. Perhaps, I'm wrong.
>>>  
>> A few pieces of history and comments here. [snipped]
>
> One thing more about this, I forgot this morning:
>
> 7) we keep the multi completely separate from the rest. Most
> Boost-people are not so interested in multi-polygons etc. So the
> multi-polygon can be removed from the within test and a new
> "test/multi/within.cpp" can be created

I've extracted the multi-test, committed.
However, I named it multi/multi_within.cpp because there seem to be
names clash between within.cpp and multi/within.cpp - bjam complains:

error: No best alternative for ./within

Perhaps it could be solved with Jamfile per directory.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Barend Gehrels]

Hi Mateusz,



> I've extracted the multi-test, committed.
> However, I named it multi/multi_within.cpp because there seem to be
> names clash between within.cpp and multi/within.cpp - bjam complains:
>  
OK, renaming is perfect for me. Double names (.cpp) give also problems
within MSVC. Double headerfile names are OK. So for the test (for multi)
from now on we take multi_X.cpp.

I will react on you other mails either tonight or tomorrowmorning, as I
had a workshop whole day and am away this evening.

Regards, Barend

[Mateusz Loskot]

Barend Gehrels wrote:
>
>> I've extracted the multi-test, committed.
>> However, I named it multi/multi_within.cpp because there seem to be
>> names clash between within.cpp and multi/within.cpp - bjam complains:
>  
> OK, renaming is perfect for me. Double names (.cpp) give also problems
> within MSVC. Double headerfile names are OK. So for the test (for multi)
> from now on we take multi_X.cpp.

Hi Barend,

OK, let's do so.

> I will react on you other mails either tonight or tomorrowmorning, as I
> had a workshop whole day and am away this evening.

There is no rush at all. We are not obliged to respond ASAP or
(even respond at all). I understand I've flooded the list recently :-)

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Barend Gehrels]

Hi Mateusz,

> There is no rush at all. We are not obliged to respond ASAP or
> (even respond at all). I understand I've flooded the list recently :-)
>  
I like your enthousiasm. I get every mail twice (because a reply). I
tried to configure the list such that it replies automatically to the
list, will see if this is working.

Barend

[Mateusz Loskot]

Barend Gehrels wrote:
> Hi Mateusz,
>
>> There is no rush at all. We are not obliged to respond ASAP or
>> (even respond at all). I understand I've flooded the list recently :-)
>>  
> I like your enthousiasm. I get every mail twice (because a reply). I
> tried to configure the list such that it replies automatically to the
> list, will see if this is working.

OK.

In case of problems, you can ask SAC/Admins:

http://lists.osgeo.org/mailman/listinfo/sac

--
Mateusz Loskot, http://mateusz.loskot.net

[Barend Gehrels]

(sent earlier but meant for the list)

Mateusz Loskot wrote:
> Are the tests structured in any way or how new tests are added is a
> matter of individual programmer choice?
>  
It also depends on the problem at hand (2D/3D, integer/double,
strategies). We didn't create real guidelines for this but here is a
small list
- test_main is provided by Boost
- in test main: call test_all to test for all geometries, using
different point types
- in test_all: call test_geometry for different geometries (linestring,
polygon, box etc)
This works for generic algorithms working the same for all geometries
(such as append). Other tests will need more specific testcases, so then:
- in test_all: call test_point, test_linestring, test_polygon etc (line
in area.cpp, but the _area may be dropped, it is a bit redunant)
There are also specific cases for different strategies (like in area.cpp).


> As I see, test files are named after algorithms (intersection) or types
> (point_ll or segment) or (probably) namespace or directories
> (i.e. core, wkt). Is there any rationale behind this structure?
>  
Testing algorithms is the main purpose. wkt is also a sort of algorithm
but moved to io/wkt to get more structure, we'll also get io/wkb and
maybe more (kml,...)
Testing geometries is especially useful to test the concepts (segment)
or specific methods (latlong), actually they might adapted or merged
with the custom tests (custom_linestring)
Core is a bit weird now, indeed, actually more core-tests might be added

>
> Do you mean that all types and algorithms under geometry/strategies
> need to be covered with tests?
>  
Actually yes, if applicable
>  
>> This case should be added. There are more differences in the
>> "comparisons" of US counties, they have to be checked, I think GEOS
>> is wrong there and Terralib is also wrong.
>>    
>
> That's interesting. I will see what I can find about that.
>  
It is interesting indeed. It was on my list but if you've time to figure
it out, would be useful. I'll send you more about this tonight. If geos
is wrong indeed, we can make a ticket and we should contact terralib. If
we are wrong (and then also cgal is) we have to do some more work :-)

Regards, Barend


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090417/8e39a7a3/attachment.html

[Barend Gehrels]

Hi Mateusz,

> It is interesting indeed. It was on my list but if you've time to
> figure it out, would be useful. I'll send you more about this tonight.
> If geos is wrong indeed, we can make a ticket and we should contact
> terralib. If we are wrong (and then also cgal is) we have to do some
> more work :-)
>
OK, not geos, not ggl, but the comparison program was wrong. Took the
wrong (decreased) envelope, sorry about that. Will commit. Will also
recheck terralib.

Regards, Barend

[Mateusz Loskot]

Barend Gehrels wrote:

> Mateusz Loskot wrote:
>> Are the tests structured in any way or how new tests are added is a
>> matter of individual programmer choice?
>  
> It also depends on the problem at hand (2D/3D, integer/double,
> strategies). We didn't create real guidelines for this but here is a
> small list
> - test_main is provided by Boost
> - in test main: call test_all to test for all geometries, using
> different point types
> - in test_all: call test_geometry for different geometries (linestring,
> polygon, box etc)
> This works for generic algorithms working the same for all geometries
> (such as append). Other tests will need more specific testcases, so then:
> - in test_all: call test_point, test_linestring, test_polygon etc (line
> in area.cpp, but the _area may be dropped, it is a bit redunant)
> There are also specific cases for different strategies (like in area.cpp).

Barend,

These are very helpful. Actually, I don't have any detailed questions
to this, before I start preparing new tests.

Or may be just one question, what about organizing tests in similar tree
as the files which are being tested?
For instance, give the current source tree, tests would go into "test"
subdirectory (following Boost) this way:

<root>/test/core
...
<root>/test/strategies
<root>/test/strategies/spherical
<root>/test/strategies/transform
...

?

>> As I see, test files are named after algorithms (intersection) or types
>> (point_ll or segment) or (probably) namespace or directories
>> (i.e. core, wkt). Is there any rationale behind this structure?
>  
> Testing algorithms is the main purpose.

Right.

> wkt is also a sort of algorithm
> but moved to io/wkt to get more structure, we'll also get io/wkb and
> maybe more (kml,...)

Got it.

> Testing geometries is especially useful to test the concepts (segment)
> or specific methods (latlong), actually they might adapted or merged
> with the custom tests (custom_linestring)
> Core is a bit weird now, indeed, actually more core-tests might be added

Yes, I've noticed core needs some love in this area ;-)

>> Do you mean that all types and algorithms under geometry/strategies
>> need to be covered with tests?
>  
> Actually yes, if applicable

OK

>>> This case should be added. There are more differences in the
>>> "comparisons" of US counties, they have to be checked, I think GEOS
>>> is wrong there and Terralib is also wrong.
>>>    
>>
>> That's interesting. I will see what I can find about that.
>>  
> It is interesting indeed. It was on my list but if you've time to figure
> it out, would be useful. I'll send you more about this tonight. If geos
> is wrong indeed, we can make a ticket and we should contact terralib. If
> we are wrong (and then also cgal is) we have to do some more work :-)

Right. I'm going to be busy moving to new flat but will try to get on it
in the next week.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Mateusz Loskot]

Barend Gehrels wrote:

> Hi Mateusz,
>
>> It is interesting indeed. It was on my list but if you've time to
>> figure it out, would be useful. I'll send you more about this
>> tonight. If geos is wrong indeed, we can make a ticket and we
>> should contact terralib. If we are wrong (and then also cgal is) we
>> have to do some more work :-)
>>
> OK, not geos, not ggl, but the comparison program was wrong. Took the
>  wrong (decreased) envelope, sorry about that. Will commit. Will also
>  recheck terralib.

OK, good to know. Nothing is perfect, no worries :-)

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

[Barend Gehrels]

>
> Or may be just one question, what about organizing tests in similar tree
> as the files which are being tested?
> For instance, give the current source tree, tests would go into "test"
> subdirectory (following Boost) this way:
>
> <root>/test/core
> ...
> <root>/test/strategies
> <root>/test/strategies/spherical
> <root>/test/strategies/transform
> ...
>
> ?
>  

OK to me, good idea. Will I do that April 25 as well? I prefer to do all
those folder / inclusion changes at once, in one session.

>
> Right. I'm going to be busy moving to new flat but will try to get on it
> in the next week.
>  
OK, success, and it gives me some more time for the updated roadmap (not
finished yet)...

Regards, Barend

[Barend Gehrels]

>> OK, not geos, not ggl, but the comparison program was wrong. Took the
>>  wrong (decreased) envelope, sorry about that. Will commit. Will also
>>  recheck terralib.
>>    
>
> OK, good to know. Nothing is perfect, no worries :-)
>  
Terralib is also right (same problem here). So "everything" is right now :-)

Barend

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090419/167fa199/attachment.html

[Mateusz Loskot]

Barend Gehrels wrote:

>
>
>>
>> Or may be just one question, what about organizing tests in similar
>> tree as the files which are being tested? For instance, give the
>> current source tree, tests would go into "test" subdirectory
>> (following Boost) this way:
>>
>> <root>/test/core ... <root>/test/strategies
>> <root>/test/strategies/spherical <root>/test/strategies/transform
>> ...
>>
>> ?
>
> OK to me, good idea. Will I do that April 25 as well? I prefer to do
> all those folder / inclusion changes at once, in one session.

Yes, of course.
Please, make a plan and feel free to assign me with these tasks.

>> Right. I'm going to be busy moving to new flat but will try to get
>> on it in the next week.

I have just moved (yesterday) to new flat, so I'll be waiting a couple
of days for broadband link, but I should be able to submit once a day
from my office.

> OK, success, and it gives me some more time for the updated roadmap
> (not finished yet)...

Thanks. Sure, take your time!

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

[Barend Gehrels]

Hi Mateusz,
>
> Please, make a plan and feel free to assign me with these tasks.
OK, sounds good. Let's assign not a boring task as moving and renaming,
but anticipate on the roadmap. I would like two things in the near
future, which are probably nice to develop:

1) WKB support. I think with your knowlegde and with the WKT as sample
you'll know enough. Streaming is convenient here, like in WKT (but here
also for the input - actually maybe in WKT as well)

2) multi-point in polygon. The first approach might be to call point in
polygon N times (for a multi-point of N points). However, considering a
polygon with V vertices, it will (if K < N) probably much faster to walk
only once through the V vertices, keeping administration in a collection
of size N. This is probably an interesting task and you can also compare
the performance of the two approaches. It is even possible to take an
approach depending on K / N.
Besides that it should be there, I also need it internally for the
buffer/intersection/union where I'm busy with (sometimes), and there I
don't need to know true/false of the collection but per point, and it is
not a multi-point in strict sense but a point-collection.
This probably touches all aspects of the library. The result might be
that the current implementation also has to change. There are currently
more strategies, please take the winding one from
strategies/agnostic/agn_within.hpp as the base.


If you like these tasks you can choose either of them in any order.

I will take care of the folder structure at Friday 25 april, starting
about 13:00 (Dutch time), if you can commit everything that we'll have a
sort of atomic change then without merging.

About the namespace: OK, let's take ggl instead of geometry, I'll take
that in the same round.

Regards, Barend