RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.29k stars 1.26k forks source link

About the interconnection of system objects #1122

Closed escKeyStroke closed 8 years ago

escKeyStroke commented 9 years ago

This issue arises from an attempt to use a controller designed for plant 1 on plant 2. Plant 1 is simplified representation of the real plant which is used to make control synthesis easy, whereas plant 2 is an accurate representation of the real plant which is used to perform accurate simulations. So this is a typical control design workflow. The specific problem is, I'm failing to make a "correct connection" between the controller (designed for plant 1) and plant 2. I arrived to this diagnosis as I tried this problem in a case where plant 1 and plant 2 were exactly equal objects (but not the same object, of course), yet I couldn't make controller for plant 1 work for plant 2. Consider the following script, which is a derivative of /drake/examples/Glider/runLQR.m (and is meant to run in the same directory). Questions: Why is this not working? What change should I apply to this script to obtain the expected result? Comment: When it comes to interconnecting systems, at least in this case, the user (sure enough not only me) would prefer that it was all down to making sure that the number or inputs of one matches the number of outputs of the other one. One should not even care about whether they are row or column vectors. Thanks for your attention.

p1 = GliderPlant;
p2 = GliderPlant;

  trajs=load('glider_trajs.mat');
  xtraj=trajs.xtraj;
  utraj=trajs.utraj;

  utraj = setOutputFrame(utraj,p1.getInputFrame);
  xtraj = setOutputFrame(xtraj,p1.getStateFrame);

p1 = p1.setInputLimits(-inf,inf);
p2 = p2.setInputLimits(-inf,inf);

c = GliderLQR(p1,xtraj,utraj);
sys1=feedback(p1,c); 

c = setInputFrame (c ,p2.getOutputFrame); % Added
c = setOutputFrame(c ,p2.getInputFrame ); % Added
p2= setInputFrame (p2, c.getOutputFrame); % Added
p2= setOutputFrame(p2, c.getInputFrame ); % Added
sys2=feedback(p2,c);

[yAcc1,xAcc1,lcmlog1] = simulate(sys1,xtraj.tspan,[-3.5; 0.2; 0; 0; 7+0.2*randn(1,1); 0;0]);
[yAcc2,xAcc2,lcmlog2] = simulate(sys2,xtraj.tspan,[-3.5; 0.2; 0; 0; 7+0.2*randn(1,1); 0;0]);
y1 = ppval(yAcc1.pp,yAcc1.pp.breaks);
y2 = ppval(yAcc2.pp,yAcc2.pp.breaks);

figure;
plot(y1(1,:),y1(2,:)); hold on; plot(y2(1,:),y2(2,:),'--'); % Should coincide!
legend('p1','p2')
RussTedrake commented 9 years ago

This code runs fine for me.

I don’t think you need these lines:

p2= setInputFrame (p2, c.getOutputFrame); % Added p2= setOutputFrame(p2, c.getInputFrame ); % Added but otherwise it looks like the workflow that I would expect. You’re not the first to ask for less error checking with the input/output frames. I’m of the opinion that more error checking is good (but I’m willing to listen to feedback you and others on this). Checking the size is insufficient in many cases; for example the LQR controllers are in a particular coordinate system (linearized relative to some operating point), and the frame logic will change coordinates as necessary when you compose these controllers.

  • Russ

On Jun 19, 2015, at 10:00 AM, KeyDamo notifications@github.com wrote:

This issue arises from an attempt to use a controller designed for plant 1 on plant 2. Plant 1 is simplified representation of the real plant which is used to make control synthesis easy, whereas plant 2 is an accurate representation of the real plant which is used to perform accurate simulations. So this is a typical control design workflow. The specific problem is, I'm failing to make a "correct connection" between the controller (designed for plant 1) and plant 2. I arrived to this diagnosis as I tried this problem in a case where plant 1 and plant 2 were exactly equal objects (but not the same object, of course), yet I couldn't make controller for plant 1 work for plant 2. Consider the following script, which is a derivative of /drake/examples/Glider/runLQR.m (and is meant to run in the same directory). Questions: Why is this not working? What change should I apply to this script to obtain the expected result? Comment: When it comes to interconnecting systems, at least in this case, the user (sure enough not only me) would prefer that it was all down to making sure that the number or inputs of one matches the number of outputs of the other one. One should not even care about whether they are row or column vectors. Thanks for your attention.

