robotology / peripersonal-space

This repository deals with the implementation of peripersonal space representations on the iCub humanoid robot.
GNU General Public License v2.0
1 stars 4 forks source link

possible bug in 1D binning and enhancement to other than 20 cm range #24

Open matejhof opened 8 years ago

matejhof commented 8 years ago

@towardthesea has been visualizing the discrete representations from some of the learned .ini files (concretely this one: taxels1D_learnedAll.ini). The plots, here left_forearm.zip, indicate that there might be something wrong with the binning around 0 - there are too often empty bins. Just to give some pointers, these are IMO the relevant pieces of code:

A related question. I feel the 20 cm range for PPS may bee too restrictive for future applications (plus in terms of the brain, much bigger receptive fields are also present). To change that to 50 or 90 cm, say, what would that entail?

alecive commented 8 years ago

I'll answer your last question first: in theory, it should be pretty straightforward (you only need to change the ext parameter in the .ini file). The only problem I can foresee is that I would increase the number of bins (also that one possible from ini file), and also the time buffer in the past from which a trajectory is recorded (that one is not in the ini file I think). This means that there are more numbers to crunch, computation might be affected (even if I think that with the virtual taxels setup that should not be a big deal). Also, bigger RFs means more training.

matejhof commented 8 years ago

An additional thought on the bug suspect - could it be also related to the way the cone is set up? Very narrow around 0? But there was this extra "patch" there for the RF (where is it in the code?). How was the RF in the negative distance domain actually?

alecive commented 8 years ago

Exactly, the reason is exactly that one. On the Matlab side we fixed it by moving from a cone to a cylinder when the distance is below a certain threshold, but I think that we did not on C++. We probably found the issue while working for IROS on the matlab learning. The bigger the systematic offset is, the more evident the "hole" (because an incoming object exits the reference frame earlier in its trajectory).

alecive commented 8 years ago

I took a look at the issue. The file to look into is this one : https://github.com/alecive/pps-gitlab/blob/master/2016_PLOS_ONE/utils/train_taxelNEW.m

And specifically:

In the matlab code binLimit has been set to twice binWidthX (i.e. two bins of the 20 in the RF will have this "cylinder" feature), but this can be arbitrarily increased if needed.

matejhof commented 8 years ago

Thanks @alecive ! So:

  1. In Matlab, there is cone & cylinder? Also in the negative domain?
  2. In C++, there is only cone? And in the negative, there is a negative cone too? I guess we will need to implement the cylinder at the base of the cone in C++ too. If you had some pointers, that would be great!
alecive commented 8 years ago

In matlab there is a cone and cylinder, yes (also in the negative domain) :)

And I gave you the pointers in the previous comment :+1:

matejhof commented 8 years ago

Could you still answer point 2. please? I meant pointers to C++ code now.

alecive commented 8 years ago

Yes there is a cone always (also in the negative).

Here is where I compute if the event is inside the RF https://github.com/robotology/peripersonal-space/blob/master/lib/src/utils.cpp#L358 (it should be straightforward to extend ti by copying the matlab code).

Also, now that I look at the code I would like to move TaxelPWE, TaxelPWE1D and TaxelPWE2D into their own separate TaxelPWE.cpp file, and cleanup utils.cpp. I can do that, or you guys can (for me it's the same) :grin:

matejhof commented 8 years ago

I'm displaying a lot of detailed debugging outputs while testing something else now. I'm getting a lot of events with positive TaxelPWE:insideFoRCheck while at the same time NRM for those events varies greatly - e.g. from -0.38 to 1.15. I assumed the NRM is Euclidean distance in meters - but 1 m away can't lie inside the RF I guess? The responses computed for them all end up 0.0.

matejhof commented 8 years ago

In C++, the cylinder was implemented too (together with the cone): https://github.com/robotology/peripersonal-space/blob/pps-with-modulations/lib/src/taxelPWE.cpp#L61

matejhof commented 8 years ago

Just to get the terminology straight - what was referred to as cone above is called a spherical sector wikipedia or spherical cone wolfram (A standard cone has a flat base; instead, we take into account all points with distance <= r, therefore we have cone + spherical cap).

matejhof commented 8 years ago

Ok, so here is my proposal for new definition of maximum visual RFs of taxels. The idea is to get rid of the spherical sector and a cylinder to patch the volume around the taxel where the sector has 0 volume. The new way would be to take the spherical sector only, but shift it toward the taxel, such that it does not start at the apex. Like this: maximumrfpositive One value that I specify is the radius I want at the taxel (z = 0), which I set to 5 cm. The rest is calculated automatically with max(z) extracted from the pps settings for the bins. The same can be applied to the negative domain. With min(z)=-0.1 and max(z)=0.2 (our default settings), this gives rise to: maximumpositiveandnegative The taxel is a red dot at [0,0,0].

I think this is a more parsimonious solution than the "cone & cylinder". I'm gonna start porting this to the C++ - if you have any ideas, please comment ASAP. (especially @alecive )

alecive commented 8 years ago

Couple comments:

  1. the idea of shifting the RF closer to the taxel seems to me a good and compact solution to the problem, but the result is that:
  2. the curvature of this thing is not at all the one of a sphere centered on the taxel. This is something I already raised in the previous issue. Although minor, this means that more information from the extreme parts of the RF is taken into account, and it may not be beneficial.

Other comments:

  1. in the pictures, only 2D surfaces are drawn, whereas the RF should be composed of 3D 'voxels' rather than parts of a two-dimensional surface
  2. the number of bins in the negative domain is the same as the number of bins in the positive, and this is not the case with the current representation.

But apart from these points, feel free to go!

matejhof commented 8 years ago

Normally, I'm not the picky one, but how can I respond to this numbering? :)

  1. "Curvature". Yes, but it seems to me that having a "fatter" sphere is not a bad thing. Actually I currently already increased the aperture from 90 to 100 deg (angle from 40 to 50).
  2. Everything is 3D, also the matlab plot, it's just a projection you're seeing.
  3. Nr. bins - the pictures are just a matlab visualization of the RF - has nothing to do with bins. This will be a is-inside-RF check prior to updating the representation. Only the extremes are retrieved.
alecive commented 8 years ago

feel free to go then :+1:

matejhof commented 8 years ago

Most of this has been addressed by: https://github.com/robotology/peripersonal-space/pull/32 now.

Currently, I have increased the RFs - radius at base 7 cm (from 5) and angle 50 deg (from 40). https://github.com/robotology/peripersonal-space/blob/pps-with-modulations-devMultiEvent/lib/src/taxelPWE.cpp#L8

We still need to test the representations after learning - whether they will be smooth around 0, for example. This is dependent on #17 which is pending.

We'd also like to extend the RF to e.g. 45 cm range. Then the size/time span of the buffer needs to be tested/adapted as noted above.

matejhof commented 8 years ago

We have successfully tested <-0.05,0.45> range. Check for the bin at 0 is pending (@towardthesea).