praveen-palanisamy/macad-gym

Support the library

johnMinelli opened this issue · 3 comments

Hi, I'm using the library for my thesis project and I'm interested in supporting/contribuiting to that since I think having a simpler interface for CARLA is great. I already solved some minor bugs and some TODOs in multi_env, I'll fill a PR with the changes.
However, (probably it's my fault since I don't have a complete picture of the library aim and previous versions of CARLA) I see a lack of structure and some classes are not even used. Can I ask for some clarifications about some files in particular and directory structure? Are you still able to follow the project (at least as advisor if not in coding)?

Thank you for your work

Hi @johnMinelli ,
It's nice to hear from you about this library's use in your thesis. I appreciate your interest in contributing to improve/enhance the library. It's good to see your initial contributions via your PRs!
Yes, feel free to ask questions and I'll be glad to answer them and help where I can.

Also, you can find the links to download previous release versions of CARLA here.

Great 👍
Sincerely i'm struggling a bit with the structure of the library. Maybe refactoring/reordering the files would be a good thing, if you could maybe take a look at it otherwise i'll come up with something but I don't want to disrupt your codebase.
(I'll paste down here the current situtation to open a discussion).

carla
PythonAPI

  • config.json
  • multi_env.py
  • reward.py
  • scenarios.py
  • spawn_control.py
  • Transform.py

core

  • controllers
    • keyboard_control.py
    • traffic.py
    • traffic_lights.py
  • maps
    • nav_utils.py
    • nodeid_coord_map.py
  • sensors
    • camera_manager.py
    • derived_sensors.py
    • hud.py
    • utils.py
  • utils
    • map_explore.py
      viz
  • render.py

multi_actor_env.py

Also some classes have not a real use in the code (maybe are part of a bigger plan not realized 🙂 but i'm here to help). Here as follow:

  • 3 *_old methods in nac_utils.py
  • The whole HUD i don't this is really used, but i think it can be integrated better
  • derived_sensor.py maybe can have a more explicative name or split in two files
  • get_transform_from_nearest_way_point function in sensors/utils.py seems a bit out of place in addition of not being used
  • map_explore.py seems to be old code or unfinished
  • spawn_control.py and Transform.py
  • could you please also explain me what is the role of multi_actor_env.py? It should be an example of env?

An additional point of discussion is about the will of ease the proccess of extension of an environment and modify constant values (most of them are thrown in the multi_env.py file). As you can see in #68 I moved the Reward class in the env init and the scenarios have been wrapped in a class. I'm not totally satisfied with the Scenarios maybe you have a suggestion, or you prefer to move toward the adoption of OpenScenario as CARLA.

As a final note, what is your take on library versioning? As you may want to keep most of the functionality frozen for this reason.

Cheers

Hey @johnMinelli !
Thank you for elaborating and starting the discussion.
I agree that there are quite a few clean-ups of unused code that can be done on this code base. Yes, they were placeholders for a larger plan but that wasn't executed so they are unused at the moment. I am glad you are here to help improve it!

I would prefer prioritizing updates in this order:

  1. Bug-fixes
  2. Support for CARLA 0.9.13+
  3. Optimizations, feature enhancements
  4. Code cleanup

An additional point of discussion is about the will of ease the proccess of extension of an environment and modify constant values (most of them are thrown in the multi_env.py file)

With the current design, any extension/modification of an existing environment is considered to be a new environment. This allows reproducibility and benchmarks against particular environment version.
A slightly-modified version of an existing environment can have the same environment name with a new version number per the Gym environment naming convention. For example, an extended/modified version of HomoNcomIndePOIntrxMASS3CTWN3-v0 env can be named with v1 suffix: HomoNcomIndePOIntrxMASS3CTWN3-v1.
Here's a brief wiki page on how to add a new environment which includes modifying existing environments.

I'm not totally satisfied with the Scenarios maybe you have a suggestion, or you prefer to move toward the adoption of OpenScenario as CARLA.

Yes, integrating OpenScenario will be a good feature enhancement. It will be even better to directly integrate CARLA's scenario_runner that already builds on top of OpenScenario.

what is your take on library versioning? As you may want to keep most of the functionality frozen for this reason

I would follow the semantic versioning and update the library version on PyPi continuously with a release. Yes, for reproducibility with the original paper, that particular version is frozen as one of the release versions here