Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 134 additions & 0 deletions beluga/include/beluga/actions/overlay.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright 2024 Ekumen, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef BELUGA_ACTIONS_OVERLAY_HPP
#define BELUGA_ACTIONS_OVERLAY_HPP

#include <algorithm>
#include <execution>

#include <range/v3/action/action.hpp>
#include <range/v3/view/common.hpp>
#include <range/v3/view/transform.hpp>

/**
* \file
* \brief Implementation of the overlay range adaptor object
*/
namespace beluga::actions {

namespace detail {

/// Implementation detail for an overlay range adaptor object.
struct overlay_base_fn {
/// Overload that implements an overlay of a value in a range.
/**
* \tparam ExecutionPolicy An [execution policy](https://en.cppreference.com/w/cpp/algorithm/execution_policy_tag_t).
* \tparam Range An [input range](https://en.cppreference.com/w/cpp/ranges/input_range).
* \tparam MaskRange An [input range](https://en.cppreference.com/w/cpp/ranges/input_range).
* \param policy The execution policy to use.
* \param range An existing range to apply this action to.
* \param mask The mask where the values will be overlaid.
* \param mask_value The value to be overlaid.
*/
template <
class ExecutionPolicy,
class Range,
class MaskRange,
class Mask,
Comment thread
DPR00 marked this conversation as resolved.
std::enable_if_t<std::is_execution_policy_v<std::decay_t<ExecutionPolicy>>, int> = 0,
std::enable_if_t<ranges::range<Range>, int> = 0,
std::enable_if_t<ranges::range<MaskRange>, int> = 0>
constexpr auto operator()(ExecutionPolicy&& policy, Range& range, MaskRange&& mask, Mask&& mask_value) const
-> Range& {
auto map = range | ranges::views::common;
const auto converted_mask_value = static_cast<ranges::range_value_t<Range>>(mask_value);

Comment thread
nahueespinosa marked this conversation as resolved.
std::transform(
policy, //
std::begin(map), //
std::end(map), //
std::begin(mask), //
std::begin(map), //
[&converted_mask_value](const auto& base_value, bool flag) {
return flag ? converted_mask_value : base_value;
});

return range;
}

/// Overload that re-orders arguments from an action closure.
template <
class ExecutionPolicy,
class Range,
class MaskRange,
class Mask,
std::enable_if_t<std::is_execution_policy_v<ExecutionPolicy>, int> = 0,
std::enable_if_t<ranges::range<Range>, int> = 0,
std::enable_if_t<ranges::range<MaskRange>, int> = 0>
constexpr auto operator()(Range&& range, MaskRange&& mask, Mask&& mask_value, ExecutionPolicy policy) const
-> Range& {
return (*this)(
std::move(policy), std::forward<Range>(range), std::forward<MaskRange>(mask), std::forward<Mask>(mask_value));
}

/// Overload that returns an action closure to compose with other actions.
template <
class ExecutionPolicy,
class MaskRange,
class Mask,
std::enable_if_t<std::is_execution_policy_v<ExecutionPolicy>, int> = 0,
std::enable_if_t<ranges::range<MaskRange>, int> = 0>
constexpr auto operator()(ExecutionPolicy policy, MaskRange&& mask, Mask&& mask_value) const {
return ranges::make_action_closure(ranges::bind_back(
overlay_base_fn{}, std::forward<MaskRange>(mask), std::forward<Mask>(mask_value), std::move(policy)));
}
};

/// Implementation detail for an overlay range adaptor object with a default execution policy.
struct overlay_fn : public overlay_base_fn {
using overlay_base_fn::operator();

/// Overload that defines a default execution policy.
template <
class Range,
class MaskRange,
class Mask,
std::enable_if_t<ranges::range<Range>, int> = 0,
std::enable_if_t<ranges::range<MaskRange>, int> = 0>
constexpr auto operator()(Range&& range, MaskRange&& mask, Mask&& mask_value) const -> Range& {
return (*this)(
std::execution::seq, std::forward<Range>(range), std::forward<MaskRange>(mask), std::forward<Mask>(mask_value));
}

/// Overload that returns an action closure to compose with other actions.
template <class MaskRange, class Mask, std::enable_if_t<ranges::range<MaskRange>, int> = 0>
constexpr auto operator()(MaskRange&& mask, Mask&& mask_value) const {
return ranges::make_action_closure(
ranges::bind_back(overlay_fn{}, std::forward<MaskRange>(mask), std::forward<Mask>(mask_value)));
}
};

} // namespace detail

/// [Range adaptor object](https://en.cppreference.com/w/cpp/named_req/RangeAdaptorObject) that
/// can overlay a range of values (or a range of particles).
/**
* The `overlay` range adaptor allows to overlay the values of the range that match a mask.
* All the values are overlaid for a given value.
*/
inline constexpr ranges::actions::action_closure<detail::overlay_fn> overlay;
} // namespace beluga::actions

