breaking change on transform strategies

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

breaking change on transform strategies

Barend
Hi,

As a follow-up of the new approach on strategies, started by Bruno (see
earlier mail about distance strategies), the transform strategies are
now also updated. This means the point-type are not template-parameter
of the strategy anymore, but of the member-method apply.

In most use-cases, as a library user, you have to do nothing. So
probably everything will compile. Also if you are using the transform
algorithm, it will work. Also if you are using svg_mapper (using
underlying transformations), it will work.

If you specify transformation strategies explicitly, such as translate,
scale or rotate, or the map transformer or the inverse transformer,
please update the source. Instead of the two point-types, specify the
calculation-type (usually double) and the dimensions (often 2). The same
strategy will now work for any point-type. If you don't update the
sources, you will get a compilation error, so you will be reminded.

Related changes:
- the svg mapper will now accept any point-type (because transformation
is later bound to point-type)
- the svg mapper now also accepts integer point-types

Regards, Barend

_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Bernhard
This post was updated on .
Hi,

I just updated to boost 1.55 and ran into the propblem of breaking changes on transform strategies.
Reading the announcement, I was wondering if I am doing something wrong.

Barend wrote
Hi,

As a follow-up of the new approach on strategies, started by Bruno (see
earlier mail about distance strategies), the transform strategies are
now also updated. This means the point-type are not template-parameter
of the strategy anymore, but of the member-method apply.

In most use-cases, as a library user, you have to do nothing. So
probably everything will compile. Also if you are using the transform
algorithm, it will work. Also if you are using svg_mapper (using
underlying transformations), it will work.
The reason I ask is that it sounds as if the transformations can somehow be done without the strategies.
Right now I rotate and translate geometries in templated functions, let's say
template<typename tSegment, typename tPoint>
tPoint doFancyStuff(const tSegment segment)

In the function's body I currently do things like
trans::translate_transformer<tPoint, tPoint> translate(...);
trans::rotate_transformer<tPoint, tPoint, boost::geometry::degree> rotate(...);
and then
bg::transform(..., translate);

With the change, as I see it, I now have to declare the strategies as
trans::rotate_transformer<bg::degree, bg::coordinate_type<tPoint>::type,      bg::dimension<tPoint>::value, bg::dimension<tPoint>::value> rotate(...);
trans::translate_transformer<bg::coordinate_type<tPoint>::type, bg::dimension<tPoint>::value, bg::dimension<tPoint>::value> translate(...);

Am I missing something? Could this be done easier? Or if not, (out of curiosity) what is the benefit of the new way of doing it? I have to write much more now.

Additionally, it might be a good idea, to announce these changes as breaking in the release notes. I read those and thought: Well no breaking changes, so I can update with no problems. ;-)

Kind regards,
Bernhard
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Bruno Lalande
Hi Bernhard,

For the distance algorithm (the one I handled the same way), the advantages for the user were more obvious. For this one I agree they're less so. Let me first summarize what they're supposed to be:
* [for the user] More user-friendly - for distance, things like pythagoras<Point1, Point2, CalculationType> became pythagoras<CalculationType>
* [for us] (that was the first motivation) more flexibility within the library - manipulating the strategies with the algo dispatch mechanism is made easier, and some internal tricks are made irrelevant - a recent communication with somebody wanting to contribute a change confirmed the new approach was better
* [generally] Less template instantiations - good for compile times and to avoid reaching number of sections limit on some implementations

Reason 2 is really the main one, as I was blocked on several developments because of that.

But I must admit that in your case, and for this algorithms, it's less appealing. In the case of an end user it's OK (translate_transformer<mypoint, mypoint> would be replaced by e.g. rotate_transformer<double, 2, 2>). But in your case you're using template point types so it gets ugly indeed.

I've looked at the code and those template parameters are indeed needed as class level - they are basically remaining parts of the point types that we couldn't move down to function level. So I think the situation would be a good candidate for maker functions (a la make_pair). This is something I intended to try for distance strategies as well, but here I can see it's even more relevant. Thanks to maker functions, our code could basically become (using C++11 syntax, just replace with BOOST_AUTO otherwise or pass the strategies directly):

auto rotate = make_rotate_transformer(...);
auto translate = make_translate_transformer(...);
bg::transform(..., rotate);
bg::transform(..., translate);

