RFGPS

[wmontgomery4]

Here's the commit for RFGPS, it's pretty large but here's the main takeaways:

• Restructure algorithm.py and algorithm_mdgps.py to allow for fitting dynamics/policies based on cluster indices rather than condition (I couldn't quite move this all to algorithm_mdgps.py unfortunately)
• Change CostState to only take a single data_type, rather than a dictionary, and also change the cost to be wp*(A.dot(x) - target). No one seems to really be using cost_state, and this is a more effective use of it in my opinion, but I'd be happy to change this back (although it will require some other changes since we need the A.dot(x) stuff to make it easy to get the difference between the end effector and target).

The one remaining issue is how to initialize the EM. Right now it starts with Kmeans, but I have some code which starts it from random points instead (commented out, near the end of algorithm_mdgps.py in the _cluster_traj_em method).

[anuragajay]

@cbfinn The code looks correct to me. Should I also make the other changes you suggested?

[wmontgomery4]

Don't worry, I'll handle those other changes Anurag. Thanks for the comments Chelsea!

[wmontgomery4]

@cbfinn Now that the PIGPS stuff is merged, it's going to be kind of hard to merge this stuff in a way that isn't overly complicated. I think it would make sense to refactor the current codebase first, in a way that allows

[wmontgomery4]

Whoops, accidentally hit 'close and comment'. I was going to send this in the email thread instead, but I'll just say it here: I think it would make sense to focus on refactoring the current codebase before merging this, so that the rfgps code can be written more succinctly. I've got a few ideas about how that should go:

• Move as much as possible to the base Algorithm class, or to algorithm_utils, so that each algorithm can be written nearly as tersely as the PI2 algorithms are right now. I think that pretty much everything but the __init__, iteration and _compute_costs functions could be moved out (and even the _compute_costs functions could be rewritten to just take some library calls). In general, I want to move away from using functions which take in the algorithm object, and instead take in the necessary parts of the algorithm, as this should make it easier to include RFGPS, and will make for better parallelism as well.
• Keep MDGPS/PIGPS separate, and probably make RFGPS a separate algorithm as well. I think the idea of merging the MDGPS/PIGPS is nice, but there are enough differences that it will be kind of challenging to maintain that structure (the traj_opt module isn't totally replaceable, since it affects other things like whether or not we need to fit dynamics). Also, if we abstract things out of those files, writing an individual algorithm file should be pretty short.

I've got my quals exam in ~10 days, so I'm going to put this stuff at a lower priority right now, and make a branch off this for the RWR stuff. In the meantime this branch can serve as the public implementation of rfgps. Maybe we should have a refactoring meeting sometime soon to talk about this more?

[EdsterG]

I don't think we should change CostState to only take a single data_type. I found the dictionary of data_type's to be very useful. It may be best to make a new cost function CostStateWeighted?

[wmontgomery4]

That's a fair point, and I agree that changing the API at this point is unnecessarily confusing. The main reason I wanted to do it was to add the A parameter, as this allows you get arbitrary linear functions of the state (i.e. the distance between the end effector and target). I think that cost_linwp does something similar on one of the other branches though, I'll try to just make it look like that.

[cbfinn]

Having a refactoring meeting sounds like a good idea. I'm pretty busy with ICLR right now (deadline in <7 days), so let's figure it out after your quals.

I agree with your two main points!