p1 = GliderPlant; p2 = GliderPlant;

trajs=load('glider_trajs.mat'); xtraj=trajs.xtraj; utraj=trajs.utraj;

utraj = setOutputFrame(utraj,p1.getInputFrame); xtraj = setOutputFrame(xtraj,p1.getStateFrame);

p1 = p1.setInputLimits(-inf,inf); p2 = p2.setInputLimits(-inf,inf);

c = GliderLQR(p1,xtraj,utraj); sys1=feedback(p1,c);

c = setInputFrame (c ,p2.getOutputFrame); % Added c = setOutputFrame(c ,p2.getInputFrame ); % Added p2= setInputFrame (p2, c.getOutputFrame); % Added p2= setOutputFrame(p2, c.getInputFrame ); % Added sys2=feedback(p2,c);

[yAcc1,xAcc1,lcmlog1] = simulate(sys1,xtraj.tspan,[-3.5; 0.2; 0; 0; 7+0.2_randn(1,1); 0;0]); [yAcc2,xAcc2,lcmlog2] = simulate(sys2,xtraj.tspan,[-3.5; 0.2; 0; 0; 7+0.2_randn(1,1); 0;0]); y1 = ppval(yAcc1.pp,yAcc1.pp.breaks); y2 = ppval(yAcc2.pp,yAcc2.pp.breaks);

figure; plot(y1(1,:),y1(2,:)); hold on; plot(y2(1,:),y2(2,:),'--'); % Should coincide! legend('p1','p2') — Reply to this email directly or view it on GitHub.

escKeyStroke commented 9 years ago

This code runs, I know... but did you check the resulting plot? p1vsp2 The two lines should coincide. These should coincide because these are the trajectories of the two equal plants running with the same controller. Can you please confirm whether they coincide or not in your machine?

Later, I have some specific instances to report about this thing of the I/O reference frames. By the way, this particular issue is not about the I/O frames. I shouldn't have commented about them. There is some bug here that I have no idea where it comes from. I am not assuming it's from the I/O frames.

RussTedrake commented 9 years ago

I saw the same (i believe) when i ran it earlier. In that case, this is probably not a wiring interconnections issue but something more subtle about your resulting dynamical system.

Bug reports are appreciated; if you can be specific about the error(s) that you see and what you expected then we can help debug more efficiently.

On Jun 19, 2015, at 2:44 PM, KeyDamo notifications@github.com wrote:

This code runs, I know... but did you check the resulting plot?

The two lines should coincide. Can you please confirm whether they coincide or not in your machine? Later, I have some specific instances to report about this thing of the I/O reference frames.

— Reply to this email directly or view it on GitHub.

RussTedrake commented 9 years ago

In fact, it could be precisely the coordinate system issue that i described. The lqr controller is written in one coordinate system, and your plane in another. Your added lines are potentially removing a necessary change of coordinates.

If you instead set the input/output of p2 to match the input/output of p1 (not the output/input of c) that might work.

