[Feature]: Add CPU multiprocessing for Panda Motionplanner's Trajecto… #611
Conversation
…ry Generation This is an elementary method that lets each process in `num_procs` to generate `num_traj // num_procs` trajectory.
…anner's CPU multiprocessing
|
I understand the reasoning somewhat, but the solver is dependent on the task. The PandArmmotionPlanningSolver is meant to a general class for the panda arm for MP problems. |
|
oh I see the bug, its the merge_trajectory.py merge_h5 code, it doesn't add those in. I think a fix would be to just copy any of the keys in the first json that aren't the keys being merged in the merge_h5 code. and if there is different data in the other parts of the json just log a warning that this key is different between the different jsons and we took the first one. |
|
Thanks for the notes! Would replay trajectory need these information too? If so, I can directly modify merge_h5 to include "source_type" and "source_desc" Regarding PandArmmotionPlanningSolver, would it make sense if child classes are created for each task? |
|
Child subclasses is another approach, either a function that calls the APIs of the solver or implements the solver. I personally don't see a good reason to have to use subclasses though in this case if one is only overriding one function. |
|
Thank you, I believe I misunderstood the concept of a vectorized environment. Thought since each process in the subenvironment would be parallelized, if we put the class inside, it would be parallelized. Will work on the fix for merge_h5 according to #611 (comment) |
… to merge_trajectories, and add docstring - Fixed bug to ensure the first value of keys (except "episodes") is used. - Renamed merge_h5 to merge_trajectories for more accurate naming. - Added docstring for better documentation.
|
|
||
| # Ignore commit info |
There was a problem hiding this comment.
Commit info is now preserved in the trajectory data because I thought it could be useful when the version of trajectory generation code is different (e.g., if someone made changes on panda motionplanner).
| cnt += 1 | ||
|
|
||
| h5_file.close() | ||
| logger.info(f"Merging{traj_path}") |
There was a problem hiding this comment.
Made this logger.info from print. Is there a recommended practice on when to use logger.info and when to use print?
There was a problem hiding this comment.
better to use logger, thanks for using the right tool here.
| cnt += 1 | ||
|
|
||
| h5_file.close() | ||
| logger.info(f"Merging{traj_path}") |
There was a problem hiding this comment.
better to use logger, thanks for using the right tool here.
Uses an elementary method that lets each process in
num_procsto generatenum_traj // num_procstrajectory.Discussion on next steps:
solvemethods defined for different tasks. For example,solveinpick_cube.py. And it seems to be using thePandaArmMotionPlanningSolverenvironment when solving. From my understanding, it might help ifsolvemethod is being moved toPandaArmMotionPlanningSolverandPandaArmMotionPlanningSolveris wrapped as a vectorized environment for multiprocessing. Would be greatly appreciated to know if this rationale sounds correct, thank you.