pre-review

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

pre-review

Radu B. Rusu
Administrator
Hi all,

We're finally ready to kick off the API review process for PCL. This
will probably constitute a big structural change, so before creating
the wiki pages and start filling them in, we need to do two more
things:

1) decide on how packages and stacks that contain PCL (& friends) will
be named in D-Turtle

2) decide what scheme we will use to guarantee a stable API throughout
D-Turtle, but also support additional development (new "unstable"
extensions that will not be API reviewed immediately)

With respect to 1), after a few discussions we came up with the
following proposal. If you do not agree with it, speak up, and let's
decide on something better _now_ :)

[point_cloud_perception] currently contains: ann, cminpack, eigen3,
flann, laser_scan_geometry, pcl, pcl_ros, pcl_tf, pcl_tutorials,
pcl_visualization, point_cloud_converter, terminal_tools.

 * [ann] will hopefully be deprecated (obsolete) when Marius finishes
the new [flann] version
 * [pcl_tf] should be moved into [pcl_ros]. There's no reason for it
to stay separate (traditionally [pcl]+[pcl_ros] were in a single
package, and we didn't want to bring an additional TF dependency, but
now [pcl_ros] already has a TF dependency)
 * [point_cloud_converter] should be moved to [sensor_msgs] - if we
can still add it there
 * [laser_scan_geometry] should be moved to [laser_pipeline] and maybe
renamed (?) - if we can still add it there

 * maybe [eigen3] should be moved to [geometry]?

And then, split the remaining packages into two stacks:
 * [perception_pcl]: could contain [pcl], [pcl_ros], [flann],
[cminpack], (+[eigen3] if not moved to [geometry]). This guarantees
that the stack contains a minimal set of packages, doesn't add too
many dependencies -- a goal is to be able to deploy these packages on
smaller embedded systems, where visualization and other tools are not
needed (see below)
 * [perception_pcl_addon] - or some name like that: could contain
[pcl_tutorials], [pcl_visualization], [terminal_tools]. This adds
additional dependencies such as VTK and wxWidgets for visualization,
contains tutorials and other demos (which are not part of the PCL core
but are useful for learning, etc), etc

With respect to 2), it gets a bit tricker. Let's write down the goals first:

 * a set of methods that get API reviewed and are marked as "stable",
should not change their API for the duration of a ROS distribution (6
months), which means that PCL releases need to guarantee stability for
at least 6 months for that set of methods/classes, so that application
developers can enjoy a solid API
 * because the 3D perception field is young, we want to attract users
and developers fast, otherwise we iterate too slowly and our robots
will still behave poorly (and navigation/manipulation people will
blame us :) ). We need users/developers for two things: a) new
functionality; b) using and testing the current functionality to we
make sure it _works_
 * we want to progress fast, and do not encourage users (only
developers) to work on trunk. Users should get both _stable_
functionality but also _new experimental_ functionality marked
accordingly (see below) as soon as possible (so that we can test and
iterate), also in binary releases, without needing to compile the
library from source


So the key idea here is to be able to mark _stable_ vs _unstable_ for
research projects such as PCL. Some suggestions that we could think
of:

  * all unstable (non API reviewed) methods/classes, should be marked
accordingly in the documentation. [we need to find the right Doxygen
label.]

  * unstable headers could be marked using #warning:
     #ifndef PCL_UNSTABLE
     #warning “Foo is an experimental feature and is not part of the
stable API!”
     #endif

  * another solution that could work on a per-class and per-function
level at compile time would be something like BOOST_STATIC_ASSERT(x)
which throws a compiler error. This could be used such that code that
uses experimental features within pcl need to define a flag like
USE_EXPERIMENTAL_API.

  * Finally, we could also A) split the API in stable and unstable
parts, and create a pcl_experimental package in perception_pcl_addon
that contains additions to pcl, or B) branch off of the stable version
of pcl, and create a new stack perception_pcl2 with its own namespace
pcl2, where development can happen.

However, we are not perfectly happy with either solution, since we
either make it too hard for ourselves in the future when we want to
add new functionality or we break things too often for others, so we
would definitely like to hear your ideas on this issue.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

garratt
Hi Radu, everyone.

I have some feedback, a rant and a crazy idea.

For 1:
I like the idea of separating visualization stuff into a different
stack.  Other than that, (since I check out all pcl related stuff
anyway) My biggest concern is that the namespaces not change around too
often: i.e. I don't care what stack the packages are in, as long as the
package name doesn't change. :)

For 2:
Why don't we just have a stable svn tag?  It seems to me that is exactly
what svn tags are for.

I am against splitting the API in stable and unstable parts , especially
if package names change, as that hurts developers who are using
experimental code that becomes stable.

the branching suggestion  (B) is similar to the tag idea, but I think
changing the package name is unnecessary.

I think one of the benefits of the ros structure is that switching
between tags and branches on a system level is as easy as changing an
environmental variable.  Creating a stable tag would capitalize on that
feature.  To that end, I would suggest that the stable tag cover as much
as possible (i.e. the whole stack)

> * we want to progress fast, and do not encourage users (only
> developers) to work on trunk. Users should get both _stable_
> functionality but also _new experimental_ functionality marked
> accordingly (see below) as soon as possible (so that we can test and
> iterate), also in binary releases, without needing to compile the
> library from source

---Rant Alert---
I applaud the push to get new users in to the ros fold, and the binary
packages certainly help with that. However, I feel (and maybe this is a
rant for the ros-users list) that most of the ros users are programmers,
who don't have a problem running 'make' (especially since binary
dependencies are explicitly listed, and suggested if the build breaks).
I, for one prefer the svn checkout approach to the binary  approach
because it gives me greater control (if a new commit breaks the build, I
can just update to a previous version, for example)
All of this is doubly true for something like pcl (unstable versions
especially) where the intended level of interaction is to use pcl
libraries in your own code.

However, given that ROS seems to be going in the binary direction,  I
can accept that a simple tag won't be sufficient, and if you end up
getting two binaries which have the 'pcl' package, there can be
confusion (particularly if you have some packages which require the
stable version and some which require the unstable version, so you can't
just set the environmental variable)  I agree with your sentiment that
the suggested solutions are not very satisfying.  So what about the
following:


---Crazy Idea Alert---
Although it would probably be a huge pain for the ROS core developers,
an elegant solution to this would be introducing versioning into the ROS
dependency system.  You could have multiple versions of a package in
your ROS_PACKAGE_PATH, (with a default version specified somehow) and
packages could depend on the default version, or a specific version, or
a range of versions.  (this makes me think of boost, and how cmake does
it's find_package) Since each package would specify the version in it's
manifest, the verion could change with svn, and would get propagated to
the binaries.  This would also allow ros users to run old code much
easier, while maintaining the latest version of the repository.  It
would also take care of the issue where you download some code which
depends on (for example) pcl, but you have no idea what svn revision or
even what repository if came from.

ooo, I like this idea.  Let's discuss.

cheers,
Garratt



On Sun, 2010-11-07 at 20:01 +0100, Radu Bogdan Rusu wrote:

> Hi all,
>
> We're finally ready to kick off the API review process for PCL. This
> will probably constitute a big structural change, so before creating
> the wiki pages and start filling them in, we need to do two more
> things:
>
> 1) decide on how packages and stacks that contain PCL (& friends) will
> be named in D-Turtle
>
> 2) decide what scheme we will use to guarantee a stable API throughout
> D-Turtle, but also support additional development (new "unstable"
> extensions that will not be API reviewed immediately)
>
> With respect to 1), after a few discussions we came up with the
> following proposal. If you do not agree with it, speak up, and let's
> decide on something better _now_ :)
>
> [point_cloud_perception] currently contains: ann, cminpack, eigen3,
> flann, laser_scan_geometry, pcl, pcl_ros, pcl_tf, pcl_tutorials,
> pcl_visualization, point_cloud_converter, terminal_tools.
>
>  * [ann] will hopefully be deprecated (obsolete) when Marius finishes
> the new [flann] version
>  * [pcl_tf] should be moved into [pcl_ros]. There's no reason for it
> to stay separate (traditionally [pcl]+[pcl_ros] were in a single
> package, and we didn't want to bring an additional TF dependency, but
> now [pcl_ros] already has a TF dependency)
>  * [point_cloud_converter] should be moved to [sensor_msgs] - if we
> can still add it there
>  * [laser_scan_geometry] should be moved to [laser_pipeline] and maybe
> renamed (?) - if we can still add it there
>
>  * maybe [eigen3] should be moved to [geometry]?
>
> And then, split the remaining packages into two stacks:
>  * [perception_pcl]: could contain [pcl], [pcl_ros], [flann],
> [cminpack], (+[eigen3] if not moved to [geometry]). This guarantees
> that the stack contains a minimal set of packages, doesn't add too
> many dependencies -- a goal is to be able to deploy these packages on
> smaller embedded systems, where visualization and other tools are not
> needed (see below)
>  * [perception_pcl_addon] - or some name like that: could contain
> [pcl_tutorials], [pcl_visualization], [terminal_tools]. This adds
> additional dependencies such as VTK and wxWidgets for visualization,
> contains tutorials and other demos (which are not part of the PCL core
> but are useful for learning, etc), etc
>
> With respect to 2), it gets a bit tricker. Let's write down the goals first:
>
>  * a set of methods that get API reviewed and are marked as "stable",
> should not change their API for the duration of a ROS distribution (6
> months), which means that PCL releases need to guarantee stability for
> at least 6 months for that set of methods/classes, so that application
> developers can enjoy a solid API
>  * because the 3D perception field is young, we want to attract users
> and developers fast, otherwise we iterate too slowly and our robots
> will still behave poorly (and navigation/manipulation people will
> blame us :) ). We need users/developers for two things: a) new
> functionality; b) using and testing the current functionality to we
> make sure it _works_
>  * we want to progress fast, and do not encourage users (only
> developers) to work on trunk. Users should get both _stable_
> functionality but also _new experimental_ functionality marked
> accordingly (see below) as soon as possible (so that we can test and
> iterate), also in binary releases, without needing to compile the
> library from source
>
>
> So the key idea here is to be able to mark _stable_ vs _unstable_ for
> research projects such as PCL. Some suggestions that we could think
> of:
>
>   * all unstable (non API reviewed) methods/classes, should be marked
> accordingly in the documentation. [we need to find the right Doxygen
> label.]
>
>   * unstable headers could be marked using #warning:
>      #ifndef PCL_UNSTABLE
>      #warning “Foo is an experimental feature and is not part of the
> stable API!”
>      #endif
>
>   * another solution that could work on a per-class and per-function
> level at compile time would be something like BOOST_STATIC_ASSERT(x)
> which throws a compiler error. This could be used such that code that
> uses experimental features within pcl need to define a flag like
> USE_EXPERIMENTAL_API.
>
>   * Finally, we could also A) split the API in stable and unstable
> parts, and create a pcl_experimental package in perception_pcl_addon
> that contains additions to pcl, or B) branch off of the stable version
> of pcl, and create a new stack perception_pcl2 with its own namespace
> pcl2, where development can happen.
>
> However, we are not perfectly happy with either solution, since we
> either make it too hard for ourselves in the future when we want to
> add new functionality or we break things too often for others, so we
> would definitely like to hear your ideas on this issue.
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator
Sorry for the late reply... Soooo much going on right now!

