Added development container for replicability

[YevgeniyEngineer]

Hi, I would like to check performance of your code.

I was able to make it build by wrapping it in Dev container in VSCode and using Dockerfile to pull necessary build dependencies.

I reorganised the folder to follow ROS formatting guidelines.

I have experienced two problems along the way, and I cannot seem to fix them:

In groundgrid.rviz there are several plugins that I am not sure where to get from, so I had to comment some lines:

  # - Class: autonomos_rviz_bag_control/BagControl
  #   Name: BagControl
  # - Class: fub_simulator_rviz_control/SimulatorControl
  #   Name: SimulatorControl

and

  # - Class: autonomos/KeyboardTool

Another problem is Kitti dataset data loader. I found the dataset format I downloaded from https://www.cvlibs.net/datasets/kitti/raw_data.php is not matching the expected data format you have in kitt_data_publisher.py. Could you point me out to the dataset you used?

The devcontainer I set up also pulls NVidia graphics installed on the host machine (requires Nvidia Docker setup), which will allow graphics acceleration of RViz. If this is troublesome, can comment out these lines:

An example of devcontainer:

Made some improvements to the documentation formatting in README.md

Setup the Docker such that the groundgrid folder should be placed in ~/workspace folder in the host machine, allowing reading data that is stored in the same folder from the development container within Docker.

Formatted each source file using LLVM style with 4 space indent width - see .clang-format

[Nussknacker]

Thank you for your contribution and for sharing your Docker-Devcontainer integration.

As for your problems: The mentioned unavailable plugins in the Rviz configuration are obsolete and can be removed.

We use the SemanticKITTI-dataset (http://www.semantic-kitti.org) which consists of the KITTI Odometry Benchmark Velodyne point clouds along with calibration data and the SemanticKitti-labels. You can find the download links here: http://www.semantic-kitti.org/dataset.html#download The kitti_data_publisher-scripts expects an argument to the folder containing the "sequences"-folder (your folder structure should look like the one for the semantic segmentation task: http://www.semantic-kitti.org/dataset.html#format)

Thank you for expanding the Readme. Just a small thing: The explicit starting of the roscore is not necessary, it is automatically started by the roslaunch command.

Additionally, you seem to have applied a global reformatting on all files. Please refrain from a global reformatting like this since it leads to conflicts with other forks/branches. Also, you seem to have moved the files in a way that the project should be put directly into the root of the ROS-workspace folder. We want to publish it as a self-contained ROS package. This means it is supposed to go into the src folder of your existing workspace.

[YevgeniyEngineer]

Thanks for the clarification.

I will attempt to follow the suggested steps to replay datasets.

I have not used ROS 1 much recently, mostly relying on ROS 2 development, so was not aware that roscore is executed during roslaunch. Thanks for the clarification.

I will undo the folder structure to it's original layout as per your recommendation, leaving just DevContainer and README changes, and removing optional packages from RViz config for repeatability, to simplify PR review.

[YevgeniyEngineer]

Hi @Nussknacker

I addressed your comments by only introducing changes that do not affect functional components of the code, but improve the quality of life for further development of code refinement.

Could you please attempt to setup Devcontainer yourself and see if it works for you?

The container will pull graphics drivers from your base system if you have nvidia-smi working and you installed Nvidia container toolkit.

The container offers several advantages over conventional development:

1. People contributing to the code base will be able to follow C++ style guide setup in .clang-format that is automatically recognised by devcontainer when it is setup
2. Devcontainer pulls all the ROS build dependencies
3. It sets up useful extensions in VSCode and binds to the physical volume allowing to browse Kitti dataset located within ~/workspace folder offering extra flexibility
4. It does not require the base Linux system to be compatible with the installed ROS version, Docker handles all dependencies and sets up virtual operating system

I did not have much time to run the code due to being extremely busy at work recently, but will do so when I find some free time most likely during weekends.

[YevgeniyEngineer]

Reading through the code, I noticed there is a bug (should be std::numeric_limits::epsilon()):

Also, I am not sure if this is intended, but it is set to std::numeric_limits::min() instead of std::numeric_limits::lowest()

[Nussknacker]

Hi @YevgeniyEngineer,

thanks again for the effort you put into this. I don't use VSCode personally, so I can't test the Devcontainer myself. However, I used the dockerfile to set up a container and can confirm that it works within the container. Only pandas was missing and had to be installed manually. This could also be added to the dockerfile. Another minor thing is that the workspace folder is owned by root and isn't writable by the devuser account. I don't know if that's intended. Also, the shell's display variable is set to display :1 via the bashrc. Not sure if that's needed for VSCode, but I had to set it to :0 to get RViz to work.

I had another look at the improved Readme file. It generally looks better now, but I found two small issues: First, the slash separator between SemanticKITTI and dataset disappeared for the playback command. It is intentional because the directory argument needs the path to the SemanticKIITI dataset, including the dataset directory. Second, you formatted the example output of the evaluation script into a proper table. This looks nicer, of course, but might give a false impression of what output to expect from the evaluation script. I think I'd prefer the original code block style so it properly reflects the script's actual output.

[Nussknacker]

Reading through the code, I noticed there is a bug (should be std::numeric_limits::epsilon()):

Also, I am not sure if this is intended, but it is set to std::numeric_limits::min() instead of std::numeric_limits::lowest()

The first usage of min() is intentional, But you are absolutely right in the second case. The maximum ground height is only collected for statistical reasons and not used for the ground segmentation. I might just remove the calculation of this layer since it's incorrect anyways.

[YevgeniyEngineer]

@Nussknacker I addressed your considerations in the latest commits. Apologies for taking so long

[Nussknacker]

Hi @YevgeniyEngineer,

thanks again for your work. I tested the updated version of the Dockerfile, but unfortunately, I can not get it to run anymore. The container immediately terminates after launching. This seems to be related to the newly defined entrypoint, which overwrites the /ros_entrypoint.sh script. If I remove the entrypoint, then it works again. Is this new entrypoint necessary?

[YevgeniyEngineer]

@Nussknacker You are right, the ENTRYPOINT line is not needed, because $DISPLAY can be loaded when executing docker run - fixed

[Nussknacker]

Hi @YevgeniyEngineer,

now it runs well again :) However, I saw that you added a symlink in the groundgrid/src folder pointing to its own parent resulting in a file system loop. You also added this to the Readme, but I don't understand what it is supposed to do. To build groundgrid in the docker container you just need to create a src-directory in the workspace folder ("mkdir src"), then run "catkin init" to initialize the workspace. Now you can just clone the groundgrid git in the src-folder of your workspace: "cd src; git clone https://github.com/dcmlr/groundgrid.git; cd .." and then build it with "catkin build -DCMAKE_BUILD_TYPE=Release groundgrid". After sourcing your workspace ("source devel/setup.bash") to make the newly build groundgrid package available, you can launch it with roslaunch as described in the Readme. For further information, you can check the documentation of ROS workspaces here: https://wiki.ros.org/catkin/workspaces

For people familiar with ROS, most of these steps won't be necessary because they already have set up a ROS workspace. For this reason, I did not put this description in the Readme.