search

huawei-noah / SMARTS

Scalable Multi-Agent RL Training School for Autonomous Driving
MIT License
908 stars 184 forks source link

Review specificity of `Exception` catches within SMARTS #1158

Open Gamenot opened 2 years ago

Gamenot commented 2 years ago

Is it a problem? Please describe. Exception catching has smothered issues within the project.

Describe the solution you'd like A review of the smarts directory code for all instances of Exception catches to ensure that exception catches response is specific to the exception that has been thrown.

Additional context https://github.com/huawei-noah/SMARTS/pull/1156

PracticalMetal commented 2 years ago

Hey, I would like to work on this, can you suggest me ways to get started?

Gamenot commented 2 years ago

Hello @PracticalMetal , thank you for reaching out and I am happy to hear that you wish to help out.

The problem

The main intention of this problem is to ensure that exception catches are not accidentally hiding issues within the smarts directory code. The need here is to find cases where we are catching the generic Exception, or where we are catching a specific exception type and throwing a generic Exception.

Helpful information

Over today I will be putting up a number of other issues if you wish to pick one of them.

We have a guide for setting up SMARTS here: https://github.com/huawei-noah/SMARTS/tree/develop#setup We have some guidelines here for contributing: https://github.com/huawei-noah/SMARTS/blob/develop/CONTRIBUTING.md

I also believe the following information is relevant: 1) We currently support python 3.7 and 3.8 officially. You will get varying millage out of other versions of python. 2) It is possible upon following the setup guide that the test_smarts_doesnt_leak_tasks_after_reset will still fail without giving a proper explanation why. This is a known issue here: https://github.com/huawei-noah/SMARTS/issues/1154. Even if this specific case fails you can still use the parts of SMARTS that do not require rendering. 3) You will want to make a fork of the SMARTS repository so you can open up a pull request against it. 4) Work towards the release is merged into the develop branch, documentation should be read from the develop branch version 5) Right now we are working towards a 0.5 release so changes are allowed to break the currently existing public interface

If you have any other questions feel free to ask.