On 11/09/2010 08:54 AM, garratt wrote:
> For 1:
> I like the idea of separating visualization stuff into a different
> stack.  Other than that, (since I check out all pcl related stuff
> anyway) My biggest concern is that the namespaces not change around too
> often: i.e. I don't care what stack the packages are in, as long as the
> package name doesn't change. :)

Awesome. We'll try to do this today or tomorrow.

>
> For 2:
> Why don't we just have a stable svn tag?  It seems to me that is exactly
> what svn tags are for.
>
> I am against splitting the API in stable and unstable parts , especially
> if package names change, as that hurts developers who are using
> experimental code that becomes stable.

I agree too.

>
> the branching suggestion  (B) is similar to the tag idea, but I think
> changing the package name is unnecessary.

Agreed.

>
> I think one of the benefits of the ros structure is that switching
> between tags and branches on a system level is as easy as changing an
> environmental variable.  Creating a stable tag would capitalize on that
> feature.  To that end, I would suggest that the stable tag cover as much
> as possible (i.e. the whole stack)

Agreed. But we need a PCL (outside of ROS) too, and whatever releases we make there have to disturb our users in the
least possible invasive manner :)

> ---Crazy Idea Alert---
> Although it would probably be a huge pain for the ROS core developers,
> an elegant solution to this would be introducing versioning into the ROS
> dependency system.  You could have multiple versions of a package in
> your ROS_PACKAGE_PATH, (with a default version specified somehow) and
> packages could depend on the default version, or a specific version, or
> a range of versions.  (this makes me think of boost, and how cmake does
> it's find_package) Since each package would specify the version in it's
> manifest, the verion could change with svn, and would get propagated to
> the binaries.  This would also allow ros users to run old code much
> easier, while maintaining the latest version of the repository.  It
> would also take care of the issue where you download some code which
> depends on (for example) pcl, but you have no idea what svn revision or
> even what repository if came from.

This is very interesting and I think we should discuss this and make it happen once Diamondback is out. I'm in favor,
though we need to iterate to find the right solution.



In the meantime... for stable vs unstable code. How does this sound?

  * stable - API reviewed, guaranteed not to change, marked accordingly in the documentation

  * unstable - not API reviewed, could change, marked accordingly in the documentation, AND...

- since there is no _attribute__ ((unstable)) in C++, but only a _attribute__ ((deprecated)), we cannot mark unstable
functions like that. However the goal is to get the user to _see_ a compiler warning when he/she uses that part of the code.

- Lorenz proposed using BOOST_STATIC_ASSERT (or looking at the magic that goes inside there to get the same
functionality). However, BOOST_STATIC_ASSERT will...ermm... assert if you use a method that contains it, at compile
time. So this is an error not a warning.

However, we can assert on #ifndef ENABLE_PCL_EXPERIMENTAL !


Bottom line: the user has to enable that. If not, and the user tries to use one of the unstable/unreviewed functions...
boom, error. In time, with every new release, we try to stabilize those functions, and we remove the STATIC_ASSERT,
similar to how older functions get marked as deprecated when new better ones are written.



Thoughts? Other ideas?


Cheers,
Radu.




_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

garratt
> In the meantime... for stable vs unstable code. How does this sound?
>
>   * stable - API reviewed, guaranteed not to change, marked accordingly in the documentation
>
>   * unstable - not API reviewed, could change, marked accordingly in the documentation, AND...
>
> - since there is no _attribute__ ((unstable)) in C++, but only a _attribute__ ((deprecated)), we cannot mark unstable
> functions like that. However the goal is to get the user to _see_ a compiler warning when he/she uses that part of the code.
>
> - Lorenz proposed using BOOST_STATIC_ASSERT (or looking at the magic that goes inside there to get the same
> functionality). However, BOOST_STATIC_ASSERT will...ermm... assert if you use a method that contains it, at compile
> time. So this is an error not a warning.
>
> However, we can assert on #ifndef ENABLE_PCL_EXPERIMENTAL !
>
>
> Bottom line: the user has to enable that. If not, and the user tries to use one of the unstable/unreviewed functions...
> boom, error. In time, with every new release, we try to stabilize those functions, and we remove the STATIC_ASSERT,
> similar to how older functions get marked as deprecated when new better ones are written.
>

I'm confused. Are you advocating errors or warnings?  

also, just to clarify, we are talking about functions that would exist
in both the stable and unstable tags (or trunk vs cturtle tag), but
whose api might change more frequently in the unstable(/trunk), right?

Either way, I foresee the following problem:

Developer A includes unstable PCL functionality in a library, and to get
it to compile, he puts #define ENABLE_PCL_EXPERIMENTAL in his code.  Now
dev B comes along, and includes that header, possibly not even including
the unstable bit of Dev A's code.  Now all protections that we hoped to
provide Dev B is nullified. If Dev A wanted to be responsible about
this, we would be forcing him to offer separate functionalities based on
whether whoever uses his header has #defined ENABLE_PCL_EXPERIMENTAL
already.

Also, if an error (or warning) is generated, I would support a method
which explains in a clear and easily remedied way:
-what is going on (you are using unstable code)
-how to make the error/warning go away (#define something)
-what circumventing this warning means for your code (your code will now
be unstable)

I'm not sure that:
 Illegal use of STATIC_ASSERTION_FAILURE<false>
fits that bill, although there may be a way to make boost generate
better errors.

Just some thoughts...
Garratt


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Geoffrey Biggs
In reply to this post by garratt
On 10/11/10 01:54, garratt wrote:

> ---Crazy Idea Alert---
> Although it would probably be a huge pain for the ROS core developers,
> an elegant solution to this would be introducing versioning into the ROS
> dependency system.  You could have multiple versions of a package in
> your ROS_PACKAGE_PATH, (with a default version specified somehow) and
> packages could depend on the default version, or a specific version, or
> a range of versions.  (this makes me think of boost, and how cmake does
> it's find_package) Since each package would specify the version in it's
> manifest, the verion could change with svn, and would get propagated to
> the binaries.  This would also allow ros users to run old code much
> easier, while maintaining the latest version of the repository.  It
> would also take care of the issue where you download some code which
> depends on (for example) pcl, but you have no idea what svn revision or
> even what repository if came from.
>
> ooo, I like this idea.  Let's discuss.

Not as crazy as you might think. Gentoo has been doing pretty much
exactly this for years. They call them "slots." It allows any package on
the system to co-exist with other versions by specifying which slot each
version gets installed into in the package manifest. A slot can be as
fine-grained as the developer likes, but typically they are done at the
major version number level.

The argument I heard against having versioning in ROS packages of
developers using too many different versions of packages doesn't work
for me, chiefly because I've been using Gentoo for nearly 8 years now
and I've never seen slots cause a problem. Seeing ROS missing such an
important feature as versioning that is present in many other
source-level package management systems makes me sad.

Geoff
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Nico Blodow
In reply to this post by Radu B. Rusu
Hi everyone,

we finally got around to answering this and blurt out our unqualified
opinion.

So, the first question we would like to raise is, what are we trying to
achieve? ABI stability? API stability?

If ABI: the static assert solution would be impossible, because changes to
the class break the ABI.

If API: in this case, the static assert would work, but it "feels like a
hack" to Lorenz, and we don't want that. So the question is: where would
this actually be needed? There are some things like computeCentroid,
"covariance" functions and the like that don't live in any class, so
additional features could go into separate files or at least live on their
own.

In short, our suggestion is to have a package pcl which contains a stable
core, which basically encompasses all functionality that is used within
(important/official/released) packages that depend on pcl (We're looking
at you, nav_stack).

This would probably mean filters, several ransac classes, and clustering,
as well as all the little helper functions such as in io and common.

The rest should go into a "supplementary" package (pcl_experimental). We
should make sure that the namespaces in pcl and pcl_experimental are the
same to prevent confusion in the future when stuff becomes stable.

This would mean that we don't break ABI nor API, since API is stable in
PCL, at least for one ROS release. New development would completely happen
in pcl_experimental, so this implies that only classes and functions that
are _really_ stable can move into PCL. We know it's hard to "let go" of
code like this, but any other solution quickly gets messy. We can't have
both, complete freedom to change stuff at will and a stable and released
API that others depend upon.

With respect to marking/warning/compile time asserting about experimental
features, this would not be necessary anymore, since you willingly depend
on pcl_experimental anyway.

The proposed naming scheme (perception_pcl/perception_pcl_addon) looks
fine to us.

Cheers,
Nico and Lorenz



> Hi all,
>
> We're finally ready to kick off the API review process for PCL. This
> will probably constitute a big structural change, so before creating
> the wiki pages and start filling them in, we need to do two more
> things:
>
> 1) decide on how packages and stacks that contain PCL (& friends) will
> be named in D-Turtle
>
> 2) decide what scheme we will use to guarantee a stable API throughout
> D-Turtle, but also support additional development (new "unstable"
> extensions that will not be API reviewed immediately)
>
> With respect to 1), after a few discussions we came up with the
> following proposal. If you do not agree with it, speak up, and let's
> decide on something better _now_ :)
>
> [point_cloud_perception] currently contains: ann, cminpack, eigen3,
> flann, laser_scan_geometry, pcl, pcl_ros, pcl_tf, pcl_tutorials,
> pcl_visualization, point_cloud_converter, terminal_tools.
>
>  * [ann] will hopefully be deprecated (obsolete) when Marius finishes
> the new [flann] version
>  * [pcl_tf] should be moved into [pcl_ros]. There's no reason for it
> to stay separate (traditionally [pcl]+[pcl_ros] were in a single
> package, and we didn't want to bring an additional TF dependency, but
> now [pcl_ros] already has a TF dependency)
>  * [point_cloud_converter] should be moved to [sensor_msgs] - if we
> can still add it there
>  * [laser_scan_geometry] should be moved to [laser_pipeline] and maybe
> renamed (?) - if we can still add it there
>
>  * maybe [eigen3] should be moved to [geometry]?
>
> And then, split the remaining packages into two stacks:
>  * [perception_pcl]: could contain [pcl], [pcl_ros], [flann],
> [cminpack], (+[eigen3] if not moved to [geometry]). This guarantees
> that the stack contains a minimal set of packages, doesn't add too
> many dependencies -- a goal is to be able to deploy these packages on
> smaller embedded systems, where visualization and other tools are not
> needed (see below)
>  * [perception_pcl_addon] - or some name like that: could contain
> [pcl_tutorials], [pcl_visualization], [terminal_tools]. This adds
> additional dependencies such as VTK and wxWidgets for visualization,
> contains tutorials and other demos (which are not part of the PCL core
> but are useful for learning, etc), etc
>
> With respect to 2), it gets a bit tricker. Let's write down the goals
> first:
>
>  * a set of methods that get API reviewed and are marked as "stable",
> should not change their API for the duration of a ROS distribution (6
> months), which means that PCL releases need to guarantee stability for
> at least 6 months for that set of methods/classes, so that application
> developers can enjoy a solid API
>  * because the 3D perception field is young, we want to attract users
> and developers fast, otherwise we iterate too slowly and our robots
> will still behave poorly (and navigation/manipulation people will
> blame us :) ). We need users/developers for two things: a) new
> functionality; b) using and testing the current functionality to we
> make sure it _works_
>  * we want to progress fast, and do not encourage users (only
> developers) to work on trunk. Users should get both _stable_
> functionality but also _new experimental_ functionality marked
> accordingly (see below) as soon as possible (so that we can test and
> iterate), also in binary releases, without needing to compile the
> library from source
>
>
> So the key idea here is to be able to mark _stable_ vs _unstable_ for
> research projects such as PCL. Some suggestions that we could think
> of:
>
>   * all unstable (non API reviewed) methods/classes, should be marked
> accordingly in the documentation. [we need to find the right Doxygen
> label.]
>
>   * unstable headers could be marked using #warning:
>      #ifndef PCL_UNSTABLE
>      #warning “Foo is an experimental feature and is not part of the
> stable API!”
>      #endif
>
>   * another solution that could work on a per-class and per-function
> level at compile time would be something like BOOST_STATIC_ASSERT(x)
> which throws a compiler error. This could be used such that code that
> uses experimental features within pcl need to define a flag like
> USE_EXPERIMENTAL_API.
>
>   * Finally, we could also A) split the API in stable and unstable
> parts, and create a pcl_experimental package in perception_pcl_addon
> that contains additions to pcl, or B) branch off of the stable version
> of pcl, and create a new stack perception_pcl2 with its own namespace
> pcl2, where development can happen.
>
> However, we are not perfectly happy with either solution, since we
> either make it too hard for ourselves in the future when we want to
> add new functionality or we break things too often for others, so we
> would definitely like to hear your ideas on this issue.
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users
>
>


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator
Hi all,

