From: SourceForge.net <noreply@so...>  20100817 07:53:21

Patches item #3046846, was opened at 20100817 09:53 Message generated for change (Tracker Item Submitted) made by torian You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=433166&aid=3046846&group_id=42445 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Player Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Thimo Langbehn (torian) Assigned to: Brian Gerkey (gerkey) Summary: Corrects localize interface to full covariance matrix Initial Comment: This patchset changes a part of the localization interface of player. The array storing the covariance matrix in player_localize_hypoth and player_localize_set_pose is extended to 6 elements, and several modules are adapted to that modification. Rationale  Previously, only the main diagonal of the covariance matrix could be stored within the cov array, meaning only the variances of x,y and the yaw angle were transported. Since the covariance matrix for 3dimensions is a symmetric 3*3 matrix, either 9 (the full matrix), or 6 (without redundant values) are necessary to transmit it. A full covariance matrix: cov(x,x) cov(x,y) cov(x,a) Further holds: cov(u,u) = var(u) cov(y,x) cov(y,y) cov(y,a) cov(u,v) = cov(v,u) cov(a,x) cov(a,y) cov(a,a) Full Matrix (9 elements)  This would have the advantage of better readability since the matrix could be represented (meaning full) with a doubleindexed array (i.e. cov[3][3]) and its elements could be accessed in a natural way (without the need for element descriptions). Furthermore this would make conversion to other matrixtypes easier (usage of loops). However, this approach is not backwards compatible since the contents of the first three elements (previously variance) would change. And three of those values are redundant and may invite errors or unexpected behavior if set to values distinct from their pair. They furthermore enlarge the memory and transmission size requirements. Half Matrix (6 elements)  By mapping all distinct values of the covariance matrix to a 6 element array, the contents of the first three elements can remain unchanged and therefore prevent breaking compatibility. This would look like: [ cov(x,x), cov(y,y), cov(a,a), cov(x,y), cov(y,a), cov(x,a) ] Only the combination of unchanged (old) data providers with new consumers could create a problem, but since the structures are mostly initialized with 0 by size, the probability of this happening is very small, and currently there is no consumer that would break due to arbitrary covariance values. This approach ensures the symmetric quality of the covariance matrix, too (had to use incorrectly). On the other hand, the mapping of the fields to their contents is non intuitive and requires the consultation of documentation for every access or conversion operation. Additional Half Matrix (+3 elements)  The variant of using an additional 3 element array and let the existing cov[3] array remain unchanged is very similar to this 6 field extension, including the downsides. The new field is even more likely to me uninitialized since a memset on cov by size would not include it. Choice in this patchset  This patchset provides changes towards the 6 element version based on the assumption that the documentation requirement (if the documentation exists) is a lesser evil and providing a hard to misuse interface is desirable. Patches (trunk rev.8857)  localize_covariancematrix.patch Extends the covariance matrix of the localize interface (025_localize.def) to 6. This is mandatory for all the other patches libplayerc_covariancematrix.patch Adapts the libplayerc client library libplayerc++_covariancematrix.patch Adapts the libplayerc++ client library This indirectly adds support for the covariance matrix to playerprint, too (operator<<). logdrivers_covariancematrix.patch Adapts the writelog and readlog drivers to the extended array size. amcl_covariancematrix.patch This enables to access (read and write) the full internal covariance matrix of the amcl driver, which was previously only partially (only variances) available. fakelocalize does not need to be patched for this interface change.  You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=433166&aid=3046846&group_id=42445 