TheCodez / dynamic-occupancy-grid-map

Implementation of "A Random Finite Set Approach for Dynamic Occupancy Grid Maps with Real-Time Application"
MIT License
281 stars 39 forks source link

Resource leak in texture.cu #78

Closed idlebear closed 3 years ago

idlebear commented 3 years ago

Hi there -- I've been rearranging the ROS build to integrate it with the Carla simulator over at dogma_ros. Everything builds but I have a question and bug for you.

The bug is simple -- CUDA isn't releasing the Texture resources and as a result, the GPU quickly runs out of headroom on my 1080 (only 6GB). If you add the following line

 void Texture::endCudaAccess(cudaSurfaceObject_t surfaceObject)
 {
     CHECK_ERROR(cudaGraphicsUnmapResources(1, &resource, nullptr));
+    CHECK_ERROR(cudaGraphicsUnregisterResource(resource));
     CHECK_ERROR(cudaDestroySurfaceObject(surfaceObject));
 }

then memory requirements stabilize at ~1Gb when feeding it a laserscan from Carla. I'd do a pull request, but my current repo/fork has been rearranged slightly to support the ROS changes.

The question might be more complicated -- I see that the grid has a position update, but doesn't seem to be tracking orientation. I haven't tried digging into the updateGrid code, but I'm guessing that I'll need to rotate the texture to world coordinates before applying it to the grid. Difficult? Already done?

Otherwise, many thanks for doing all the hard work!

TheCodez commented 3 years ago

Hello there,

thanks for your interest in the project. I cherry picked your fix and added you to the list of contributors 👍

I've been rearranging the ROS build to integrate it with the Carla simulator over at dogma_ros.

I would love to have those changes integrated into this repo at some point. Is the rviz visualization plugin working by any chance?

The question might be more complicated -- I see that the grid has a position update, but doesn't seem to be tracking orientation. I haven't tried digging into the updateGrid code, but I'm guessing that I'll need to rotate the texture to world coordinates before applying it to the grid. Difficult? Already done?

Yes, the orientation should also be saved in the DOGM, but thats currently missing. I'm guessing you need to rotate the scanner data using the orientation before using it as input for the measurement grid. Ideally, the measurement grid generator should also perform some coordinate system transformations, such as converting the laser scanner data into the vehicle coordinate system. Sadly, I'm not really familiar with that.

idlebear commented 3 years ago

I made enough changes to allow the plugin to build, but I don't think its functional as it was missing the DOGM::update() and DOGM::updateTopic() functions. I've stubbed them for now and updated my fork.

I'm still having issues getting the main prog to run -- keeps segfaulting on invalid read of the grid array when publishing. I'm using the same calls as the demo, but it's not happy. Does it even if I don't update/process the laserScan so something is up. I'm open to suggestions on where to look.

TheCodez commented 3 years ago

I made enough changes to allow the plugin to build, but I don't think its functional as it was missing the DOGM::update() and DOGM::updateTopic() functions.

The visualization was made up of parts of an older version thats why some code is missing.

DOGMDisplay::update can be implemented as follows:

void DOGMDisplay::update(float /*wall_dt*/, float /*ros_dt*/)
{
  transformMap();
}

updateTopic should be implemented like the MapDisplay but its missing some code, such as https://github.com/ros-visualization/rviz/blob/c6810642ab75f9056c1b9c1129ea9f5f1d1a4895/src/rviz/default_plugin/map_display.cpp#L70

https://github.com/ros-visualization/rviz/blob/c6810642ab75f9056c1b9c1129ea9f5f1d1a4895/src/rviz/default_plugin/map_display.cpp#L146

https://github.com/ros-visualization/rviz/blob/c6810642ab75f9056c1b9c1129ea9f5f1d1a4895/src/rviz/default_plugin/map_display.cpp#L167

I'm still having issues getting the main prog to run -- keeps segfaulting on invalid read of the grid array when publishing. I'm using the same calls as the demo, but it's not happy. Does it even if I don't update/process the laserScan so something is up. I'm open to suggestions on where to look.

I'm guessing the problem happens, because the grid_cell_array resides in the GPU memory. There's a method called getGridCells that copies the grid cells from GPU to the host memory and returns a vector. Those lines should be changed: https://github.com/idlebear/dogm_ros/blob/main/src/dogm_ros/src/dogm_ros/dogm_ros_converter.cpp#L34 https://github.com/idlebear/dogm_ros/blob/main/src/dogm_ros/src/dogm_ros/dogm_ros_converter.cpp#L71

similar to https://github.com/TheCodez/dynamic-occupancy-grid-map/blob/master/dogm/demo/utils/image_creation.cpp#L27

idlebear commented 3 years ago

Progress! Calling getGridCells() resolved it -- it works as a regular occupancy map in rViz. I'll tackle the dogm visualizer plugin next and open a new issue if something comes up.