Based on the discussion(s) that we had, we put together a Roadmap (see http://www.ros.org/wiki/pcl/Roadmap) regarding
the versioning policy and the proposed development model for PCL.

We hope this addresses most of the problems that have been discussed. However, can everyone interested take another peek
at it, and let's do one more iteration to see what needs to be changed before we carve it into stone? :)

Cheers,
Radu.


On 11/24/2010 07:51 AM, Nico Blodow wrote:

> Hi everyone,
>
> we finally got around to answering this and blurt out our unqualified
> opinion.
>
> So, the first question we would like to raise is, what are we trying to
> achieve? ABI stability? API stability?
>
> If ABI: the static assert solution would be impossible, because changes to
> the class break the ABI.
>
> If API: in this case, the static assert would work, but it "feels like a
> hack" to Lorenz, and we don't want that. So the question is: where would
> this actually be needed? There are some things like computeCentroid,
> "covariance" functions and the like that don't live in any class, so
> additional features could go into separate files or at least live on their
> own.
>
> In short, our suggestion is to have a package pcl which contains a stable
> core, which basically encompasses all functionality that is used within
> (important/official/released) packages that depend on pcl (We're looking
> at you, nav_stack).
>
> This would probably mean filters, several ransac classes, and clustering,
> as well as all the little helper functions such as in io and common.
>
> The rest should go into a "supplementary" package (pcl_experimental). We
> should make sure that the namespaces in pcl and pcl_experimental are the
> same to prevent confusion in the future when stuff becomes stable.
>
> This would mean that we don't break ABI nor API, since API is stable in
> PCL, at least for one ROS release. New development would completely happen
> in pcl_experimental, so this implies that only classes and functions that
> are _really_ stable can move into PCL. We know it's hard to "let go" of
> code like this, but any other solution quickly gets messy. We can't have
> both, complete freedom to change stuff at will and a stable and released
> API that others depend upon.
>
> With respect to marking/warning/compile time asserting about experimental
> features, this would not be necessary anymore, since you willingly depend
> on pcl_experimental anyway.
>
> The proposed naming scheme (perception_pcl/perception_pcl_addon) looks
> fine to us.
>
> Cheers,
> Nico and Lorenz
>
>
>
>> Hi all,
>>
>> We're finally ready to kick off the API review process for PCL. This
>> will probably constitute a big structural change, so before creating
>> the wiki pages and start filling them in, we need to do two more
>> things:
>>
>> 1) decide on how packages and stacks that contain PCL (&  friends) will
>> be named in D-Turtle
>>
>> 2) decide what scheme we will use to guarantee a stable API throughout
>> D-Turtle, but also support additional development (new "unstable"
>> extensions that will not be API reviewed immediately)
>>
>> With respect to 1), after a few discussions we came up with the
>> following proposal. If you do not agree with it, speak up, and let's
>> decide on something better _now_ :)
>>
>> [point_cloud_perception] currently contains: ann, cminpack, eigen3,
>> flann, laser_scan_geometry, pcl, pcl_ros, pcl_tf, pcl_tutorials,
>> pcl_visualization, point_cloud_converter, terminal_tools.
>>
>>   * [ann] will hopefully be deprecated (obsolete) when Marius finishes
>> the new [flann] version
>>   * [pcl_tf] should be moved into [pcl_ros]. There's no reason for it
>> to stay separate (traditionally [pcl]+[pcl_ros] were in a single
>> package, and we didn't want to bring an additional TF dependency, but
>> now [pcl_ros] already has a TF dependency)
>>   * [point_cloud_converter] should be moved to [sensor_msgs] - if we
>> can still add it there
>>   * [laser_scan_geometry] should be moved to [laser_pipeline] and maybe
>> renamed (?) - if we can still add it there
>>
>>   * maybe [eigen3] should be moved to [geometry]?
>>
>> And then, split the remaining packages into two stacks:
>>   * [perception_pcl]: could contain [pcl], [pcl_ros], [flann],
>> [cminpack], (+[eigen3] if not moved to [geometry]). This guarantees
>> that the stack contains a minimal set of packages, doesn't add too
>> many dependencies -- a goal is to be able to deploy these packages on
>> smaller embedded systems, where visualization and other tools are not
>> needed (see below)
>>   * [perception_pcl_addon] - or some name like that: could contain
>> [pcl_tutorials], [pcl_visualization], [terminal_tools]. This adds
>> additional dependencies such as VTK and wxWidgets for visualization,
>> contains tutorials and other demos (which are not part of the PCL core
>> but are useful for learning, etc), etc
>>
>> With respect to 2), it gets a bit tricker. Let's write down the goals
>> first:
>>
>>   * a set of methods that get API reviewed and are marked as "stable",
>> should not change their API for the duration of a ROS distribution (6
>> months), which means that PCL releases need to guarantee stability for
>> at least 6 months for that set of methods/classes, so that application
>> developers can enjoy a solid API
>>   * because the 3D perception field is young, we want to attract users
>> and developers fast, otherwise we iterate too slowly and our robots
>> will still behave poorly (and navigation/manipulation people will
>> blame us :) ). We need users/developers for two things: a) new
>> functionality; b) using and testing the current functionality to we
>> make sure it _works_
>>   * we want to progress fast, and do not encourage users (only
>> developers) to work on trunk. Users should get both _stable_
>> functionality but also _new experimental_ functionality marked
>> accordingly (see below) as soon as possible (so that we can test and
>> iterate), also in binary releases, without needing to compile the
>> library from source
>>
>>
>> So the key idea here is to be able to mark _stable_ vs _unstable_ for
>> research projects such as PCL. Some suggestions that we could think
>> of:
>>
>>    * all unstable (non API reviewed) methods/classes, should be marked
>> accordingly in the documentation. [we need to find the right Doxygen
>> label.]
>>
>>    * unstable headers could be marked using #warning:
>>       #ifndef PCL_UNSTABLE
>>       #warning “Foo is an experimental feature and is not part of the
>> stable API!”
>>       #endif
>>
>>    * another solution that could work on a per-class and per-function
>> level at compile time would be something like BOOST_STATIC_ASSERT(x)
>> which throws a compiler error. This could be used such that code that
>> uses experimental features within pcl need to define a flag like
>> USE_EXPERIMENTAL_API.
>>
>>    * Finally, we could also A) split the API in stable and unstable
>> parts, and create a pcl_experimental package in perception_pcl_addon
>> that contains additions to pcl, or B) branch off of the stable version
>> of pcl, and create a new stack perception_pcl2 with its own namespace
>> pcl2, where development can happen.
>>
>> However, we are not perfectly happy with either solution, since we
>> either make it too hard for ourselves in the future when we want to
>> add new functionality or we break things too often for others, so we
>> would definitely like to hear your ideas on this issue.
>> _______________________________________________
>> [hidden email] / http://pcl.ros.org
>> https://code.ros.org/mailman/listinfo/pcl-users
>>
>>
>
>
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Lorenz Mösenlechner
+1 for the roadmap from my side.

