Add Module to record FullSystem Stats - #3579
Conversation
…into sauravbanna/add_fullsystem_stats
| # From GoalieTacticConfig | ||
| INCOMING_SHOT_MIN_VELOCITY = 0.2 |
There was a problem hiding this comment.
Why not get these from the protobuf?
Andrewyx
left a comment
Andrewyx
left a comment
There was a problem hiding this comment.
I spoke to william about this today and was wondering if there was a way to implement this outisde of the GL/UI related code. Since we are not using any graphics capabilities and also making disk writes, it would probably more appropriate to have this functionality away from the UI-related layers.
From thunderscope.py we should be able to register the refresh observers for this stats module.
I moved all the stats stuff to its own module. i initially thought we wanted to visualize the stats at some point, but i think yea they should be separate from UI otherwise. i made a generic stats "holder" to hold all stats related function / classes, since i do plan on adding more to this (eg: the pass ML tuning data collection, which collects world state and saves to file similar to this stats class) i'm initializing this in thunderloop main itself, right next to fullsystem, as in my mind it's not a part of fullsystem or thunderscope, but an extra logging type module we're adding on. but lmk what you think |
sauravbanna
changed the title
Andrewyx
left a comment
Andrewyx
left a comment
There was a problem hiding this comment.
Looks good and works so far, left some comments
| @@ -0,0 +1,18 @@ | |||
| load("@thunderscope_deps//:requirements.bzl", "requirement") | |||
| py::arg("expected_kick_direction"), py::arg("min_kick_speed") = 0.5, | ||
| py::arg("min_kick_speed") = Angle::fromDegrees(20)) |
There was a problem hiding this comment.
Are these two backwards? Also, are there non constants for us to use here instead?
There was a problem hiding this comment.
fixed, but for the constants, we currently use magic numbers as default arguments in ball.h where the actual cpp function is defined. so i just did the same here, not sure which constant file to put them in if they should be constants
|
|
||
| # the python __del__ destructor isn't called reliably | ||
| # so printing this at the start instead | ||
| print(f"\n\n\n##### Writing FS Stats to {self._get_stats_file()}#####\n\n\n") |
There was a problem hiding this comment.
Not sure why there are so many new lines inserted here, but one thing to make things look a bit nicer would be to add
from rich import printOf course, this dep would need to be added in the bazel side of things too, but it allows for some nicer printing
There was a problem hiding this comment.
added, the new lines were just to space out the log from the rest of the log stream but this looks better
4 participants
Description
Adds a new layer to keep track of FullSystem Stats
These are stats that help us measure the performance of FullSystem -> how "well" it plays
Testing Done
Manual testing to
Resolved Issues
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue