Skip to content
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

A plethora of improvements and additions #39

Merged
merged 206 commits into from
Aug 19, 2024

Conversation

alhasacademy96
Copy link
Member

@alhasacademy96 alhasacademy96 commented Aug 16, 2024

Proposed change(s)

General:

  1. Cleaned up folder GitHub/ Bug/Issue_Template(s).
  2. New Feature: .csv File Data Logging. More on this in the docs.
  3. Added ObservationsFolder to .gititnore file (so the .csv files generated are not tracked during development).
  4. New Game Object: DataZone. More on this in the docs.
  5. Enhanced Testing Capabilities: We now have automated (native to Unity) back-end testing on key functionalities (before it was manually done via VSCode) via TestRunner. Slowly more tests will be added.
  6. Added NuGet Package Manager for adding external packages not found in Unity. Purpose of this addition is to use Moq/NSubstitute for enhanced Mocking testing in later stages.
  7. Added missing game objects from last build.
  8. Reverted to using non-static, non-singleton instances for ArenasParameter.cs (ArenasConfiguration).

Bug Fixes:

  1. Fixed incorrectly assigned game object tags, such as RipenGoal.
  2. Fixed Movable objects' collider's to be more accurate and complete.
  3. Fixed Movable objects' masses to be more accurate (objects now behave more realistically when pushed).
  4. Fixed default y-axis position to be 0 so objects spawn on the ground if y-axis value is not set.

Project Changes:

  1. Moved ProgressBar.cs script to Scripts folder for organisation.
  2. Codebase-wide script reformatting/cleanup.

YAML Syntax Changes:

  1. Reformatted YAML configuration file parameters t and pass_Mark to be timeLimit and passMark, respectively. In effect from AAI versions 4.1.0 and above.

UI Changes:

  1. Added a new UI Element: Build Version. A small text element in the bottom-right corner to display the current version of AAI (build) to the user.
  2. Removed the Yaml Information UI Element. Reason is no valid use case reported.

Useful links (Github issues, ML-Agents forum threads etc.)

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe)

Checklist

  • Added tests that prove my fix is effective or that my feature works
    • Added test script CSVLoggerTests
    • Confirmed all current tests passed with these changes
  • Updated the changelog (if applicable)
  • Updated the documentation (on animalai repo)
  • Updated the migration guide (if applicable)

Other comments

Screenshots (if any)

its foundational right now and saves per step information on agent. it needs to be able to save the .csv file outside of the root directory (currently in unity folder so not ideal to user).

Next step would be to save to desktop or better still to save the .csv in a specified location in the yaml file as a new syntax.
However, it needs to be cleaned up further.
The logs are now stored in a folder called observationLogs, in editor, play and train modes. The location of the editor/play is root directory, where in the build/training, it's currently: ""C:\Users\ia424\Desktop\WINDOWS\AnimalAI_Data\observationLogs\Observations.csv"" (on my PC).

Next would need to see if we can store the folder and .csv files one directory up or potentially in root directory. Will also need to see if there is any FPS lag as the data is being written simultaneously for obvious reasons...
for editor, in root directory of project files and;
for builds, in root directory of .exe file.

Ideally, the logic for this feature should have its own class (and will so in the future).

Tested on editor plus builds running on play and train modes. The folder and files are produced as expected.
works on manual + editor.
Couldn't test on training as mlagents error occurred today (attributeerror).

Changes:
added code to log action vector and appended to attributes list (headers in .csv file).

Made actionForward and actionRotate global variables.

Properly handled onDisable() so it disables streamIO writer correctly.

Removed dublicate logic/code for logging data to .csv
also hidden the yaml data UI - this will be used for testing purposes only.
unfreezeCountdown ---> UnfreezeCountdown
action movement and rotation
@alhasacademy96 alhasacademy96 requested review from benaslater and removed request for benaslater August 16, 2024 11:23
The objects now have increased masses to reflect their purposes.

J/L/U Blocks: 180
Light/Heavy Blocks: 220/440
The original colliders set to these objects were partially setup. They are now complete. Fixes the issues where the agent would go through the object's part where no collider was set (i.e., corners of the UBlock).
Copy link
Member Author

@alhasacademy96 alhasacademy96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went over the changes to see if anything extraordinary is present.

@alhasacademy96 alhasacademy96 marked this pull request as ready for review August 16, 2024 13:29
@alhasacademy96 alhasacademy96 removed their assignment Aug 16, 2024
Copy link
Contributor

@benaslater benaslater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some superficial comments up to around Assets/Scripts/DataZone.cs. The issue is I don't think I can follow all the changes and how they fit together, so I can't confidently review this - is there any possibility you could split this out into some more self-contained PRs? e.g. you could do one for each point under general, perhaps with the last two merged, and make additional ones for anything that doesn't fit under the heading of any of these points

.gitignore Show resolved Hide resolved
Assets/Prefabs/AAI3Arena.prefab Show resolved Hide resolved
Assets/Prefabs/Moveable/HeavyBlock.prefab Show resolved Hide resolved
Assets/Prefabs/Moveable/HeavyBlock.prefab Show resolved Hide resolved
Assets/Prefabs/Moveable/UBlock.prefab Show resolved Hide resolved
Assets/Scenes/AAI3EnvironmentManager.unity Show resolved Hide resolved
Assets/Scripts/AnimalSkinManager.cs Show resolved Hide resolved
Assets/Scripts/ArenasParametersSideChannel.cs Show resolved Hide resolved
Assets/Scripts/CollisionImpulseTracker.cs Show resolved Hide resolved
@alhasacademy96
Copy link
Member Author

I've resolved the comments but if they're still valid, reopen them @benaslater.

@alhasacademy96
Copy link
Member Author

alhasacademy96 commented Aug 16, 2024

I've added some superficial comments up to around Assets/Scripts/DataZone.cs. The issue is I don't think I can follow all the changes and how they fit together, so I can't confidently review this - is there any possibility you could split this out into some more self-contained PRs? e.g. you could do one for each point under general, perhaps with the last two merged, and make additional ones for anything that doesn't fit under the heading of any of these points

That could make things worse in my experience. I think the PR is self explanatory but also admittedly a bit disorganised. I've highlighted the important changes in the PR description and the commits themselves are informative largely. The most important changes occur in the scripts. I understand your legitimate concerns but the PR is still valid and sound. You can take your time no rush :)

Also let me know what exactly confuses you. I will be able to explain more clearly.

@alhasacademy96 alhasacademy96 dismissed benaslater’s stale review August 19, 2024 11:16

requested to be removed

@alhasacademy96 alhasacademy96 merged commit d3a4d4a into main Aug 19, 2024
@alhasacademy96 alhasacademy96 deleted the feature-cleanup-observations branch August 19, 2024 12:12
@alhasacademy96 alhasacademy96 self-assigned this Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean Up Log File Output Trigger Data "Zones"
2 participants