(Can't test it myself now; reading from my phone)

On Jun 19, 2015, at 5:45 PM, Russ Tedrake russt@csail.mit.edu wrote:

I saw the same (i believe) when i ran it earlier. In that case, this is probably not a wiring interconnections issue but something more subtle about your resulting dynamical system.

Bug reports are appreciated; if you can be specific about the error(s) that you see and what you expected then we can help debug more efficiently.

On Jun 19, 2015, at 2:44 PM, KeyDamo notifications@github.com wrote:

This code runs, I know... but did you check the resulting plot?

The two lines should coincide. Can you please confirm whether they coincide or not in your machine? Later, I have some specific instances to report about this thing of the I/O reference frames.

— Reply to this email directly or view it on GitHub.

RussTedrake commented 9 years ago

Reading your original post more carefully now, i'm pretty sure that will fix it

escKeyStroke commented 9 years ago

Right! that was the solution. Thank you very much. I have not grasped how to work with these coordinate frames. But I will try to make a more precise post about it later. I take the opportunity to congratulate you and your team on the DARPA contest (I learned that it was just finished from reading here a recent issue). Hope to see you closer to 1st place next time!.

escKeyStroke commented 9 years ago

You know, it is not intuitive that I have to set the input/output of p2 to match the input/output of p1 considering that they are equal... I thought this was a good occasion to say it.

RussTedrake commented 9 years ago

Do getInputFrame and getOutputFrame for the systems and the controller at the console so that you can see the output

On Jun 19, 2015, at 6:24 PM, KeyDamo notifications@github.com wrote:

You know, it is not intuitive that I have to make set the input/output of p2 to match the input/output of p1 considering that they are equal... I thought this is a good occasion to say it.

— Reply to this email directly or view it on GitHub.

escKeyStroke commented 9 years ago

I had to add p2= setInputFrame (p2, p1.getInputFrame); to make my script work. However, I get the following in console right before executing this line:

>> p2.getInputFrame()
ans = 
Coordinate Frame: GliderPlantInput (1 elements)
  u1

>> p1.getInputFrame()
ans = 
Coordinate Frame: GliderPlantInput (1 elements)
  u1

It's the same output for both! yet isequal(p1.getInputFrame(),p2.getInputFrame()) returns 0. I think this can be improved from the user experience point of view. So I have to sit down and dig into Drake to understand what's going on. (Regarding frames, I've tried to understand the code a couple of times but evidently I have to be more serious next time).

escKeyStroke commented 9 years ago

Roundup

Having taken the time to understand the CoordinateFrame class, here I would like to roundup the issue, hopefully with some useful feedback.

Coordinate frame objects are a great idea

Some time ago I worked in aerospace navigation problems packed with so many different reference frames and it made everything so unwieldy at times, like when one wanted to manipulate data for visualization or when one connected new sensors. Having a framework that allows to introduce methods that automatize the creation and transformation of coordinates is really great.

Having the plant changed without the user's notice

The syntactical fashion of the command:

[ltvsys,Vtraj] = tvlqr(plant,xtraj,utraj,Q,R,Qf,options,p2);

whereby I synthesize the controller from the plant, suggests me that the plant remains unchanged. However, what really happens is that tvlqr changes the reference frame of my plant without any notice. This happens in line 55 of tvlqr.m:

plant.getStateFrame.addTransform(AffineTransform(plant.getStateFrame,iframe,eye(nX),-xtraj));

So the plant is not equal before and after the execution of tvlqr.
If I take the code of someone else (tvlqr in this case), I should be able to incorporate it in my code without fear that I have my objects (the plant.state_frame property in this case) being modified without notice. Otherwise, one reads code as the script of the first post and one is not able to tell how objects are changed line after line. This is dangerous because this opens up the possibility of injecting stuff into the objects and create that kind of bugs that don't crash you program but cause it to give you results that look right but are not. (You know this because this is basic OOP stuff, so let's say I fail to understand why you decided to make Drake this way). In this particular case, I would argue that methods like setStateFrame or addTransform should be of restricted access (I'm aware that probably this entails a big number of changes). tvlqr should be capable of returning the controller with a reference frame ready to be interconnected with the plant without having to change the plant.

Information about CoordinateFrame objects

I find it uncomfortable that I cannot query the differences between coordinateFrame objects that are not equal (under the isequal command criteria). I think that the (overloaded) disp method should present to the programmer all the info they contain so that one is able to tell whatever it is that makes them different. That would be a help in debugging. Or perhaps you don't want to show the differences because you don't want people custom crafting CoordinateFrame instances manually, and you want to enforce their creation within specific functions (like tvlqr) and copying them using the get/set methods. In such case, I would appreciate some form of identifier that told me, say, where a frame was created.

Documentation

I am just guessing the purposes of the architecture of choice for the CoordinateFrame class. I think it would be very nice if they were documented, specially in that document "Conceptual Overview" (in the wiki), section 2.2. That would make it easier for me to make contributions. This applies to other aspects of Drake as well.

I apologize for the length and thanks!

RussTedrake commented 8 years ago

The coordinate frame logic is being revised (and better documented) in the C++ version of the API; this will trickle back up to matlab. I've tried to capture your other comments in #1525. Thanks!