> Hi all,
>
> Based on the discussion(s) that we had, we put together a Roadmap (see http://www.ros.org/wiki/pcl/Roadmap) regarding
> the versioning policy and the proposed development model for PCL.
>
> We hope this addresses most of the problems that have been discussed. However, can everyone interested take another peek
> at it, and let's do one more iteration to see what needs to be changed before we carve it into stone? :)
>
> Cheers,
> Radu.
>
>
> On 11/24/2010 07:51 AM, Nico Blodow wrote:
> > Hi everyone,
> >
> > we finally got around to answering this and blurt out our unqualified
> > opinion.
> >
> > So, the first question we would like to raise is, what are we trying to
> > achieve? ABI stability? API stability?
> >
> > If ABI: the static assert solution would be impossible, because changes to
> > the class break the ABI.
> >
> > If API: in this case, the static assert would work, but it "feels like a
> > hack" to Lorenz, and we don't want that. So the question is: where would
> > this actually be needed? There are some things like computeCentroid,
> > "covariance" functions and the like that don't live in any class, so
> > additional features could go into separate files or at least live on their
> > own.
> >
> > In short, our suggestion is to have a package pcl which contains a stable
> > core, which basically encompasses all functionality that is used within
> > (important/official/released) packages that depend on pcl (We're looking
> > at you, nav_stack).
> >
> > This would probably mean filters, several ransac classes, and clustering,
> > as well as all the little helper functions such as in io and common.
> >
> > The rest should go into a "supplementary" package (pcl_experimental). We
> > should make sure that the namespaces in pcl and pcl_experimental are the
> > same to prevent confusion in the future when stuff becomes stable.
> >
> > This would mean that we don't break ABI nor API, since API is stable in
> > PCL, at least for one ROS release. New development would completely happen
> > in pcl_experimental, so this implies that only classes and functions that
> > are _really_ stable can move into PCL. We know it's hard to "let go" of
> > code like this, but any other solution quickly gets messy. We can't have
> > both, complete freedom to change stuff at will and a stable and released
> > API that others depend upon.
> >
> > With respect to marking/warning/compile time asserting about experimental
> > features, this would not be necessary anymore, since you willingly depend
> > on pcl_experimental anyway.
> >
> > The proposed naming scheme (perception_pcl/perception_pcl_addon) looks
> > fine to us.
> >
> > Cheers,
> > Nico and Lorenz
> >
> >
> >
> >> Hi all,
> >>
> >> We're finally ready to kick off the API review process for PCL. This
> >> will probably constitute a big structural change, so before creating
> >> the wiki pages and start filling them in, we need to do two more
> >> things:
> >>
> >> 1) decide on how packages and stacks that contain PCL (&  friends) will
> >> be named in D-Turtle
> >>
> >> 2) decide what scheme we will use to guarantee a stable API throughout
> >> D-Turtle, but also support additional development (new "unstable"
> >> extensions that will not be API reviewed immediately)
> >>
> >> With respect to 1), after a few discussions we came up with the
> >> following proposal. If you do not agree with it, speak up, and let's
> >> decide on something better _now_ :)
> >>
> >> [point_cloud_perception] currently contains: ann, cminpack, eigen3,
> >> flann, laser_scan_geometry, pcl, pcl_ros, pcl_tf, pcl_tutorials,
> >> pcl_visualization, point_cloud_converter, terminal_tools.
> >>
> >>   * [ann] will hopefully be deprecated (obsolete) when Marius finishes
> >> the new [flann] version
> >>   * [pcl_tf] should be moved into [pcl_ros]. There's no reason for it
> >> to stay separate (traditionally [pcl]+[pcl_ros] were in a single
> >> package, and we didn't want to bring an additional TF dependency, but
> >> now [pcl_ros] already has a TF dependency)
> >>   * [point_cloud_converter] should be moved to [sensor_msgs] - if we
> >> can still add it there
> >>   * [laser_scan_geometry] should be moved to [laser_pipeline] and maybe
> >> renamed (?) - if we can still add it there
> >>
> >>   * maybe [eigen3] should be moved to [geometry]?
> >>
> >> And then, split the remaining packages into two stacks:
> >>   * [perception_pcl]: could contain [pcl], [pcl_ros], [flann],
> >> [cminpack], (+[eigen3] if not moved to [geometry]). This guarantees
> >> that the stack contains a minimal set of packages, doesn't add too
> >> many dependencies -- a goal is to be able to deploy these packages on
> >> smaller embedded systems, where visualization and other tools are not
> >> needed (see below)
> >>   * [perception_pcl_addon] - or some name like that: could contain
> >> [pcl_tutorials], [pcl_visualization], [terminal_tools]. This adds
> >> additional dependencies such as VTK and wxWidgets for visualization,
> >> contains tutorials and other demos (which are not part of the PCL core
> >> but are useful for learning, etc), etc
> >>
> >> With respect to 2), it gets a bit tricker. Let's write down the goals
> >> first:
> >>
> >>   * a set of methods that get API reviewed and are marked as "stable",
> >> should not change their API for the duration of a ROS distribution (6
> >> months), which means that PCL releases need to guarantee stability for
> >> at least 6 months for that set of methods/classes, so that application
> >> developers can enjoy a solid API
> >>   * because the 3D perception field is young, we want to attract users
> >> and developers fast, otherwise we iterate too slowly and our robots
> >> will still behave poorly (and navigation/manipulation people will
> >> blame us :) ). We need users/developers for two things: a) new
> >> functionality; b) using and testing the current functionality to we
> >> make sure it _works_
> >>   * we want to progress fast, and do not encourage users (only
> >> developers) to work on trunk. Users should get both _stable_
> >> functionality but also _new experimental_ functionality marked
> >> accordingly (see below) as soon as possible (so that we can test and
> >> iterate), also in binary releases, without needing to compile the
> >> library from source
> >>
> >>
> >> So the key idea here is to be able to mark _stable_ vs _unstable_ for
> >> research projects such as PCL. Some suggestions that we could think
> >> of:
> >>
> >>    * all unstable (non API reviewed) methods/classes, should be marked
> >> accordingly in the documentation. [we need to find the right Doxygen
> >> label.]
> >>
> >>    * unstable headers could be marked using #warning:
> >>       #ifndef PCL_UNSTABLE
> >>       #warning “Foo is an experimental feature and is not part of the
> >> stable API!”
> >>       #endif
> >>
> >>    * another solution that could work on a per-class and per-function
> >> level at compile time would be something like BOOST_STATIC_ASSERT(x)
> >> which throws a compiler error. This could be used such that code that
> >> uses experimental features within pcl need to define a flag like
> >> USE_EXPERIMENTAL_API.
> >>
> >>    * Finally, we could also A) split the API in stable and unstable
> >> parts, and create a pcl_experimental package in perception_pcl_addon
> >> that contains additions to pcl, or B) branch off of the stable version
> >> of pcl, and create a new stack perception_pcl2 with its own namespace
> >> pcl2, where development can happen.
> >>
> >> However, we are not perfectly happy with either solution, since we
> >> either make it too hard for ourselves in the future when we want to
> >> add new functionality or we break things too often for others, so we
> >> would definitely like to hear your ideas on this issue.
> >> _______________________________________________
> >> [hidden email] / http://pcl.ros.org
> >> https://code.ros.org/mailman/listinfo/pcl-users
> >>
> >>
> >
> >
> > _______________________________________________
> > [hidden email] / http://pcl.ros.org
> > https://code.ros.org/mailman/listinfo/pcl-users
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users
>
--
Lorenz M�senlechner            | [hidden email]
Technische Universit�t M�nchen | Boltzmannstr. 3
85748 Garching bei M�nchen     | Germany
http://ias.cs.tum.edu/         | Tel: +49 (89) 289-26910

_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Geoffrey Biggs
In reply to this post by Radu B. Rusu
I'll just give you a list of comments. In no particular order...

I don't like the odd/even style of releasing. I think it's great for
developers and clued-in users who understand the development process,
but the less experienced users may run off and use the unstable because
"it's the latest version." I like the OGRE style (actually, closer to
the Player style) where there's an unstable trunk and a stable branch
with the fixes for the most recent stable release, plus a non-broken-API
branch for the next minor release. This style does not preclude releases
at a set interval, but I do not feel that regular releases of major
versions with different APIs are beneficial for a library - that's a
distro thing. Having said that, if you feel that users will be OK with
the odd/even cycle then go for that (with 1.0.1 etc bug fix releases).

For versioning, I think that the wikipedia explanation leaves out one
important principle: if a breaking API change is made, it requires a
major version increment; if a new API is added, it only requires a minor
version increment. This means that the "stable" version of PCL can still
have minor version increments, so 1.0, 1.1, etc will be equivalent in
terms of "stable," but may contain new features that are not API-breaking.

The release schedule seems a tad fast - it implies your API is going to
change constantly. Is the API really likely to break that often? A year
between major releases seems more realistic (and more reasonable for
users) to me.

I don't like the idea of changing the namespace for every version. Do
you have a real-world scenario for someone wanting to use stuff from two
major versions of the library at once in the same binary (i.e. not in
separate ROS nodes)? I think that the namespace should remain the same,
while the library so version and the include directory suffix changes to
indicate the version number. This allows multiple versions to be
installed at once but only allows one two be used at a time, and is
standard practice. I don't see the argument for allowing a user to use
algorithms from PCL 1.0 and PCL 2.0 at once (in fact, I don't understand
why you would remove algorithms from PCL at all).

Having ROS packages per version number seems likely to confuse users
(and is a direct consequence of ROS not supporting package versioning, I
guess). There should be *the* PCL package, and ROS users use whichever
one is in their ROS distribution.

PCL does need to iterate fast, but this iteration is supposed to occur
in the pre-1.0 (i.e. before the first "stable" release) releases. Let's
not rush for the glorious tag of 1.0 before we're ready, only to have to
introduce huge API breakage 3 months later. We can still try to keep the
API stable before the 1.0 release (only change it in minor release
increments, for example). The fact that there are so many users out
there already shows that people are not that worried about using
pre-stable software.

