Software icon indicating copy to clipboard operation
Software copied to clipboard

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

Open williamckha opened this issue 1 year ago • 9 comments

Description

  • Python updated to 3.10.0
    • Python 3.11 makes Thunderscope very slow for some reason and I can't figure out why
    • Python 3.12 is not supported by some dependencies
  • rules_python updated to 0.30.0
    • Bazel kept spewing out indecipherable errors until I upgraded rules_python. I think Python 3.10 is unsupported by our current version of rules_python
    • Replaced usages of pip_install (deprecated) with pip_parse
    • requirements.txt files are now requirements.in files that must be compiled into requirements_lock.txt files using the targets generated by the compile_pip_requirements macro. For example, to compile software/thunderscope/requirements.in you should run
      bazel run //software/thunderscope:requirements.update
      
  • Bazel updated to 5.4.0
    • rules_python only supports >= Bazel 5.4
  • Replaced Black and Autoflake with Ruff python linter and formatter
  • Removed imports from typing for collections (list, tuple, dict) -- no longer required as of Python 3.9
  • Ran fix_formatting.sh
  • Fixed fsm_diagram_generator.py so that transitions with multiple guards/action are displayed properly
  • Switch to using pyqtdarktheme in Thunderscope instead of qt-material

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

  • [ ] Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • [ ] Remove all commented out code
  • [ ] Remove extra print statements: for example, those just used for testing
  • [ ] Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

williamckha avatar Jul 26 '24 19:07 williamckha

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

itsarune avatar Jul 26 '24 20:07 itsarune

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

itsarune avatar Jul 26 '24 20:07 itsarune

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

williamckha avatar Jul 26 '24 21:07 williamckha

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).

nimazareian avatar Jul 26 '24 21:07 nimazareian

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

itsarune avatar Jul 28 '24 20:07 itsarune

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

itsarune avatar Aug 05 '24 02:08 itsarune

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

itsarune avatar Aug 05 '24 02:08 itsarune

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

williamckha avatar Aug 05 '24 04:08 williamckha

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

itsarune avatar Aug 05 '24 19:08 itsarune

pinging @itsarune for rereview

williamckha avatar Aug 19 '24 04:08 williamckha

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

Mr-Anyone avatar Aug 23 '24 20:08 Mr-Anyone

Will resolve remaining nits in a different pr

williamckha avatar Aug 23 '24 20:08 williamckha