That would automatically return the right type of strategy, depending on the type of parameters passed (so you'd need to use the same type as what your points use, does that make the code even uglier in your situation?) and the number of parameters (e.g. the 9 params overload would return a strategy for points of dimension 2).

Barend: what do you think of this?
Bernhard: would this help at all?

Regards
Bruno

_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Mark R Stallard

I also noticed this change when I installed Boost 1.55 last week.  My typedef's
using strategy::transform::ublas_transformer and inverse_transformer both failed
to compile.  I was able to resolve that by reading matrix_transformers.hpp and
comparing it to the 1.54 version.  After updating my code, I found the new
template format to be more obvious and easier to read.

One related issue not mentioned yet is the doc/html pages for the two types
I mentioned here.  They show the new template format, but have no "Description"
written for the new Parameter named "CalculationType".  My suggestion for that
would be "Matrix cell type (typically double or float)".  Again, I would not
have known what "CalculationType" was supposed to mean if I had not first
read matrix_transformers.hpp.

|+|  M a r k  |+|

    Mark Stallard
    Rapid Response Development

    Business Application Services
    Enterprise Services Information Technology
    Global Business Services
    Raytheon Company



    (business)
    339-645-6423
    (cell)
    617-331-5443
    [hidden email]

235 Presidential Way
Woburn, MA 01801-1060  

www.raytheon.com

This message contains information that may be confidential and privileged. Unless you are the addressee (or authorized to receive mail for the addressee), you should not use, copy or disclose to anyone this message or any information contained in this message. If you have received this message in error, please so advise the sender by reply e-mail and delete this message. Thank you for your cooperation.


Inactive hide details for Bruno Lalande ---11/22/2013 04:44:17 PM---Hi Bernhard, For the distance algorithm (the one I handled Bruno Lalande ---11/22/2013 04:44:17 PM---Hi Bernhard, For the distance algorithm (the one I handled the same way), the advantages

From: Bruno Lalande <[hidden email]>
To: "Boost.Geometry library mailing list" <[hidden email]>
Date: 11/22/2013 04:44 PM
Subject: Re: [geometry] breaking change on transform strategies
Sent by: "Geometry" <[hidden email]>




Hi Bernhard,

For the distance algorithm (the one I handled the same way), the advantages for the user were more obvious. For this one I agree they're less so. Let me first summarize what they're supposed to be:
* [for the user] More user-friendly - for distance, things like pythagoras<Point1, Point2, CalculationType> became pythagoras<CalculationType>
* [for us] (that was the first motivation) more flexibility within the library - manipulating the strategies with the algo dispatch mechanism is made easier, and some internal tricks are made irrelevant - a recent communication with somebody wanting to contribute a change confirmed the new approach was better
* [generally] Less template instantiations - good for compile times and to avoid reaching number of sections limit on some implementations

Reason 2 is really the main one, as I was blocked on several developments because of that.

But I must admit that in your case, and for this algorithms, it's less appealing. In the case of an end user it's OK (translate_transformer<mypoint, mypoint> would be replaced by e.g. rotate_transformer<double, 2, 2>). But in your case you're using template point types so it gets ugly indeed.

I've looked at the code and those template parameters are indeed needed as class level - they are basically remaining parts of the point types that we couldn't move down to function level. So I think the situation would be a good candidate for maker functions (a la make_pair). This is something I intended to try for distance strategies as well, but here I can see it's even more relevant. Thanks to maker functions, our code could basically become (using C++11 syntax, just replace with BOOST_AUTO otherwise or pass the strategies directly):

auto rotate = make_rotate_transformer(...);

auto translate = make_translate_transformer(...);
bg::transform(..., rotate);
bg::transform(..., translate);

That would automatically return the right type of strategy, depending on the type of parameters passed (so you'd need to use the same type as what your points use, does that make the code even uglier in your situation?) and the number of parameters (e.g. the 9 params overload would return a strategy for points of dimension 2).

Barend: what do you think of this?
Bernhard: would this help at all?

Regards
Bruno
_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry


_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Barend
Hi,

Right, sorry that this change did not make the Release Notes. It is indeed a breaking change.

There has been send a clear mail about this but of course I agree that is not enough.

http://boost-geometry.203548.n3.nabble.com/breaking-change-on-transform-strategies-td4025494.html



Mark R Stallard wrote On 22-11-2013 23:03:

I also noticed this change when I installed Boost 1.55 last week.  My typedef's
using strategy::transform::ublas_transformer and inverse_transformer both failed
to compile.  I was able to resolve that by reading matrix_transformers.hpp and
comparing it to the 1.54 version.  After updating my code, I found the new
template format to be more obvious and easier to read.

One related issue not mentioned yet is the doc/html pages for the two types
I mentioned here.  They show the new template format, but have no "Description"
written for the new Parameter named "CalculationType".  My suggestion for that
would be "Matrix cell type (typically double or float)".  Again, I would not
have known what "CalculationType" was supposed to mean if I had not first
read matrix_transformers.hpp.



Yes, besides the release notes the docs have not been updated. All this has slipped through, sorry again.




For the distance algorithm (the one I handled the same way), the advantages for the user were more obvious. For this one I agree they're less so. Let me first summarize what they're supposed to be:
* [for the user] More user-friendly - for distance, things like pythagoras<Point1, Point2, CalculationType> became pythagoras<CalculationType>
* [for us] (that was the first motivation) more flexibility within the library - manipulating the strategies with the algo dispatch mechanism is made easier, and some internal tricks are made irrelevant - a recent communication with somebody wanting to contribute a change confirmed the new approach was better
* [generally] Less template instantiations - good for compile times and to avoid reaching number of sections limit on some implementations

Reason 2 is really the main one, as I was blocked on several developments because of that.


For transform: some things could not be done in the past (up to 1.54). It was not possible to (once) define a strategy and then apply the transform function with any point-type afterwards. That was inconvenient.

For the rest it aligned with the other actions.

However, indeed we could not get rid of everything, because a matrix must have a type (e.g. double) and some dimensions to store the matrix itself. They cannot be decuded at a call, that is too late, they have to be instantiated first.



But I must admit that in your case, and for this algorithms, it's less appealing. In the case of an end user it's OK (translate_transformer<mypoint, mypoint> would be replaced by e.g. rotate_transformer<double, 2, 2>). But in your case you're using template point types so it gets ugly indeed.

I've looked at the code and those template parameters are indeed needed as class level - they are basically remaining parts of the point types that we couldn't move down to function level. So I think the situation would be a good candidate for maker functions (a la make_pair). This is something I intended to try for distance strategies as well, but here I can see it's even more relevant. Thanks to maker functions, our code could basically become (using C++11 syntax, just replace with BOOST_AUTO otherwise or pass the strategies directly):

auto rotate = make_rotate_transformer(...);

auto translate = make_translate_transformer(...);
bg::transform(..., rotate);
bg::transform(..., translate);

That would automatically return the right type of strategy, depending on the type of parameters passed (so you'd need to use the same type as what your points use, does that make the code even uglier in your situation?) and the number of parameters (e.g. the 9 params overload would return a strategy for points of dimension 2).

Barend: what do you think of this?
Bernhard: would this help at all?


The way you (Bernhard) did this (using coordinate_type and dimension) is OK. But indeed larger and more complex.

The additional functions of Bruno to make this easier sound good to me.

I hope that my change was OK and in line with the other changes on strategies, the point-type, formerly specified, was just used to get the information (coordinate type, dimensions, coordinate system) from, it was not really used inside the strategy. Now this is moved to user-level, the change itself was actually small (the library did not change a lot). But it is now a bit more flexible, for those who use that.

I cannot remember why I did not add this immediately afterwards inside the release notes and the docs, that would have been more convenient. Anyway we can do that now (as soon git is up and running), because there are also many users skipping a version.

Regards, Barend



_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Bernhard
In reply to this post by Bruno Lalande
Bruno Lalande wrote
Thanks to maker functions, our code could basically
become (using C++11 syntax, just replace with BOOST_AUTO otherwise or pass
the strategies directly):

auto rotate = make_rotate_transformer(...);
auto translate = make_translate_transformer(...);
bg::transform(..., rotate);
bg::transform(..., translate);

That would automatically return the right type of strategy, depending on
the type of parameters passed (so you'd need to use the same type as what
your points use, does that make the code even uglier in your situation?)
and the number of parameters (e.g. the 9 params overload would return a
strategy for points of dimension 2).

Barend: what do you think of this?
Bernhard: would this help at all?
Thanks for the explanation, I understand the reason for the change better now. Also, I'm in favour of everything that reduces compile time. ;-)

For me, I think, I can live with it as it is now. But generally I think this kind of maker functions would be a good addition.

What I do not yet understand, is why there are two dimension parameters. If this can indeed be used with different source and target dimensions, then I think the documentation should explain what happens if the dimensions are unequal, because I cannot think of a "standard" behaviour for that.

Regards,
Bernhard
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Bruno Lalande
Hi Bernhard,

What I do not yet understand, is why there are two dimension parameters. If
this can indeed be used with different source and target dimensions, then I
think the documentation should explain what happens if the dimensions are
unequal, because I cannot think of a "standard" behaviour for that.

Yep, I seem to remember having seen that in the code - it's possible and valid to have a 2D and a 3D point. Not sure how it's handled, Barend can probably explain. Barend, given such a usage is quite infrequent maybe we can make the 2nd one optional? (i.e. equal to the first one by default)

Regards
Bruno

_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: breaking change on transform strategies

Barend
Hi Bruno,

Bruno Lalande wrote On 30-11-2013 22:27:
Hi Bernhard,

What I do not yet understand, is why there are two dimension parameters. If
this can indeed be used with different source and target dimensions, then I
think the documentation should explain what happens if the dimensions are
unequal, because I cannot think of a "standard" behaviour for that.

Yep, I seem to remember having seen that in the code - it's possible and valid to have a 2D and a 3D point. Not sure how it's handled, Barend can probably explain. Barend, given such a usage is quite infrequent maybe we can make the 2nd one optional? (i.e. equal to the first one by default)

Yes, good idea. Indeed it is possible to go from 2D to 3D or v.v., and indeed it is not frequent.

Regards, Barend


_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry