Improved raycastparser.py

[alhasacademy96]

Hi peeps!

I've made major changes to the overall Raycastparser.py script.

Changes:

Documentation Added:

• Added an overall script description to explain the purpose and functionality of the script.
• Added class and method-level docstrings to provide a more detailed understanding of each component.
• Added comments to the RayCastObjects enum to describe what each enum value stands for.

Type Annotations:

• Introduced type annotations for method arguments and return types, making it easier to understand what kind of data each method expects and returns.

Dynamic Length Handling:

• Removed hard-coded array length assertions in the parse method, replacing them with dynamic checks based on the actual length of the input raycast array. This makes the code more robust to changes in the raycast array's length as opposed to having to restructure it with every new addition to the list of raycast objects (highly scalable operation).

Code Readability:

• Used more descriptive variable names for better readability.
• Added inline comments to complex operations, explaining what they are intended to do.

Detailed Changes:

Documentation:

• Added an overall script description at the top of the script.
• Included class-level docstrings for the RayCastParser class to describe its purpose.
• Each method now has a detailed docstring describing its parameters, return types, and functionality.
• Enum RayCastObjects now has comments to describe what each object type is.

Type Annotations:

• Added Python type annotations to method parameters and return types for parse, reorderRow, and prettyPrint.

Robustness:

• Replaced static array length checks with dynamic length checks based on the length of the input array in the parse method.

Code Readability:

• Introduced more meaningful variable names to make the code self-explanatory.
• Added inline comments to clarify the steps involved in complex operations.

---

Priority is HIGH for this pull so please do test as much as you can. All my unit testing produced successful returns but I haven't tested on training models such as Braitenberg.

-Ibrahim

[wschella]

Before I do a proper review, a question already: are we aware/ok that a raycaster can not detect the side of button pillar that has the button? Or is that not true?

[wschella]

I don't have time to properly test, but the code looks good. Definite improvement.

[alhasacademy96]

I don't understand the question Wout can you elaborate? The button mesh of the pillar is a child object of pillar button which is indeed detectable if that's the question

[wschella]

I mean, can a raycast agent differentiate a pillarbutton observation from the back, from a pillarbutton observation from the front (where it can bump against the button)?

[alhasacademy96]

That would require the entire pillar button mesh to be individualised which can be done. However it will increase the complexity by adding new raycast objects where currently it has just one entry. I could however add a unique tag to the actual button of the pillar button which can then be differentiated from the rest of the whole pillar button object/mesh. Did I answer your question? On 5 Sep 2023 at 12:38 +0300, Wout Schellaert @.***>, wrote:

I mean, can a raycast agent differentiate a pillarbutton observation from the back, from a pillarbutton observation from the front (where it can bump against the button)? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were assigned.Message ID: @.***>

[wschella]

That answers my question, thanks! Just something to be mindful off in documentation and explanation of the buttonpillar in the future.

[kozzy97]

Hi everyone, I've spent a few hours looking at this and I can't get any of the old raycast-based stuff to work. There are a few things that I've noticed:

• As far as I can tell, only 6 object categories are being returned in the observations, plus distance. I can't see where the other 5 object types are.
• I'm having issues with the actions now. Some of that code seems to have broken too, perhaps under the update to mlagents 0.30.0.

I'm going to keep exploring to see if I can find the issue, but I can't approve the PR till we can get the braitenberg vehicles working again.

[kozzy97]

I'm happy that everything seems to work for now. It should be noted that I haven't been able to run tests with the new raycasting, so I'm not sure whether everything is working for the new AAI objects. However, reviewing the code, it ought to be flexible enough to handle this. @alhasacademy96 let us know when you have the new experimental build ready for the button, etc. and we can try it out.

For what it's worth, I've added an updated braitenberg script that we can run to check nothing has broken. You can request outputs from it to see what the raycasts are looking like. At the moment, I'm only getting 6 object types corresponding to the old ontology. I haven't been able to get more even when building afresh from animal-ai-unity, but perhaps I'm looking in the wrong place?

I'm going to approve this PR, as I think its purpose has been fulfilled. Thanks everyone!

[kozzy97]

I'll leave it to you @alhasacademy96 to merge when you're happy (especially with the updated egg)

[wschella]

Not the first time there are some issues with the egg. As far as I know, this shouldn't even be committed to version control. This is normally generated by build tools like pip based on specifications like setup.py. So, @kozzy97, I don't think it's safe to specify dependencies in the egg, as opposed to in the setup.py.

We could merge right now, but I've already made #31 to not lose it out of sight.

[alhasacademy96]

@kozzy97 can you look into the comments myself and Wout posted? I'm planning on merging on Sunday after the final details are done