mhoward3210 / ccl

GNU General Public License v2.0
3 stars 4 forks source link

model_lambda_ccl bug when using kuka wiping data set and regressors #4

Open joaomoura24 opened 6 years ago

joaomoura24 commented 6 years ago

When trying to compare the estimated projection matrix using our latest method (implemented in def_constraint_estimator.m) and the method implemented in model_lambda_ccl.m I am getting a bug concerning the dimensions of matrix multiplication:

Error in learn_lambda_ccl (line 60) Vn(:,n) = J(X(:,n)) * Un(:,n) ;

The idea is that this 2 methods for constraint estimation should interchange because, at least according documentation, both estimate the null space projection given a constraint modelled as a linear combination of regressors, and provided the regressors. No specific conditions on these regressors are advertised.

When trying to fix this, it generates an error regarding dimensions elsewhere and I couldn't track what exactly is causing this. I don't know if this is due to:

The file demo_N_estimate_comp.m in the demos folder runs the set up and the 2 methods for the a synthetic data sets of the kuka arm performing circular motions.

To generate this data set we have to run the file generate_data_kuka_wiping.m (this requires the installation of the MatLab Peter Corke Robotic toolbox through http://petercorke.com/wordpress/toolboxes/robotics-toolbox) in the data_generation folder.

I would greatly appreciate some help on trying to figure out the error source.

My intension would be from now on to start interchanging our methods such constraint and policy estimation and data sets to make sure the code becomes as modular as possible and also to make us understand what are the components of code that are basically doing the same thing (and should be merged) and the components that are different - and how can we compare them. Kind regards,

Joao Moura

zhaoyuchen100 commented 6 years ago

Hi Joao Thanks for your reply. I agree with your intention to make the code as modular as possible. Regarding the bug, it is a little abstract to trace it down from my side, but I believe there is nothing wrong with the learn_lambda_ccl code itself. Might be the data prepared is not ok. What's your suggestion to merge the functions? Best, Yuchen

发自我的 iPhone

在 2017年11月11日,下午12:08,joaomoura24 notifications@github.com 写道:

When trying to compare the estimated projection matrix using our latest method (implemented in def_constraint_estimator.m) and the method implemented in model_lambda_ccl.m I am getting a bug concerning the dimensions of matrix multiplication:

Error in learn_lambda_ccl (line 60) Vn(:,n) = J(X(:,n)) * Un(:,n) ;

The idea is that this 2 methods for constraint estimation should interchange because, at least according documentation, both estimate the null space projection given a constraint modelled as a linear combination of regressors, and provided the regressors. No specific conditions on these regressors are advertised.

When trying to fix this, it generates an error regarding dimensions elsewhere and I couldn't track what exactly is causing this. I don't know if this is due to:

wrong set up of the settings structure; bug in the code; or some assumption on the regressors that I did't realise. The file demo_N_estimate_comp.m in the demos folder runs the set up and the 2 methods for the a synthetic data sets of the kuka arm performing circular motions.

To generate this data set we have to run the file generate_data_kuka_wiping.m (this requires the installation of the MatLab Peter Corke Robotic toolbox through http://petercorke.com/wordpress/toolboxes/robotics-toolbox) in the data_generation folder.

I would greatly appreciate some help on trying to figure out the error source.

My intension would be from now on to start interchanging our methods such constraint and policy estimation and data sets to make sure the code becomes as modular as possible and also to make us understand what are the components of code that are basically doing the same thing (and should be merged) and the components that are different - and how can we compare them. Kind regards,

Joao Moura

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

joaomoura24 commented 6 years ago

Hi Yuchen,

I think that the data is correct , but not really sure about if I am defining well the input argument settings: https://github.com/mhoward3210/ccl/blob/fea66fd45fe2c45fae186a304277a420f5455215/Functions/demos/demo_N_estimate_comp.m#L79 or even if there are some assumption on the dimensionality of the regressors. I would suggest to leave this issue open until we have a bit more time to debug it properly.

Regarding on how to merge this functions, I think we don't need to integrate everything just now, but rather keep working on it over time and slowly improve code, documentation, etc.

My idea is that once we define a project to work on, instead of working in separate code bases we always use this repository and then go about integrating our functions over time, by trying to use existent functions, adding features, correcting bugs and adding documentation, and if there there are something we are not able to solve straight away, we open an issue so we can discuss and solve it together.

But we can talk a bit more when we have a meeting on possible projects. I have been finishing another work, so I haven't send any suggestions yet. But I will next week.

Regarding this issue in particular, I will try to use the data from your examples in our method to check if there is major differences in the way we are setting up our problems. This will help us understand better how each code base is organised. Best regards,

Joao

zhaoyuchen100 commented 6 years ago

Hi Joao Good to hear from you. Yes, I agree with the suggestion that debugging the code while working on a specific project. If you have got any problem when using the original dataset, please feel free to share with me, I believe that is a good way to deal with the bug. You can suggest a time next week, so that we can probably chat on the ideas. Best regards, yuchen

On 14 November 2017 at 11:13, joaomoura24 notifications@github.com wrote:

Hi Yuchen,

I think that the data is correct , but not really sure about if I am defining well the input argument settings: https://github.com/ mhoward3210/ccl/blob/fea66fd45fe2c45fae186a304277a4 20f5455215/Functions/demos/demo_N_estimate_comp.m#L79 http://url or even if there are some assumption on the dimensionality of the regressors. I would suggest to leave this issue open until we have a bit more time to debug it properly.

Regarding on how to merge this functions, I think we don't need to integrate everything just now, but rather keep working on it over time and slowly improve code, documentation, etc.

My idea is that once we define a project to work on, instead of working in separate code bases we always use this repository and then go about integrating our functions over time, by trying to use existent functions, adding features, correcting bugs and adding documentation, and if there there are something we are not able to solve straight away, we open an issue so we can discuss and solve it together.

But we can talk a bit more when we have a meeting on possible projects. I have been finishing another work, so I haven't send any suggestions yet. But I will next week.

Regarding this issue in particular, I will try to use the data from your examples in our method to check if there is major differences in the way we are setting up our problems. This will help us understand better how each code base is organised. Best regards,

Joao

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mhoward3210/ccl/issues/4#issuecomment-344226525, or mute the thread https://github.com/notifications/unsubscribe-auth/AH_Pbe7_yLlOWSGsd1ZhEeYiDCUPP6zYks5s2XX2gaJpZM4QaeC9 .