add normals and outliers #200
Conversation
packages/lidar_map/README.md
Outdated
| 3. `icp-log.mcap` file contains normals and outliers for {cloud-number} cloud, percentage of normals will be shown = {percentage} | ||
|
|
||
| ```console | ||
| ros2 run lidar_map lidar_map_executable | ||
| --mcap-input data/rides/ride_atrium_XX_YY_ZZ.mcap | ||
| --mcap-output data/clouds/cloud_atrium_XX_YY_ZZ | ||
| --mcap-log data/logs/mcap/cloud_atrium_XX_YY_ZZ_log | ||
| --json-log data/logs/json/cloud_atrium_XX_YY_ZZ_log.json | ||
| --icp-log /root/truck/packages/lidar_map/data/results/f_normals | ||
| --cloud-number 3 | ||
| --percentage 90 |
There was a problem hiding this comment.
Так не будет делать, см. другие комментарии.
packages/lidar_map/README.md
Outdated
| 5. `--icp-log`: path to NON-EXISTING folder for saving ICP log file (normals and outliers) (optional) | ||
|
|
||
| 6. `--cloud-number`: Cloud number for visualizing normals and outliers for icp-log (optional) | ||
|
|
||
| 7. `--percentage`: Percentage of normals rendered [0;100] for icp-log (optional) | ||
|
|
||
|
|
There was a problem hiding this comment.
Мне кажется, что не нужно добавлять в аргументы терминала логику с включением логов для дебага нормалей и весов облака.
Приведу пример: я покатался по Атриуму, хочу очередную карту создать, делает её за меня черный ящик в виде пакета под названием lidar_map. Этому пакету в аргументах командной сктроки я обязан указать входной мкап (--mcap-input) и выходной мкап (--mcap-output). Пакет будет строить карту через решение задачи оптимизации, и мне может захотеться посмотреть глазками на шаги этой самой оптимизации, для этого у меня есть два вида логов: --mcap-log и --json-log. Эти логи я назову "глобальными", потому что они позволяют дебажить на "глобальном" уровне, то есть на уровне задачи оптимизации / на уровне всей карты.
В то же время, при решении этой самой задачи оптимизации через, например, json логи, я увидел, что есть какое-то кривое ICP-ребро и я хочу глянуть на ICP-матчинг соответствующих ему облаков. Для этого мне может быть полезно посмотреть на нормали и веса reference облака, которое матчится с reading облаком. Это тоже будет дебагинг, но уже "локальный", потому что он относится не ко всей задачи оптимизации / не ко всей карте, а к конкретной паре облаков. Именно по этой причине я не вижу смысла задавать для терминала аргументы логирования нормалей и весов, это выглядит немного странно.
| @@ -52,12 +52,16 @@ struct BuilderParams { | |||
|
|
|||
| class Builder { | |||
| public: | |||
| std::vector<CloudWithAttributes> clouds_with_attributes; | |||
There was a problem hiding this comment.
- Делать поле класса с модификатором доступа
public- это преступление... - Зачем нам собирать аттрибуты для всех-всех облаков? Атрибуты (нормали и веса) хочется считать только для того облака (reference облака), которое я передам в параметры специальной функции для рассчета этих самых аттрибутов, твое поле выпиливаем
Как я хотел бы, чтобы выглядела функция для вычисления аттрибутов:
CloudWithAttributes calculateAttributesForReferenceCloud(
const Cloud& reference_cloud, const Cloud& reading_cloud {
// Выплевываем по сути тот же reference_cloud, но уже с посчитанными полями нормалей и весов
// Помимо reference облака нужно передавать также и reading облако, чтобы посчитать веса,
// но аттрибуты мы считаем именно для reference облака, что явно отражено в названии функции
CloudWithAttributes reference_cloud_with_attr = ...; // TODO
return reference_cloud_with_attr;
}| void initPoseGraph( | ||
| const geom::Poses& poses, const Clouds& clouds, | ||
| const bool get_clouds_with_attributes = false); |
There was a problem hiding this comment.
Выпиливаем флаг get_clouds_with_attributes, см. другие комментарии.
| CloudWithAttributes getCloudWithAttributes( | ||
| const DataPoints& reference_dp, const DataPoints& reading_dp, const Cloud& cloud); | ||
|
|
There was a problem hiding this comment.
В комментариях выше я писал, как хотел бы, чтобы эта функция выглядела, продублирую сюда:
CloudWithAttributes calculateAttributesForReferenceCloud(
const Cloud& reference_cloud, const Cloud& reading_cloud {
// Выплевываем по сути тот же reference_cloud, но уже с посчитанными полями нормалей и весов
// Помимо reference облака нужно передавать также и reading облако, чтобы посчитать веса,
// но аттрибуты мы считаем именно для reference облака, что явно отражено в названии функции
CloudWithAttributes reference_cloud_with_attr = ...; // TODO
return reference_cloud_with_attr;
}Комментарии к твоей версии функции:
- очень путанные аргументы, должно быть только два облака на вход:
reference(для него считаем все аттрибуты) иreading(нужно для вычисления весов) - два входных облака должны иметь строго тип
Cloud
packages/lidar_map/src/builder.cpp
Outdated
| } | ||
|
|
||
| attributes.outliers = outliers; | ||
| cloud_with_attributes.attributes = attributes; |
There was a problem hiding this comment.
Зачем делать лишнее копирование из outliers в attributes.outliers? Можно сразу добавлять все веса в поле attributes.outliers. Тоже самое и с полем attributes: почему бы не заполнять его сразу же, без создания отдельной переменной attributes?
packages/lidar_map/src/main.cpp
Outdated
| if (!icp_log_path.empty()) { | ||
| mcap_writer.writeCloudWithAttributes( | ||
| icp_log_path, | ||
| builder.clouds_with_attributes[icp_cloud_number], | ||
| normals_rarefaction_percentage); | ||
| } |
There was a problem hiding this comment.
Не хотим собирать аттрибуты для всех облаков.
| size_t step = static_cast<size_t>(points_count / (points_count * (1 - (percent / 100)))); | ||
| for (size_t i = 0; i < points_count; i += step) { | ||
| visualization_msgs::msg::Marker msg_; | ||
| msg_.header.frame_id = "world"; |
There was a problem hiding this comment.
Фрейм для визуализации должен задаваться параметров отдельным, хардкодить его в функцию не очень идея, посмотри на другие функции класса MCAPWriter, там есть примеры как это аккуратно сделать.
| double dir_x = | ||
| cloud_with_attributes.attributes.normals(4, i) - cloud_with_attributes.cloud(0, i); | ||
| double dir_y = | ||
| cloud_with_attributes.attributes.normals(5, i) - cloud_with_attributes.cloud(1, i); |
There was a problem hiding this comment.
Выглядит как черная магия, не хватает текстовый комментариев, что, зачем и как именно здесь вычисляется.
| msg::toPointCloud2(cloud_with_attributes.attributes.outliers, "world"), | ||
| "outliers", |
There was a problem hiding this comment.
Фрейм для визуализации и названия топика должны задаваться аргументами функции, хардкодить их в внутри функции не очень идея, посмотри на другие функции класса MCAPWriter, там есть примеры как это аккуратно сделать.
No description provided.