-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add logging for h5 #119
Add logging for h5 #119
Conversation
Important notes about this PR in this issue (addresses MVP 1 ) |
sim/play.py
Outdated
velocity=env.dof_vel[robot_index, i].item(), | ||
torque=env.torques[robot_index, i].item(), | ||
) | ||
command = krec.ActuatorCommand( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to have 1to1 mapping the command information is missing here which is fundamental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means the 2D and 3D velocity command to the robot, right? yeah I dont think krec has that so it will be a design choice how we wanna save it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can't save fully proper datapoint with Krec (it should have command) then let's remove it for now.
sim/play.py
Outdated
path = ppo_runner.alg.actor_critic | ||
convert_model_to_onnx(path, ActorCfg(), save_path="policy.onnx") | ||
print("Exported policy as onnx to: ", path) | ||
# # export policy as a onnx module (used to run it on web) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why commented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its outdated / something that was changed/handled in kinfer, when I run it I get this. Maybe wesley knows more, but low priority for now imo.
loaded_dict = torch.load(path)
Exported policy as jit script to: .
Traceback (most recent call last):
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/serialization.py", line 564, in _check_seekable
f.seek(f.tell())
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/nn/modules/module.py", line 1729, in __getattr__
raise AttributeError(f"'{type(self).__name__}' object has no attribute '{name}'")
AttributeError: 'ActorCritic' object has no attribute 'seek'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "sim/play.py", line 332, in <module>
play(args)
File "sim/play.py", line 94, in play
convert_model_to_onnx(path, ActorCfg(), save_path="policy.onnx")
File "/home/kasm-user/ali_repos/sim/sim/model_export.py", line 220, in convert_model_to_onnx
all_weights = torch.load(model_path, map_location="cpu", weights_only=True)
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/serialization.py", line 1065, in load
with _open_file_like(f, 'rb') as opened_file:
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/serialization.py", line 473, in _open_file_like
return _open_buffer_reader(name_or_buffer)
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/serialization.py", line 458, in __init__
_check_seekable(buffer)
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/serialization.py", line 567, in _check_seekable
raise_err_msg(["seek", "tell"], e)
File "/opt/anaconda3/envs/ksl/lib/python3.8/site-packages/torch/serialization.py", line 560, in raise_err_msg
raise type(e)(msg)
AttributeError: 'ActorCritic' object has no attribute 'seek'. You can only torch.load from a file that is seekable. Please pre-load the data into a buffer like io.BytesIO and try to load from it instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets remove it then
|
||
if args.render: | ||
video.release() | ||
|
||
if args.log_h5: | ||
print("Saving data to " + os.path.abspath(f"data{now}.h5")) | ||
h5_file.close() | ||
# print(f"Saving HDF5 file to {h5_logger.h5_file_path}") # TODO use code from kdatagen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proper todo - remove print ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes proper TODO, will remove as soon as its done.
sim/play.py
Outdated
path = ppo_runner.alg.actor_critic | ||
convert_model_to_onnx(path, ActorCfg(), save_path="policy.onnx") | ||
print("Exported policy as onnx to: ", path) | ||
# # export policy as a onnx module (used to run it on web) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets remove it then
sim/play.py
Outdated
|
||
h5_loggers[env_idx].log_data({ | ||
"t": np.array([t * env.dt], dtype=np.float32), | ||
"2D_command": np.array(cur_obs[command_2d_start:command_2d_end], dtype=np.float32), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check if current and below are the same values?
"t": np.array([count_lowlevel * cfg.dt], dtype=np.float32),
"2D_command": np.array(
[
np.sin(2 * math.pi * count_lowlevel * cfg.dt / cfg.cycle_time),
np.cos(2 * math.pi * count_lowlevel * cfg.dt / cfg.cycle_time),
],
dtype=np.float32,
),
"3D_command": np.array([x_vel_cmd, y_vel_cmd, yaw_vel_cmd], dtype=np.float32),
```
addressed TODO 1 in the issue comment here https://github.com/kscalelabs/kmodel/issues/27#issuecomment-2513414582 |
play.py already had h5 saving, now this PR adds krec saving as well.
TODO: consolidate the saving logic into a utils file and move it to kdatagen. Right now the code exists (and has to maintained) in two places, one for mujoco and one for isaacsim.