Geoff


On 29/11/10 10:43, Radu Bogdan Rusu wrote:

> Hi all,
>
> Based on the discussion(s) that we had, we put together a Roadmap (see http://www.ros.org/wiki/pcl/Roadmap) regarding
> the versioning policy and the proposed development model for PCL.
>
> We hope this addresses most of the problems that have been discussed. However, can everyone interested take another peek
> at it, and let's do one more iteration to see what needs to be changed before we carve it into stone? :)
>
> Cheers,
> Radu.
>
>
> On 11/24/2010 07:51 AM, Nico Blodow wrote:
>> Hi everyone,
>>
>> we finally got around to answering this and blurt out our unqualified
>> opinion.
>>
>> So, the first question we would like to raise is, what are we trying to
>> achieve? ABI stability? API stability?
>>
>> If ABI: the static assert solution would be impossible, because changes to
>> the class break the ABI.
>>
>> If API: in this case, the static assert would work, but it "feels like a
>> hack" to Lorenz, and we don't want that. So the question is: where would
>> this actually be needed? There are some things like computeCentroid,
>> "covariance" functions and the like that don't live in any class, so
>> additional features could go into separate files or at least live on their
>> own.
>>
>> In short, our suggestion is to have a package pcl which contains a stable
>> core, which basically encompasses all functionality that is used within
>> (important/official/released) packages that depend on pcl (We're looking
>> at you, nav_stack).
>>
>> This would probably mean filters, several ransac classes, and clustering,
>> as well as all the little helper functions such as in io and common.
>>
>> The rest should go into a "supplementary" package (pcl_experimental). We
>> should make sure that the namespaces in pcl and pcl_experimental are the
>> same to prevent confusion in the future when stuff becomes stable.
>>
>> This would mean that we don't break ABI nor API, since API is stable in
>> PCL, at least for one ROS release. New development would completely happen
>> in pcl_experimental, so this implies that only classes and functions that
>> are _really_ stable can move into PCL. We know it's hard to "let go" of
>> code like this, but any other solution quickly gets messy. We can't have
>> both, complete freedom to change stuff at will and a stable and released
>> API that others depend upon.
>>
>> With respect to marking/warning/compile time asserting about experimental
>> features, this would not be necessary anymore, since you willingly depend
>> on pcl_experimental anyway.
>>
>> The proposed naming scheme (perception_pcl/perception_pcl_addon) looks
>> fine to us.
>>
>> Cheers,
>> Nico and Lorenz
>>
>>
>>
>>> Hi all,
>>>
>>> We're finally ready to kick off the API review process for PCL. This
>>> will probably constitute a big structural change, so before creating
>>> the wiki pages and start filling them in, we need to do two more
>>> things:
>>>
>>> 1) decide on how packages and stacks that contain PCL (&  friends) will
>>> be named in D-Turtle
>>>
>>> 2) decide what scheme we will use to guarantee a stable API throughout
>>> D-Turtle, but also support additional development (new "unstable"
>>> extensions that will not be API reviewed immediately)
>>>
>>> With respect to 1), after a few discussions we came up with the
>>> following proposal. If you do not agree with it, speak up, and let's
>>> decide on something better _now_ :)
>>>
>>> [point_cloud_perception] currently contains: ann, cminpack, eigen3,
>>> flann, laser_scan_geometry, pcl, pcl_ros, pcl_tf, pcl_tutorials,
>>> pcl_visualization, point_cloud_converter, terminal_tools.
>>>
>>>   * [ann] will hopefully be deprecated (obsolete) when Marius finishes
>>> the new [flann] version
>>>   * [pcl_tf] should be moved into [pcl_ros]. There's no reason for it
>>> to stay separate (traditionally [pcl]+[pcl_ros] were in a single
>>> package, and we didn't want to bring an additional TF dependency, but
>>> now [pcl_ros] already has a TF dependency)
>>>   * [point_cloud_converter] should be moved to [sensor_msgs] - if we
>>> can still add it there
>>>   * [laser_scan_geometry] should be moved to [laser_pipeline] and maybe
>>> renamed (?) - if we can still add it there
>>>
>>>   * maybe [eigen3] should be moved to [geometry]?
>>>
>>> And then, split the remaining packages into two stacks:
>>>   * [perception_pcl]: could contain [pcl], [pcl_ros], [flann],
>>> [cminpack], (+[eigen3] if not moved to [geometry]). This guarantees
>>> that the stack contains a minimal set of packages, doesn't add too
>>> many dependencies -- a goal is to be able to deploy these packages on
>>> smaller embedded systems, where visualization and other tools are not
>>> needed (see below)
>>>   * [perception_pcl_addon] - or some name like that: could contain
>>> [pcl_tutorials], [pcl_visualization], [terminal_tools]. This adds
>>> additional dependencies such as VTK and wxWidgets for visualization,
>>> contains tutorials and other demos (which are not part of the PCL core
>>> but are useful for learning, etc), etc
>>>
>>> With respect to 2), it gets a bit tricker. Let's write down the goals
>>> first:
>>>
>>>   * a set of methods that get API reviewed and are marked as "stable",
>>> should not change their API for the duration of a ROS distribution (6
>>> months), which means that PCL releases need to guarantee stability for
>>> at least 6 months for that set of methods/classes, so that application
>>> developers can enjoy a solid API
>>>   * because the 3D perception field is young, we want to attract users
>>> and developers fast, otherwise we iterate too slowly and our robots
>>> will still behave poorly (and navigation/manipulation people will
>>> blame us :) ). We need users/developers for two things: a) new
>>> functionality; b) using and testing the current functionality to we
>>> make sure it _works_
>>>   * we want to progress fast, and do not encourage users (only
>>> developers) to work on trunk. Users should get both _stable_
>>> functionality but also _new experimental_ functionality marked
>>> accordingly (see below) as soon as possible (so that we can test and
>>> iterate), also in binary releases, without needing to compile the
>>> library from source
>>>
>>>
>>> So the key idea here is to be able to mark _stable_ vs _unstable_ for
>>> research projects such as PCL. Some suggestions that we could think
>>> of:
>>>
>>>    * all unstable (non API reviewed) methods/classes, should be marked
>>> accordingly in the documentation. [we need to find the right Doxygen
>>> label.]
>>>
>>>    * unstable headers could be marked using #warning:
>>>       #ifndef PCL_UNSTABLE
>>>       #warning “Foo is an experimental feature and is not part of the
>>> stable API!”
>>>       #endif
>>>
>>>    * another solution that could work on a per-class and per-function
>>> level at compile time would be something like BOOST_STATIC_ASSERT(x)
>>> which throws a compiler error. This could be used such that code that
>>> uses experimental features within pcl need to define a flag like
>>> USE_EXPERIMENTAL_API.
>>>
>>>    * Finally, we could also A) split the API in stable and unstable
>>> parts, and create a pcl_experimental package in perception_pcl_addon
>>> that contains additions to pcl, or B) branch off of the stable version
>>> of pcl, and create a new stack perception_pcl2 with its own namespace
>>> pcl2, where development can happen.
>>>
>>> However, we are not perfectly happy with either solution, since we
>>> either make it too hard for ourselves in the future when we want to
>>> add new functionality or we break things too often for others, so we
>>> would definitely like to hear your ideas on this issue.
>>> _______________________________________________
>>> [hidden email] / http://pcl.ros.org
>>> https://code.ros.org/mailman/listinfo/pcl-users
>>>
>>>
>>
>>
>> _______________________________________________
>> [hidden email] / http://pcl.ros.org
>> https://code.ros.org/mailman/listinfo/pcl-users
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Jack O'Quin
On Mon, Nov 29, 2010 at 6:46 PM, Geoffrey Biggs
<[hidden email]> wrote:

> For versioning, I think that the wikipedia explanation leaves out one
> important principle: if a breaking API change is made, it requires a
> major version increment; if a new API is added, it only requires a minor
> version increment. This means that the "stable" version of PCL can still
> have minor version increments, so 1.0, 1.1, etc will be equivalent in
> terms of "stable," but may contain new features that are not API-breaking.

I think the Wikipedia article is just trying to describe the wide
range of software version numbers in use. I doubt they intended it as
any kind of "best practices" recommendation.

I agree with Geoffrey's recommendation to only change major number
(post-1.0) for an incompatible API change. Minor numbers would change
for a new release, which may contain extensions that are
backwards-compatible. Micro numbers correspond to bug fixes, with no
change to the interface.

Beginning with the Diamondback release, these micro updates need to be
binary-compatible (no change to API or ABI). The idea is that a bug
fix should not require other packages depending on the library to be
re-released.

> The release schedule seems a tad fast - it implies your API is going to
> change constantly. Is the API really likely to break that often? A year
> between major releases seems more realistic (and more reasonable for
> users) to me.

+1

> I don't like the idea of changing the namespace for every version. Do
> you have a real-world scenario for someone wanting to use stuff from two
> major versions of the library at once in the same binary (i.e. not in
> separate ROS nodes)? I think that the namespace should remain the same,
> while the library so version and the include directory suffix changes to
> indicate the version number. This allows multiple versions to be
> installed at once but only allows one two be used at a time, and is
> standard practice. I don't see the argument for allowing a user to use
> algorithms from PCL 1.0 and PCL 2.0 at once (in fact, I don't understand
> why you would remove algorithms from PCL at all).

+1

The namespace might legitimately change for a new (incompatible) major
version. But, those should not happen every year.

> Having ROS packages per version number seems likely to confuse users
> (and is a direct consequence of ROS not supporting package versioning, I
> guess). There should be *the* PCL package, and ROS users use whichever
> one is in their ROS distribution.

The ROS release unit is the stack, in this case
point_cloud_perception. Stacks have version numbers and correspond to
Debian packages. That seems reasonable to me.

> PCL does need to iterate fast, but this iteration is supposed to occur
> in the pre-1.0 (i.e. before the first "stable" release) releases. Let's
> not rush for the glorious tag of 1.0 before we're ready, only to have to
> introduce huge API breakage 3 months later. We can still try to keep the
> API stable before the 1.0 release (only change it in minor release
> increments, for example).

+1

> The fact that there are so many users out
> there already shows that people are not that worried about using
> pre-stable software.

