--onnx_export_path fails if observations are not stored in property named "obs"

[metinc]

I tried to run the Virtual Camera example and use the --onnx_export_path argument. But it fails with KeyError: 'camera_2d'. camera_2d is the name of the obs property here. In other examples where observations are stored inside a property named obs this error does not occur. I tried the RobotFPS example aswell where obs is used and it worked. But then I renamed obs to robot in get_obs() and get_obs_space() and it failed with KeyError: 'robot'.

Steps to reproduce:

1. Get latest Godot RL from pip.
2. Execute python examples/stable_baselines3_example.py --timesteps=400 --onnx_export_path=model.onnx (I had to rename export_model_as_onnx to export_ppo_model_as_onnx for this step, because this rename was not published to pip yet I think)
3. Open the Virtual Camera example in Godot and hit play.

[Ivan-267]

Hello, thanks for reporting this, I have been aware of this limitation due to the implementation, we use "obs" in all of the examples except the camera one. Also onnx inference was implemented to use 'obs'. The camera example is currently known not to be exportable with SB3 to onnx.

I will apply a fix for testing the export so it at least allows changing the key name and make a PR, however it still doesn't guarantee that the camera export will work properly with SB3. Still, it's one step closer.

I have previously tested it with Rllib: edbeeching/godot_rl_agents_examples#32, but, although it worked, even there there might be some pre-processing done as SB3 does which we don't yet do exactly the same in our onnx inference code (there's more to test and check).

Until all of this is supported, we can also use --inference with SB3 to test trained models, when experimenting with the camera sensor. In that case all of the needed processing should be applied automatically by SB3.

[Ivan-267]

I added the change, PR here: #212

This still isn't well tested (only quickly), feel free to check if it works well with a change in the obs key. It does not guarantee that the camera obs export to onnx will work however, it just adds this first step and allows you to use a different name for the key.

[metinc]

Hi, I just tested your changes. Unfortunately I still get KeyError. The error happens inside the forward_ppo() function. Somehow it complains that the key was not found inside the obs. I printed the obs inside that function and they look like this:
[tensor([[-0.7703, -0.3113, -0.7159, -0.7463, -0.0942, 0.8985, 0.0851, -0.6522, -0.8274, -0.1904, -0.9800, 0.0131, 0.3366, 0.2892, -0.6755, -0.1522, -0.0359]])]
Do we need to add the same check here that you used in export_model_as_onnx()?

[Ivan-267]

Hi, I just tested your changes. Unfortunately I still get KeyError.

Is this with camera sensor? So far I tried with vector obs only, although it was a brief test. Did you apply the same changes as in the PR usage guide? #212

Also, does it work fine with the default obs key (without adding any arguments for the key to export_model_as_onnx)? In this case it should act the same as before. If yes, does it also work if you do set the key as ['obs'], and stops working if you set it to anything different? Is it the RobotFPS example? I'd like to try to reproduce the steps/error.