Using dynamic number of labels #56
Conversation
There was a problem hiding this comment.
Hi @RozDavid, fantastic work!
I wonder if there could be a way to make it dynamic yet known at compile time to avoid the slow-down? I am not sure if Eigen vectorizes dynamic matrices by default or not, or if there is any type of memory alignment that needs to be done (I'd expect Eigen to do that automagically).
Otherwise, we could alternatively also allow static size matrices, perhaps using:
i) The quick (a bit ugly) way would be to use pre-processor directives to switch between dyanmic and static...
ii) Perhaps making the hardcoded number of labels being a #define directive which we could set from the CMakeLists.txt (though that doesn't avoid the re-compilation)...
iii) There must be a better way than i) and ii) :)
.gitignore
Outdated
| @@ -0,0 +1 @@ | |||
| cmake-build-debug/ | |||
There was a problem hiding this comment.
Can you delete this? (or rather add it to your global gitignore).
| cmake-build-debug/ |
| @@ -53,6 +53,8 @@ class SemanticLabel2Color { | |||
| // Make these public if someone wants to access them directly. | |||
| ColorToSemanticLabelMap color_to_semantic_label_; | |||
| SemanticLabelToColorMap semantic_label_to_color_map_; | |||
|
|
|||
| size_t number_of_colored_labels; | |||
There was a problem hiding this comment.
we use a trailing _ for members of a class.
| size_t number_of_colored_labels; | |
| size_t number_of_colored_labels_; |
| SemanticProbabilities; | ||
| // A `#Labels X #Labels` Eigen matrix where each `j` column represents the | ||
| // probability of observing label `j` when current label is `i`, where `i` | ||
| // is the row index of the matrix. | ||
| typedef Eigen:: | ||
| Matrix<SemanticProbability, kTotalNumberOfLabels, kTotalNumberOfLabels> | ||
| Matrix<SemanticProbability, Eigen::Dynamic, Eigen::Dynamic> | ||
| SemanticLikelihoodFunction; |
| @@ -76,6 +76,10 @@ class SemanticIntegratorBase { | |||
| // probability of non-match = 1 - measurement_probability_. | |||
| SemanticProbability semantic_measurement_probability_ = 0.9f; | |||
|
|
|||
| //The total number of labels in the segmenation node | |||
| //Used by the confidence matrices | |||
| size_t total_number_of_labels = 21; | |||
There was a problem hiding this comment.
| size_t total_number_of_labels = 21; | |
| size_t total_number_of_labels_ = 21; |
kimera_semantics/src/color.cpp
Outdated
| @@ -62,6 +62,7 @@ SemanticLabel2Color::SemanticLabel2Color(const std::string& filename) | |||
| // TODO(Toni): remove | |||
| // Assign color 255,255,255 to unknown object 0u | |||
| color_to_semantic_label_[HashableColor::White()] = 0u; | |||
| number_of_colored_labels = row_number; | |||
There was a problem hiding this comment.
| number_of_colored_labels = row_number; | |
| number_of_colored_labels_ = row_number; |
| @@ -325,9 +344,9 @@ void SemanticIntegratorBase::normalizeProbabilities( | |||
| if (normalization_factor != 0.0) { | |||
| unnormalized_probs->normalize(); | |||
| } else { | |||
| CHECK_EQ(unnormalized_probs->size(), kTotalNumberOfLabels); | |||
| CHECK_EQ(unnormalized_probs->size(), semantic_config_.total_number_of_labels); | |||
There was a problem hiding this comment.
| CHECK_EQ(unnormalized_probs->size(), semantic_config_.total_number_of_labels); | |
| CHECK_EQ(unnormalized_probs->size(), semantic_config_.total_number_of_labels_); |
| SemanticProbabilities semantic_label_frequencies = | ||
| SemanticProbabilities::Zero(); | ||
| SemanticProbabilities semantic_label_frequencies; | ||
| semantic_label_frequencies.resize(semantic_config_.total_number_of_labels, 1); |
There was a problem hiding this comment.
| semantic_label_frequencies.resize(semantic_config_.total_number_of_labels, 1); | |
| semantic_label_frequencies.resize(semantic_config_.total_number_of_labels_, 1); |
| @@ -319,7 +320,7 @@ void MergedSemanticTsdfIntegrator::integrateVoxel( | |||
| merged_weight, voxel); | |||
|
|
|||
| SemanticVoxel* semantic_voxel = | |||
| allocateStorageAndGetSemanticVoxelPtr(global_voxel_idx, &semantic_block, &semantic_block_idx); | |||
| allocateStorageAndGetSemanticVoxelPtr(global_voxel_idx, semantic_config_.total_number_of_labels, &semantic_block, &semantic_block_idx); | |||
There was a problem hiding this comment.
| allocateStorageAndGetSemanticVoxelPtr(global_voxel_idx, semantic_config_.total_number_of_labels, &semantic_block, &semantic_block_idx); | |
| allocateStorageAndGetSemanticVoxelPtr(global_voxel_idx, semantic_config_.total_number_of_labels_, &semantic_block, &semantic_block_idx); |
| nh_private.param("total_number_of_labels", | ||
| total_number_of_labels, | ||
| total_number_of_labels); | ||
| semantic_config.total_number_of_labels = static_cast<size_t>(total_number_of_labels); |
There was a problem hiding this comment.
| semantic_config.total_number_of_labels = static_cast<size_t>(total_number_of_labels); | |
| semantic_config.total_number_of_labels = static_cast<size_t>(total_number_of_labels_); |
| @@ -48,6 +48,13 @@ getSemanticTsdfIntegratorConfigFromRosParam(const ros::NodeHandle& nh_private) { | |||
| semantic_config.semantic_measurement_probability_ = | |||
| static_cast<SemanticProbability>(semantic_measurement_probability); | |||
|
|
|||
| // Get the total number of labels of the prediction | |||
| int total_number_of_labels = static_cast<int>(semantic_config.total_number_of_labels); | |||
There was a problem hiding this comment.
| int total_number_of_labels = static_cast<int>(semantic_config.total_number_of_labels); | |
| int total_number_of_labels = static_cast<int>(semantic_config.total_number_of_labels_); |
|
Hey @ToniRV, Thanks a lot for the feedback and also for the ideas. I am sadly not a pro here, but did a short research on Eigen Dynamic memory allocation and found this. Source.
I believe if we don't change the directives What do you think of this? Do you think if I replace the initialization with |
|
@RozDavid I also understood that when re-reading Eigen, the operations should already be vectorized... So it might well be we can't improve on that. It's fine for me, I think this adds a lot of flexibility anyway. |
|
Hello @ToniRV, I ran a few quick tests comparing the static and dynamic approaches with different number of labels for the probability matrices. So when tested with static I recompiled the code with It was a bit surprising for me, that there is no difference between the layer sizes, only in the integration times. My interpretation is that Eigen allocates fixed sized memory up until a certain size of matrix (turns out this might be a bigger treshold than 128). As all the vxblx file sizes are between 66.3 and 67.2 mb, the only difference here is the number of allocated blocks and about 15% performance loss in pointcloud integration. Surely it depends on the use case if the flexibility is worth the performance loss or not, but I wanted to share this with you either way if you choose to merge or not. The results can be compared here: ########## Vxblx file size: 67.1Mb ########## Vxblx file size: 66.3Mb ########## Vxblx file size: 67.2Mb ########## Vxblx file size: 66.5Mb |
Addressing my own issue #55, using dynamic sized semantic prior matrices and initialization in runtime.
Reading number of labels from the launch file as a rosparam.
I believe the biggest concern could be the performance issue, for this here is a comparison of timings on the provided simulated semantic rosbag file.
With static labels
With dynamic labels