Some people.

We decided to wait until it stabilizes before using it at UT Austin
with undergraduate students.  Maybe next semester. PCL does look like
it will be very useful for some of these research projects.
--
 joq
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator
In reply to this post by garratt
I wasn't sure what e-mail to reply to, so here goes:


On 11/09/2010 08:54 AM, garratt wrote:
> I like the idea of separating visualization stuff into a different
> stack.  Other than that, (since I check out all pcl related stuff
> anyway) My biggest concern is that the namespaces not change around too
> often: i.e. I don't care what stack the packages are in, as long as the
> package name doesn't change. :)


We create two new stacks today, to mirror the conclusion of our discussion:

  * perception_pcl, with: ann, cminpack, eigen3, flann, pcl, pcl_ros, and pcl_tf

  * perception_pcl_addons, with: pcl_tutorials, pcl_visualization, and terminal_tools

The two new stacks are accessible at:

https://code.ros.org/svn/ros-pkg/stacks/perception_pcl/
(use https://code.ros.org/svn/ros-pkg/stacks/perception_pcl/trunk for now as there is no release)
and
https://code.ros.org/svn/ros-pkg/stacks/perception_pcl_addons/
(use https://code.ros.org/svn/ros-pkg/stacks/perception_pcl_addons/trunk for now as there is no release).

We expect to have perception_pcl 0.6 released tomorrow the latest. We'll change the versioning number on
perception_pcl_addons to 0.1, as it represents something else right now.



The documentation of the stacks is not up properly yet, but we managed to rename all the pages under

http://www.ros.org/wiki/point_cloud_perception/XXXX

to

http://www.ros.org/wiki/perception_pcl/XXXX

We also added a notice to the point_cloud_perception stack. In the next few days, we should arrange these pages better,
and add content to perception_pcl_addons.

All the other pages should be intact as they are relative to the name of the package (thank God!). :)

Cheers,
Radu.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator
In reply to this post by Geoffrey Biggs

On 11/29/2010 04:46 PM, Geoffrey Biggs wrote:

> I don't like the odd/even style of releasing. I think it's great for
> developers and clued-in users who understand the development process,
> but the less experienced users may run off and use the unstable because
> "it's the latest version." I like the OGRE style (actually, closer to
> the Player style) where there's an unstable trunk and a stable branch
> with the fixes for the most recent stable release, plus a non-broken-API
> branch for the next minor release. This style does not preclude releases
> at a set interval, but I do not feel that regular releases of major
> versions with different APIs are beneficial for a library - that's a
> distro thing. Having said that, if you feel that users will be OK with
> the odd/even cycle then go for that (with 1.0.1 etc bug fix releases).


I also like the OGRE style best. I'm not a big fan of odd/even, and it doesn't seem to fit PCL's development cycle. That
can change in the future, but I agree that for now we should have OGRE/Player-like releases (Geoff, can you actually add
Player to that page please? It's important to have it as a reference). Also, it seems that the Linux kernel dropped that
model already.

> For versioning, I think that the wikipedia explanation leaves out one
> important principle: if a breaking API change is made, it requires a
> major version increment; if a new API is added, it only requires a minor
> version increment. This means that the "stable" version of PCL can still
> have minor version increments, so 1.0, 1.1, etc will be equivalent in
> terms of "stable," but may contain new features that are not API-breaking.

I agree with you here, and that's why I added this in the text:
" * a stable branch, so they can build solid applications on top. We can keep the API 100% fixed and do only patch
releases  (or additionally we can consider adding minor features that do not break the API -- need a discussion about
what's considered minor)"

The only problem is that, as far as I understand, we're not allowed to do that in a ROS package throughout a
distribution. So if PCL 1.0 gets released in Diamondback, we cannot add new features to it -- only bugfixes -- in
Diamondback. All the development has to happen in a different stack/branch/package... I don't like this too much, but I
_can_ understand the reasons for it. We want to have a "D-Turtle" distribution, and packages in it should work
independent of versions and etc throughout its 6-months life-cycle.

The solution here is ABI compatibility, but until we make this work, we're stuck with the above policy.

> The release schedule seems a tad fast - it implies your API is going to
> change constantly. Is the API really likely to break that often? A year
> between major releases seems more realistic (and more reasonable for
> users) to me.


Good point. I added the following comments to the wiki:

"""
That being said, we will try to guarantee:

  * '''up to 100% API backwards compatibility between releases'''. New releases should build on top of the old releases
and add new features, and attempt not to break the base API through any core modifications;

  * any new API modification decision has to be supported by a '''strong case/argument'''. The proposer needs to clearly
explain why the current API cannot accommodate the existing design decisions.

The above two points will hopefully address the -- what it may seem -- rapid development pace, as shown in the next section.
"""

The API should not change from 1.0 to 2.0. I think it's realistic to expect "a lot of new functionality" over the course
of 6 months.

The naming structure for ROS packages is only useful in umm.... ROS :) so pcl/pcl2/pcl3/... The other alternative would
be branching, but that would make it hard for users to have both pcl_stable and pcl_devel in the same distribution.

Non-ROS users will simply enjoy a PCL library with a linear growing versioning scheme, and for them none of these things
should matter.

> I don't like the idea of changing the namespace for every version. Do
> you have a real-world scenario for someone wanting to use stuff from two
> major versions of the library at once in the same binary (i.e. not in
> separate ROS nodes)? I think that the namespace should remain the same,
> while the library so version and the include directory suffix changes to
> indicate the version number. This allows multiple versions to be
> installed at once but only allows one two be used at a time, and is
> standard practice. I don't see the argument for allowing a user to use
> algorithms from PCL 1.0 and PCL 2.0 at once (in fact, I don't understand
> why you would remove algorithms from PCL at all).

I agree -- we're not going to remove algorithms (in theory). Is there a way to prevent conflicts in ROS where we could
have two packages: pcl and pcl2, with the same namespace? Remember that we export all the includes/libs automatically
which means that if a package includes both pcl and pcl2 (through some other dependencies), we'll have
pcl::PointCloud<T> from pcl and pcl::PointCloud<T> from pcl2.

If there is a way to solve this, we should drop the namespace rename. I find the namespace rename only useful for ROS
packages (Eigen vs Eigen3).


> Having ROS packages per version number seems likely to confuse users
> (and is a direct consequence of ROS not supporting package versioning, I
> guess). There should be *the* PCL package, and ROS users use whichever
> one is in their ROS distribution.

I know but until we get there, we need to compensate/find alternative solutions :)

> PCL does need to iterate fast, but this iteration is supposed to occur
> in the pre-1.0 (i.e. before the first "stable" release) releases. Let's
> not rush for the glorious tag of 1.0 before we're ready, only to have to
> introduce huge API breakage 3 months later. We can still try to keep the
> API stable before the 1.0 release (only change it in minor release
> increments, for example). The fact that there are so many users out
> there already shows that people are not that worried about using
> pre-stable software.


In theory I agree, though I have some reservations too:

  * I personally think that the open source community is striving way too hard to get a ton of versions under 1.0 and
gloriously prepare for 1.0 as if it's _the_ bomb. :) Versions should be just versions and nothing else. Sure, 1.0 is a
milestone, but I don't know why we have to have 100 versions before 1.0 :)

  * We can iterate over a library like PCL for ages... reviewing something right now, and marking it _usable_ at 1.0 is
not the worst thing.


On the other hand, I do agree with you about the huge API breakage 3 months later. But that's why we need to do the
review now, before 1.0. Also, I hope that the new notes that we added to the page clarify that we're not seeking to
break the API on purpose.



In my opinion, if we solve the namespace issue, we're solid. :)


Cheers,
Radu.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator
In reply to this post by Jack O'Quin

On 11/29/2010 06:10 PM, Jack O'Quin wrote:
> On Mon, Nov 29, 2010 at 6:46 PM, Geoffrey Biggs
> <[hidden email]>  wrote:
>
>> For versioning, I think that the wikipedia explanation leaves out one
>> important principle: if a breaking API change is made, it requires a
>> major version increment; if a new API is added, it only requires a minor
>> version increment. This means that the "stable" version of PCL can still
>> have minor version increments, so 1.0, 1.1, etc will be equivalent in
>> terms of "stable," but may contain new features that are not API-breaking.
[...]
> I agree with Geoffrey's recommendation to only change major number
> (post-1.0) for an incompatible API change. Minor numbers would change
> for a new release, which may contain extensions that are
> backwards-compatible. Micro numbers correspond to bug fixes, with no
> change to the interface.

Sounds good to me. What do we define as "extensions that are backwards-compatible" (see my previous e-mail or the text
below)? :)

"(or additionally we can consider adding minor features that do not break the API -- need a discussion about what's
considered minor)"

> Beginning with the Diamondback release, these micro updates need to be
> binary-compatible (no change to API or ABI). The idea is that a bug
> fix should not require other packages depending on the library to be
> re-released.

Agreed. That's the micro numbers, e.g. 1.0.X.

>> The fact that there are so many users out
>> there already shows that people are not that worried about using
>> pre-stable software.
>
> Some people.
>
> We decided to wait until it stabilizes before using it at UT Austin
> with undergraduate students.  Maybe next semester. PCL does look like
> it will be very useful for some of these research projects.

Glad to hear that! We'll make more slides and demos/tutorials for our class at Stanford University in the next 1-3
months, and we'll share them with you all to relieve the burden of teaching/preparing classwork. :)

Cheers,
Radu.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Jack O'Quin
On Mon, Nov 29, 2010 at 10:00 PM, Radu Bogdan Rusu
<[hidden email]> wrote:
> On 11/29/2010 06:10 PM, Jack O'Quin wrote:
>> On Mon, Nov 29, 2010 at 6:46 PM, Geoffrey Biggs
>> <[hidden email]>  wrote:

> Sounds good to me. What do we define as "extensions that are
> backwards-compatible" (see my previous e-mail or the text below)? :)
>
> "(or additionally we can consider adding minor features that do not break
> the API -- need a discussion about what's considered minor)"

It's not so much whether the new features are "minor" or
"insignificant", but that they preserve backwards API compatibility.

Simplifying slightly, that means: all existing programs that compiled
and worked with the previous minor version (e.g. 1.2) will compile and
work with the new one (e.g. 1.3).