#endif
10 changes: 5 additions & 5 deletions beluga/include/beluga/algorithm/distance_map.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ namespace beluga {
* (std::size_t) -> NeighborsT, where NeighborsT is a
* [Range](https://en.cppreference.com/w/cpp/ranges/range)
* with value type std::size_t.
* \param obstacle_map A map that represents obstacles in an environment.
* \param obstacle_mask A mask that represents obstacles in an environment.
* If the value of a cell is True, the cell has an obstacle.
* \param distance_function Given the indexes of two cells in the map i and j,
* obstacle_map(i, j) must return the distance between the two cells.
Expand All @@ -52,7 +52,7 @@ namespace beluga {
*/
template <class Range, class DistanceFunction, class NeighborsFunction>
auto nearest_obstacle_distance_map(
Range&& obstacle_map,
Range&& obstacle_mask,
DistanceFunction&& distance_function,
NeighborsFunction&& neighbors_function) {
struct IndexPair {
Expand All @@ -61,15 +61,15 @@ auto nearest_obstacle_distance_map(
};

using DistanceType = std::invoke_result_t<DistanceFunction, std::size_t, std::size_t>;
auto distance_map = std::vector<DistanceType>(ranges::size(obstacle_map));
auto visited = std::vector<bool>(ranges::size(obstacle_map), false);
auto distance_map = std::vector<DistanceType>(ranges::size(obstacle_mask));
auto visited = std::vector<bool>(ranges::size(obstacle_mask), false);

auto compare = [&distance_map](const IndexPair& first, const IndexPair& second) {
return distance_map[first.index] > distance_map[second.index];
};
auto queue = std::priority_queue<IndexPair, std::vector<IndexPair>, decltype(compare)>{compare};

for (auto [index, is_obstacle] : ranges::views::enumerate(obstacle_map)) {
for (auto [index, is_obstacle] : ranges::views::enumerate(obstacle_mask)) {
if (is_obstacle) {
visited[index] = true;
distance_map[index] = 0;
Expand Down
15 changes: 12 additions & 3 deletions beluga/include/beluga/sensor/data/occupancy_grid.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ namespace beluga {
* `g.coordinates_for(r, f)` returns a range of embedding space coordinates in the
* corresponding frame as `Eigen::Vector2d` values;
* - `g.free_cells()` returns a range of `std::size_t` indices to free grid cells;
* - `g.obstacle_data()` returns a range of `bool` values, representing grid cell occupancy;
* - `g.obstacle_mask()` returns a range of `bool` values, representing grid cell occupancy;
* - `g.unknown_mask()` returns a range of `bool` values, representing the unkwnown space of the grid cells;
*/

/// Occupancy 2D grid base type.
Expand Down Expand Up @@ -174,13 +175,21 @@ class BaseOccupancyGrid2 : public BaseLinearGrid2<Derived> {
ranges::views::transform([](const auto& tuple) { return std::get<0>(tuple); });
}

/// Retrieves grid data using true booleans for obstacles.
[[nodiscard]] auto obstacle_data() const {
/// Retrieves a mask over occupied cells in the grid.
[[nodiscard]] auto obstacle_mask() const {
return this->self().data() |
ranges::views::transform([value_traits = this->self().value_traits()](const auto& value) {
return value_traits.is_occupied(value);
});
}

/// Retrieves a mask over unknown cells in the grid.
[[nodiscard]] auto unknown_mask() const {
return this->self().data() |
ranges::views::transform([value_traits = this->self().value_traits()](const auto& value) {
return value_traits.is_unknown(value);
});
}
};

} // namespace beluga
Expand Down
36 changes: 26 additions & 10 deletions beluga/include/beluga/sensor/likelihood_field_model.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
#include <random>
#include <vector>

#include <beluga/actions/overlay.hpp>
#include <beluga/algorithm/distance_map.hpp>
#include <beluga/sensor/data/occupancy_grid.hpp>
#include <beluga/sensor/data/value_grid.hpp>
#include <range/v3/action/transform.hpp>
#include <range/v3/range/conversion.hpp>
#include <range/v3/view/all.hpp>
#include <range/v3/view/transform.hpp>
Expand Down Expand Up @@ -58,6 +60,8 @@ struct LikelihoodFieldModelParam {
* Used to calculate the probability of the obstacle being hit.
*/
double sigma_hit = 0.2;
/// Whether to model unknown space or assume it free.
bool model_unknown_space = false;
};

/// Likelihood field sensor model for range finders.
Expand Down Expand Up @@ -161,23 +165,35 @@ class LikelihoodFieldModel {
return std::min(squared_distance, squared_max_distance);
};

const auto to_likelihood = [amplitude =
params.z_hit / (params.sigma_hit * std::sqrt(2 * Sophus::Constants<double>::pi())),
two_squared_sigma = 2 * params.sigma_hit * params.sigma_hit,
offset = params.z_random / params.max_laser_distance](double squared_distance) {
assert(two_squared_sigma > 0.0);
assert(amplitude > 0.0);
/// Pre-computed variables
const double two_squared_sigma = 2 * params.sigma_hit * params.sigma_hit;
assert(two_squared_sigma > 0.0);

const double amplitude = params.z_hit / (params.sigma_hit * std::sqrt(2 * Sophus::Constants<double>::pi()));
assert(amplitude > 0.0);

const double offset = params.z_random / params.max_laser_distance;

const auto to_likelihood = [amplitude, two_squared_sigma, offset](double squared_distance) {
return amplitude * std::exp(-squared_distance / two_squared_sigma) + offset;
};

const auto neighborhood = [&grid](std::size_t index) { return grid.neighborhood4(index); };

// determine distances to obstacles and calculate likelihood values in-place
// to minimize memory usage when dealing with large maps
auto likelihood_values = nearest_obstacle_distance_map(grid.obstacle_data(), squared_distance, neighborhood);
std::transform(
likelihood_values.begin(), likelihood_values.end(), likelihood_values.begin(), truncate_to_max_distance);
std::transform(likelihood_values.begin(), likelihood_values.end(), likelihood_values.begin(), to_likelihood);
auto distance_map = nearest_obstacle_distance_map(grid.obstacle_mask(), squared_distance, neighborhood);

if (params.model_unknown_space) {
const double inverse_max_distance = 1 / params.max_laser_distance;

@glpuga-eka glpuga-eka Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DPR00 @hidmic I'm going through this code to help rodrigo with #577, but I can't fully make sense of this.

After a while of thinking this go a long way around assigning 1 / params.max_laser_distance likelihood to any unknown cell. The latest version of this blurs this even more because that value may be overriden here, which may be right or may be wrong depending what we model here.

So my question is: why 1 / params.max_laser_distance? That models a uniform distribution over the lenght of the lidar assuming with 100% certainty that this is a random measurement in that range. Why is this not offset = params.z_random / params.max_laser_distance which is the overall probability of a random measurement falling here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this was took from this part of Probabilistic Robotics book (Section 6.4.2, page 173):
image

@hidmic hidmic Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That. IIUC the idea is that the probability of a hit for any given measurement is uniform: $z \in [0, z_{max}]$, then $p(z) = \frac{1}{z_{max}}$). Mixing parameters are not applicable because those presume knowledge we don't have (well, kinda, we could still model failure and unexpected objects, I think, but they didn't and we don't).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's right, and I explain why.

The problem is that that causes the likelihood map to implement different likelihood functions depending on the lidar happens to hit, causing non-uniform weight scales. Depending on the lidar range the likelihood might be higher through an open window than 20cm away from an actually mapped obstacle.

Mixing parameters are not applicable because those presume knowledge we don't have

I think that not mixing assumes a knowledge we don't have. It asummes we know with 100% certainty that the measurement IS a random hit, which we can't know. Where a hit falls does not depend as much on the measurement as much as on the hyphotesis/particle you are tracing it from, so a hit on unknown area behind a wall surface or through a window most likely just means a hyphotesis which is 5 cm closer to the wall than the actual pose of the lidar. That hypothesis is not wrong, it's just part of the internal representation of the distribution that the filter uses for the pose.

If we assign 1 / params.max_laser_distance, we are overweighing particles with beams that hit within unknown areas, because z_random < 1.0,

If we assign a weight params.z_random / params.max_laser_distance to that case, that's the weight we assign to random impacts everywhere, and indeed that's how we model "random obstacles that are not mapped", which makes sense for unknown areas.

Notice that while I think that using params.z_random / params.max_laser_distance is at least in line with the larger model and does not distort the likelihood distribution value scale, I still don't think it is right.

That's because the likelihood model only really models known obstacles; more than a probabilistic model, LF is a "scan correlator". In fact, the z_random term is not really there to decide on the "goodness" of an impact over another, since it's the same everywhere; it's just there to avoid the likelihood product going to zero too fast if we hit unmapped obstacles.

In ignoring the z_hit term behind a wall we are biasing the estimation solution towards the external side of the wall by truncating the natural symmetry of the distance-likelihood distribution on both sides of the surface. In closed quarters this may not make much of a difference, but if you only see a straight wall on your left, this alone may add you a few centimeters of error due to that bias this causes in the particle distribution.

@hidmic hidmic Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we assign a weight params.z_random / params.max_laser_distance to that case, that's the weight we assign to random impacts everywhere, and indeed that's how we model "random obstacles that are not mapped", which makes sense for unknown areas.

Hmm, but random hit != unknown hit. Random hits have lower probability because it is implied they are transient. Thrun mentions cross-talk and echoes as potential sources.

That said...

In ignoring the z_hit term behind a wall we are biasing the estimation solution towards the external side of the wall by truncating the natural symmetry of the distance-likelihood distribution on both sides of the surface.

Yeah, I see the problem now 馃

(stream of consciousness ahead)

IIUC we are folding both unobservable and unmapped as unknown, and we are overpowering the base likelihood model when we shouldn't. Assigning a higher likelihood to hits within internal walls or past external walls just gives us bias. Assigning a higher likelihood to hits within an unmapped but otherwise visible region does prevent us from excessively penalizing hypothesis close to such regions when those regions happen to be occupied. If the unmapped space is within a room that had its door closed during mapping, we do want hits within that unexplored space not be ruled out as highly unlikely.

Hmm, what if we let the user configure a distance threshold? Unexplored space that is within that distance from occupied cells decays normally, while unexplored space past that distance gets the truncated likelihood to signal we don't really know if a hit in that region is likely or not. We cover for the variance in the estimate while not (excessively) penalizing hitting a car parked out the window.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created #582 to further discuss this.

const double background_distance = -two_squared_sigma * std::log((inverse_max_distance - offset) / amplitude);

distance_map |= beluga::actions::overlay(grid.unknown_mask(), background_distance);
Comment thread
DPR00 marked this conversation as resolved.
}

auto likelihood_values = std::move(distance_map) | //
ranges::actions::transform(truncate_to_max_distance) | //
ranges::actions::transform(to_likelihood);

return ValueGrid2<float>{std::move(likelihood_values), grid.width(), grid.resolution()};
}
Expand Down
2 changes: 2 additions & 0 deletions beluga/test/beluga/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ add_executable(
test_beluga
actions/test_assign.cpp
actions/test_normalize.cpp
actions/test_overlay.cpp
actions/test_propagate.cpp
actions/test_reweight.cpp
algorithm/raycasting/test_bresenham.cpp
Expand Down Expand Up @@ -51,6 +52,7 @@ add_executable(
sensor/test_beam_model.cpp
sensor/test_bearing_sensor_model.cpp
sensor/test_landmark_sensor_model.cpp
sensor/test_lfm_with_unknown_space.cpp
sensor/test_likelihood_field_model.cpp
sensor/test_ndt_model.cpp
test_3d_embedding.cpp
Expand Down
Loading