UBC-Thunderbots / Software

Robot Soccer Playing AI
http://www.ubcthunderbots.ca
GNU Lesser General Public License v3.0
53 stars 110 forks source link

Update to Python 3.10, Bazel 5.4 and replace Black/Autoflake with Ruff #3292

Closed williamckha closed 1 month ago

williamckha commented 2 months ago

Description

Testing Done

Resolved Issues

Resolves #3175 Resolves #3166 Resolves #3251

Length Justification and Key Files to Review

This PR is very long because the new ruff linter/formatter updated a lot of files (removed unused imports + also did docstring formatting)

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

itsarune commented 2 months ago

Have you cross-checked Ubuntu 20 btw? I believe python3.10 is outside of what the Ubuntu 20 package repository provides

itsarune commented 2 months ago

We also need to check if this is backwards compatible with the Jetson Nanos, those run Ubuntu 18 and almost certainly don't have python3.10 in the package universe

williamckha commented 2 months ago

Have you cross-checked Ubuntu 20 btw? I believe python3.10 is outside of what the Ubuntu 20 package repository provides

I'm on Ubuntu 22 so haven't checked, but CI is looking ok. We install python from the deadsnakes ppa so we should be good

We also need to check if this is backwards compatible with the Jetson Nanos, those run Ubuntu 18 and almost certainly don't have python3.10 in the package universe

I think the nanos will stay at python 3.8 for now. I don't even think we use python on the nanos now that the onboard display is gone

nimazareian commented 2 months ago

The likely reason behind why 3.11 significantly slowed down Thunderscope is the change in Protobuf's backend to be Python instead of cpp (cpp is deprecated in new protobuf versions). Have a look here for more information on newer (and supposedly faster) protobuf backend in C: https://github.com/protocolbuffers/protobuf/blob/main/python%2FREADME.md

You can check that this is the use by printing the protobuf backend being being used in Thunderscope.

I believe our Protoc is too old for upb, so you will likely have to upgrage protobuf as well if you want to support newer Python. Worth noting that upb may break our pybinded types that have Protobuf arguments/return values (e.g. UDP sender/listeners and conversion functions).

itsarune commented 2 months ago

consider making bazel run //software/thunderscope:requirements.update a part of precommit

itsarune commented 2 months ago

There's a couple of nits but those could be addressed later

itsarune commented 2 months ago

Commenting out qdarktheme didn't help, so I'm not sure what could be causing this

williamckha commented 2 months ago

I was playing around with Thunderscope and it seems a bit weird when running pytests.

./tbots.py run goalie_tactic_test -t runs one or two tests with Thunderscope but then it seems to hang.

Doesn't hang for me, though FPS is pretty low. Not sure why since AI vs AI runs perfectly fine

itsarune commented 2 months ago

Yeah not sure. Let's see if it happens for other people.

williamckha commented 1 month ago

pinging @itsarune for rereview

Mr-Anyone commented 1 month ago

Overall, LGTM! It runs without problem.

The performance seems to be about the same

Old versions:

20.462095737457275 ms 
48.8708494394067 fps

New version:

21.095266342163087 ms
47.40399973056046 fps

Measured in the following two branches (with autoref on): python 3.8 branch, python 3.10 branch

williamckha commented 1 month ago

Will resolve remaining nits in a different pr