There may be many new public or private methods, variables and
classes. Namespaces allow adding names without colliding with those in
user programs. It's OK if everything needs to be recompiled. The
memory layout and implementation may be different. Code using the new
features need not work with the older version (i.e. compatibility is
only "backwards").

There are always corner cases that have to be considered individually
on their merits, but "source compatible" is the overall goal. That
makes things easier for the users. They concentrate on solving their
own research problems, rather than tracking library changes.

>> Beginning with the Diamondback release, these micro updates need to be
>> binary-compatible (no change to API or ABI). The idea is that a bug
>> fix should not require other packages depending on the library to be
>> re-released.
>
> Agreed. That's the micro numbers, e.g. 1.0.X.

Yes, exactly.

Those kinds of maintenance bug fixes can be issued after the release
ships. Normally one would not add new features in a micro version.

Regards,
--
 joq
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator


On 11/29/2010 09:37 PM, Jack O'Quin wrote:

> On Mon, Nov 29, 2010 at 10:00 PM, Radu Bogdan Rusu
> <[hidden email]>  wrote:
>> On 11/29/2010 06:10 PM, Jack O'Quin wrote:
>>> On Mon, Nov 29, 2010 at 6:46 PM, Geoffrey Biggs
>>> <[hidden email]>    wrote:
>
>> Sounds good to me. What do we define as "extensions that are
>> backwards-compatible" (see my previous e-mail or the text below)? :)
>>
>> "(or additionally we can consider adding minor features that do not break
>> the API -- need a discussion about what's considered minor)"
>
> It's not so much whether the new features are "minor" or
> "insignificant", but that they preserve backwards API compatibility.
>
> Simplifying slightly, that means: all existing programs that compiled
> and worked with the previous minor version (e.g. 1.2) will compile and
> work with the new one (e.g. 1.3).
>
> There may be many new public or private methods, variables and
> classes. Namespaces allow adding names without colliding with those in
> user programs. It's OK if everything needs to be recompiled. The
> memory layout and implementation may be different. Code using the new
> features need not work with the older version (i.e. compatibility is
> only "backwards").


I completely agree with you and I think that's how most packages in ROS should function. Some of my colleagues
unfortunately share a different view, because they argue that:

  * if you make a piece of code X that depends on another piece of software (stable, released) libY version M.N,
  * within a distribution D, that piece of code X needs to be able to compile against all versions of libY released in
the distribution D,
  * because we want to tell people that X works against the distribution D, and not against the distribution D snapshot
updated in month MM (after the initial release), etc.


To be a bit more explicit... if you make a skeleton tracking project that depends on PCL 1.5 (arbitrarily chosen) which
has a segmentation algorithm implemented that was inexistent when PCL 1.0 got released in D-Turtle, then you require a
specific "state" of D-Turtle, which is "D-Turtle that contains PCL 1.5 in it", so if I take your skeleton tracking
project and attempt to compile it on "D-Turtle stock" (released in February or something), it will not work.

The argument is sort of valid as we attempt to do something similar to what Ubuntu does -- though they don't always
respect it themselves. A solution to this is to have backports, and other fancy things, but I'm not sure when/if we'll
have that.

One thing's certain: having a policy like this makes life easier for the maintainers of the distribution. Whether it
makes it harder for package/software developers, that's a different thing.

Cheers,
Radu.


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator
It feels silly to reply to myself I know... but did you guys read http://www.ros.org/reps/rep-0009.html ? It might be
that this solves some of the issues that we have.

If we're only supposed to have ABI compatibility for packages released in the same distribution in ROS, then this would
help.

Cheers,
Radu.


On 11/29/2010 10:44 PM, Radu Bogdan Rusu wrote:

>
>
> On 11/29/2010 09:37 PM, Jack O'Quin wrote:
>> On Mon, Nov 29, 2010 at 10:00 PM, Radu Bogdan Rusu
>> <[hidden email]> wrote:
>>> On 11/29/2010 06:10 PM, Jack O'Quin wrote:
>>>> On Mon, Nov 29, 2010 at 6:46 PM, Geoffrey Biggs
>>>> <[hidden email]> wrote:
>>
>>> Sounds good to me. What do we define as "extensions that are
>>> backwards-compatible" (see my previous e-mail or the text below)? :)
>>>
>>> "(or additionally we can consider adding minor features that do not break
>>> the API -- need a discussion about what's considered minor)"
>>
>> It's not so much whether the new features are "minor" or
>> "insignificant", but that they preserve backwards API compatibility.
>>
>> Simplifying slightly, that means: all existing programs that compiled
>> and worked with the previous minor version (e.g. 1.2) will compile and
>> work with the new one (e.g. 1.3).
>>
>> There may be many new public or private methods, variables and
>> classes. Namespaces allow adding names without colliding with those in
>> user programs. It's OK if everything needs to be recompiled. The
>> memory layout and implementation may be different. Code using the new
>> features need not work with the older version (i.e. compatibility is
>> only "backwards").
>
>
> I completely agree with you and I think that's how most packages in ROS should function. Some of my colleagues
> unfortunately share a different view, because they argue that:
>
> * if you make a piece of code X that depends on another piece of software (stable, released) libY version M.N,
> * within a distribution D, that piece of code X needs to be able to compile against all versions of libY released in the
> distribution D,
> * because we want to tell people that X works against the distribution D, and not against the distribution D snapshot
> updated in month MM (after the initial release), etc.
>
>
> To be a bit more explicit... if you make a skeleton tracking project that depends on PCL 1.5 (arbitrarily chosen) which
> has a segmentation algorithm implemented that was inexistent when PCL 1.0 got released in D-Turtle, then you require a
> specific "state" of D-Turtle, which is "D-Turtle that contains PCL 1.5 in it", so if I take your skeleton tracking
> project and attempt to compile it on "D-Turtle stock" (released in February or something), it will not work.
>
> The argument is sort of valid as we attempt to do something similar to what Ubuntu does -- though they don't always
> respect it themselves. A solution to this is to have backports, and other fancy things, but I'm not sure when/if we'll
> have that.
>
> One thing's certain: having a policy like this makes life easier for the maintainers of the distribution. Whether it
> makes it harder for package/software developers, that's a different thing.
>
> Cheers,
> Radu.
>
>
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Jack O'Quin
In reply to this post by Radu B. Rusu
On Tue, Nov 30, 2010 at 12:44 AM, Radu Bogdan Rusu
<[hidden email]> wrote:

> On 11/29/2010 09:37 PM, Jack O'Quin wrote:
>>
>> On Mon, Nov 29, 2010 at 10:00 PM, Radu Bogdan Rusu
>> <[hidden email]>  wrote:
>>>
>>> On 11/29/2010 06:10 PM, Jack O'Quin wrote:
>>>>
>>>> On Mon, Nov 29, 2010 at 6:46 PM, Geoffrey Biggs
>>>> <[hidden email]>    wrote:
>>
>>> Sounds good to me. What do we define as "extensions that are
>>> backwards-compatible" (see my previous e-mail or the text below)? :)
>>>
>>> "(or additionally we can consider adding minor features that do not break
>>> the API -- need a discussion about what's considered minor)"
>>
>> It's not so much whether the new features are "minor" or
>> "insignificant", but that they preserve backwards API compatibility.
>>
>> Simplifying slightly, that means: all existing programs that compiled
>> and worked with the previous minor version (e.g. 1.2) will compile and
>> work with the new one (e.g. 1.3).
>>
>> There may be many new public or private methods, variables and
>> classes. Namespaces allow adding names without colliding with those in
>> user programs. It's OK if everything needs to be recompiled. The
>> memory layout and implementation may be different. Code using the new
>> features need not work with the older version (i.e. compatibility is
>> only "backwards").
>
>
> I completely agree with you and I think that's how most packages in ROS
> should function. Some of my colleagues unfortunately share a different view,
> because they argue that:
>
>  * if you make a piece of code X that depends on another piece of software
> (stable, released) libY version M.N,
>  * within a distribution D, that piece of code X needs to be able to compile
> against all versions of libY released in the distribution D,
>  * because we want to tell people that X works against the distribution D,
> and not against the distribution D snapshot updated in month MM (after the
> initial release), etc.
>
>
> To be a bit more explicit... if you make a skeleton tracking project that
> depends on PCL 1.5 (arbitrarily chosen) which has a segmentation algorithm
> implemented that was inexistent when PCL 1.0 got released in D-Turtle, then
> you require a specific "state" of D-Turtle, which is "D-Turtle that contains
> PCL 1.5 in it", so if I take your skeleton tracking project and attempt to
> compile it on "D-Turtle stock" (released in February or something), it will
> not work.

That is not "backwards compatibility" as I understand the term. It
seems like a different issue related to stable and unstable
distribution versions.

When ROS 1.4 (D-turtle, stable) comes out, we hope to have a
perception_pcl stack (or whatever it's called at that point) with a
useful feature set frozen at some major.minor version API. Ideally,
that will be 1.0. People would like perception_pcl 1.0 to continue
working during the lifespan of ROS 1.4. Programs written to that API
need not change. Bug fixes (1.0.1, 1.0.2, ...) would preserve binary
compatibility if at all possible. No new features would be added to
perception_pcl 1.0.

Once ROS 1.4 ships, ROS 1.5 will begin development as the new
"unstable" distribution. There, the rules will be different. We can
release new, backwards-compatible versions with new features as 1.1,
1.2, etc. until it's time for the E-turtle feature-freeze. Then, the
newest stable version must be maintained throughout the ROS 1.6
(E-turtle) lifespan. Subsequent development can continue on 1.7
unstable after that.

> The argument is sort of valid as we attempt to do something similar to what
> Ubuntu does -- though they don't always respect it themselves. A solution to
> this is to have backports, and other fancy things, but I'm not sure when/if
> we'll have that.

A backport would correspond to releasing a new minor version (say 1.1)
after the final D-turtle ROS 1.4 release. I can see why that might be
controversial. I doubt the ROS build system even handles that
correctly. Debian and Ubuntu can, because they use version numbers as
part of the library shared object names. But, despite that, they
normally wait until the next distribution before releasing anything
more than bug fixes.

People who really want the latest features are probably willing to run
the unstable branch to get them.

> One thing's certain: having a policy like this makes life easier for the
> maintainers of the distribution. Whether it makes it harder for
> package/software developers, that's a different thing.

The main goal is making life easier for *users*. All of us who
maintain packages end up doing extra work to maintain a stable
released version in addition to our latest development trunk. That's
the price we pay for making things that are useful for lots of people.
It's generally worthwhile.

Regards,
--
 joq
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Radu B. Rusu
Administrator


On 11/30/2010 04:00 PM, Jack O'Quin wrote:
> Once ROS 1.4 ships, ROS 1.5 will begin development as the new
> "unstable" distribution. There, the rules will be different. We can
> release new, backwards-compatible versions with new features as 1.1,
> 1.2, etc. until it's time for the E-turtle feature-freeze. Then, the
> newest stable version must be maintained throughout the ROS 1.6
> (E-turtle) lifespan. Subsequent development can continue on 1.7
> unstable after that.
[...]
> People who really want the latest features are probably willing to run
> the unstable branch to get them.

Just a note here: if we tell our users...

  * "you can use PCL 1.0.x as part of D-Turtle, but you will only get bugfixes to the existing algorithms"

and

  * "you need to use unstable (E-Turtle in progress) for PCL 1.1.x (or PCL 2.x) to get any new functionality"...

This is very bad. That means that we are either telling people:

  * please jump to unstable (which means they pull in a lot of other code that is _not meant_ to be fully integrated,
work properly, etc)

or

  * use a source overlay. But this means that they need to recompile all the software in D-Turtle that uses PCL.


Neither solution is good. ROS Unstable should not be used under any circumstances (even I don't like using it a lot as
some things can be really broken -- I usually just have a tree that I test from time to time) until it's released as a
new distribution. Most of the package developers that do work on unstable don't like to support those packages until
they are released in a standard distribution.

The only solution here is to either:

  * allow new features that keep the ABI compatibility in the same PCL package version in ROS, and allow 1.Y releases
(1.0, 1.1, 1.2, etc) - I am hoping that REP 9 goes along these lines.

or

  * have two packages, one PCL stable (that only gets bugfixes) and one PCL devel (that gets new features that people
want to use) in the same distribution -- as we proposed in the roadmap.


Following the replies that Geoff and other people gave, we need to understand that there are two different categories of
users here: 1) researchers and people that do not need to build rock-solid applications that get sold to customers or
whatever, and are willing to experiment with new features (they need cool demos, they need to be on the edge of things,
write papers, etc) and 2) users who do want to build applications that work in a ROS distribution no matter what version
of the library they use.

The latter seems really ROS specific for me, and while I think we should support it (through the use of a frozen PCL
1.0.x branch), my concerns go beyond that.


PS. Sorry for the unnecessary bullet-ization :)

Cheers,
Radu.

_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

Jack O'Quin
On Tue, Nov 30, 2010 at 6:35 PM, Radu Bogdan Rusu <[hidden email]> wrote:

>
>
> On 11/30/2010 04:00 PM, Jack O'Quin wrote:
>>
>> Once ROS 1.4 ships, ROS 1.5 will begin development as the new
>> "unstable" distribution. There, the rules will be different. We can
>> release new, backwards-compatible versions with new features as 1.1,
>> 1.2, etc. until it's time for the E-turtle feature-freeze. Then, the
>> newest stable version must be maintained throughout the ROS 1.6
>> (E-turtle) lifespan. Subsequent development can continue on 1.7
>> unstable after that.
>
> [...]
>>
>> People who really want the latest features are probably willing to run
>> the unstable branch to get them.
>
> Just a note here: if we tell our users...
>
>  * "you can use PCL 1.0.x as part of D-Turtle, but you will only get
> bugfixes to the existing algorithms"
>
> and
>
>  * "you need to use unstable (E-Turtle in progress) for PCL 1.1.x (or PCL
> 2.x) to get any new functionality"...
>
> This is very bad. That means that we are either telling people:
>
>  * please jump to unstable (which means they pull in a lot of other code
> that is _not meant_ to be fully integrated, work properly, etc)
>
> or
>
>  * use a source overlay. But this means that they need to recompile all the
> software in D-Turtle that uses PCL.

or
 * unless you really need this new feature, wait for the next distribution

Naive question: does an overlay necessarily require recompiling all
the other D-turtle packages?

Seems like a reasonable alternative might be to release new PCL stack
versions that 1) work with D-turtle and 2) are backwards-compatible.
In that case, D-turtle packages might not need to be recompiled.

> Neither solution is good. ROS Unstable should not be used under any
> circumstances (even I don't like using it a lot as some things can be really
> broken -- I usually just have a tree that I test from time to time) until
> it's released as a new distribution. Most of the package developers that do
> work on unstable don't like to support those packages until they are
> released in a standard distribution.

I don't use unstable very often, either, but my experience with it to
date has been pretty good.

> The only solution here is to either:
>
>  * allow new features that keep the ABI compatibility in the same PCL
> package version in ROS, and allow 1.Y releases (1.0, 1.1, 1.2, etc) - I am
> hoping that REP 9 goes along these lines.

I don't object to REP 9 allowing such extensions, but it adds
significant complexity and does not really solve all these problems.

> or
>
>  * have two packages, one PCL stable (that only gets bugfixes) and one PCL
> devel (that gets new features that people want to use) in the same
> distribution -- as we proposed in the roadmap.

Would that require two stacks? Moving a package from one stack to
another seems relatively easy. Moving a library from one package to
another breaks both API and ABI compatibility.

> Following the replies that Geoff and other people gave, we need to
> understand that there are two different categories of users here: 1)
> researchers and people that do not need to build rock-solid applications
> that get sold to customers or whatever, and are willing to experiment with
> new features (they need cool demos, they need to be on the edge of things,
> write papers, etc) and 2) users who do want to build applications that work
> in a ROS distribution no matter what version of the library they use.
>
> The latter seems really ROS specific for me, and while I think we should
> support it (through the use of a frozen PCL 1.0.x branch), my concerns go
> beyond that.

I suspect PCL users represent a continuous spectrum, not discrete.

Some researchers need the latest and greatest PCL, others will
probably be happy with a basic set of filters that work reliably while
they focus on other goals.

People developing products, of course, generally value stability over
function. I'm not sure how many of them there are yet, but there must
be some.
--
 joq
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: pre-review

tfoote


On 11/30/2010 06:04 PM, Jack O'Quin wrote:

> On Tue, Nov 30, 2010 at 6:35 PM, Radu Bogdan Rusu<[hidden email]>  wrote:
>    
>>
>> On 11/30/2010 04:00 PM, Jack O'Quin wrote:
>>      
>>> Once ROS 1.4 ships, ROS 1.5 will begin development as the new
>>> "unstable" distribution. There, the rules will be different. We can
>>> release new, backwards-compatible versions with new features as 1.1,
>>> 1.2, etc. until it's time for the E-turtle feature-freeze. Then, the
>>> newest stable version must be maintained throughout the ROS 1.6
>>> (E-turtle) lifespan. Subsequent development can continue on 1.7
>>> unstable after that.
>>>        
>> [...]
>>      
>>> People who really want the latest features are probably willing to run
>>> the unstable branch to get them.
>>>        
>> Just a note here: if we tell our users...
>>
>>   * "you can use PCL 1.0.x as part of D-Turtle, but you will only get
>> bugfixes to the existing algorithms"
>>
>> and
>>
>>   * "you need to use unstable (E-Turtle in progress) for PCL 1.1.x (or PCL
>> 2.x) to get any new functionality"...
>>
>> This is very bad. That means that we are either telling people:
>>
>>   * please jump to unstable (which means they pull in a lot of other code
>> that is _not meant_ to be fully integrated, work properly, etc)
>>
>> or
>>
>>   * use a source overlay. But this means that they need to recompile all the
>> software in D-Turtle that uses PCL.
>>      
> or
>   * unless you really need this new feature, wait for the next distribution
>
> Naive question: does an overlay necessarily require recompiling all
> the other D-turtle packages?
>    
If the changes are ABI compatible you do not need to recompile.  Note
however ABI compatibility cannot have new features, thus negating the
value.

Tully

> Seems like a reasonable alternative might be to release new PCL stack
> versions that 1) work with D-turtle and 2) are backwards-compatible.
> In that case, D-turtle packages might not need to be recompiled.
>
>    
>> Neither solution is good. ROS Unstable should not be used under any
>> circumstances (even I don't like using it a lot as some things can be really
>> broken -- I usually just have a tree that I test from time to time) until
>> it's released as a new distribution. Most of the package developers that do
>> work on unstable don't like to support those packages until they are
>> released in a standard distribution.
>>      
> I don't use unstable very often, either, but my experience with it to
> date has been pretty good.
>
>    
>> The only solution here is to either:
>>
>>   * allow new features that keep the ABI compatibility in the same PCL
>> package version in ROS, and allow 1.Y releases (1.0, 1.1, 1.2, etc) - I am
>> hoping that REP 9 goes along these lines.
>>      
> I don't object to REP 9 allowing such extensions, but it adds
> significant complexity and does not really solve all these problems.
>
>    
>> or
>>
>>   * have two packages, one PCL stable (that only gets bugfixes) and one PCL
>> devel (that gets new features that people want to use) in the same
>> distribution -- as we proposed in the roadmap.
>>      
> Would that require two stacks? Moving a package from one stack to
> another seems relatively easy. Moving a library from one package to
> another breaks both API and ABI compatibility.
>
>    
>> Following the replies that Geoff and other people gave, we need to
>> understand that there are two different categories of users here: 1)
>> researchers and people that do not need to build rock-solid applications
>> that get sold to customers or whatever, and are willing to experiment with
>> new features (they need cool demos, they need to be on the edge of things,
>> write papers, etc) and 2) users who do want to build applications that work
>> in a ROS distribution no matter what version of the library they use.
>>
>> The latter seems really ROS specific for me, and while I think we should
>> support it (through the use of a frozen PCL 1.0.x branch), my concerns go
>> beyond that.
>>      
> I suspect PCL users represent a continuous spectrum, not discrete.
>
> Some researchers need the latest and greatest PCL, others will
> probably be happy with a basic set of filters that work reliably while
> they focus on other goals.
>
> People developing products, of course, generally value stability over
> function. I'm not sure how many of them there are yet, but there must
> be some.
>    
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
12