search

edbeeching / godot_rl_agents_examples

Example Environments for the Godot RL Agents library
MIT License
37 stars 13 forks source link

Updates virtual camera example #22

Closed Ivan-267 closed 6 months ago

Ivan-267 commented 7 months ago

Updates the camera example to work with the new changes from: https://github.com/edbeeching/godot_rl_agents/pull/158 and: https://github.com/edbeeching/godot_rl_agents_plugin/pull/31 so it can be used with CNN.

Ivan-267 commented 7 months ago

LGTM, did you train an agent? I did long ago with a MLP based agent. But it was before onnx support was added so I never saved the weights.

I'm not sure if we support this yet with our onnx export script / plugin. If not, we may need a separate set of PRs of export/plugin to get it to work fully.

Option 2: For a quick solution, we can locally modify the plugin of the env or the obs space name (I think onnx inference is currently reading obs from the "obs" key of the obs dictionary) to make it work without CNN / normalization, just to have a basic onnx that works to some degree. However, I expect it won't perform as well and may take longer to train.

Would you like me to look into the export, or should I try option 2 for this PR and we can tackle the CNN export at some future point? - I'm not yet sure that it doesn't work, but I'm just guessing we may need some additional preprocessing like sb3 does to normalize the obs, and/or other changes.

Ivan-267 commented 7 months ago

Even option two would need more work, I tried to set the dictionary key name in Godot to "obs" but there are other errors in that case, possibly due to the obs encoding.

Exporting to onnx will need a bit more investigating, keeping things as is but just switching the obs key name in the onnx export script produces some errors such as (didn't look into it in-depth yet):

\site-packages\torch\nn\modules\conv.py", line 459, in _conv_forward return F.conv2d(input, weight, bias, self.stride, RuntimeError: Input type (unsigned char) and bias type (float) should be the same

While training and inference via Python will work, we probably need to adjust more things to export properly.

Ivan-267 commented 6 months ago

@edbeeching Should we merge as is (training and Python inference will work, onnx export/inference will not), or wait for onnx export modifications?

edbeeching commented 6 months ago

@Ivan-267 , lets merge as is. We can do the onnx stuff in another PR.