Fix inference when action names are not alphabetic #159
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes onnx inference when the action names are not sorted, together with this PR on the plugin side:
edbeeching/godot_rl_agents_plugin#27
Related issue: edbeeching/godot_rl_agents_plugin#25
One way to test this is to try e.g. current version of FlyBy (may need to update plugin before testing). For me, when I try the onnx file, the result doesn't work unless I swap the order of the two actions.
After the changes, here is a quick test with the onnx working properly after retraining the agent for ~50_000 steps using the SB3 example:
flyby_onnx_test.mp4
(The extra actions and unnecessary dimensions were added just for debugging)
As for what causes the issue, during conversion to
spaces.Dict
, the items of the dictionary were getting sorted while converted to OrderedDict internally:godot_rl_agents/godot_rl/core/godot_env.py
Line 332 in ac7241b
The is fine for training and Python inference since it sends a dictionary containing action names and values back to Godot, but during onnx inference the only information about action order available is from the action space as defined in gdscript. In envs where that was already sorted, there weren't any issues with onnx, but as I tried training an env with action names ordered differently, I noticed the onnx wasn't working correctly.
This should fix those cases, changing to OrderedDict seems to remove the conversion and sorting, so the order sent from Godot is preserved and should be what onnx inference expects.