-
Notifications
You must be signed in to change notification settings - Fork 168
New planner #792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New planner #792
Changes from all commits
c3b2544
ddf381f
130da36
f4890c9
98c899b
3edcf9b
e1a51d3
bb6a809
9814f63
8548c0b
62ce4de
e6a1ad7
93685bb
ca5ead8
2d6ea5f
4917ff4
fd366cc
61ad191
0beef85
d7ef9bb
0ede7eb
5e142a2
771df08
8347f13
485a69d
1925e15
1215efa
6868247
5af8bc9
cbfa7d3
f417bed
0bedaae
37c533c
3c9d974
662c8c0
7895dea
610fce5
2cbf19b
62e7599
24ee6dd
435b131
95148ba
4655f41
f586de0
d1ec605
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,3 +55,5 @@ yolo11n.pt | |
| .claude | ||
|
|
||
| /logs | ||
|
|
||
| *.so | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -13,18 +13,36 @@ | |||
| # limitations under the License. | ||||
|
|
||||
| from functools import cached_property | ||||
| import re | ||||
|
|
||||
| from pydantic_settings import BaseSettings, SettingsConfigDict | ||||
|
|
||||
| from dimos.mapping.occupancy.path_map import NavigationStrategy | ||||
| from dimos.navigation.global_planner.types import AStarAlgorithm | ||||
|
|
||||
|
|
||||
| def _get_all_numbers(s: str) -> list[float]: | ||||
| return [float(x) for x in re.findall(r"-?\d+\.?\d*", s)] | ||||
|
|
||||
|
|
||||
| class GlobalConfig(BaseSettings): | ||||
| robot_ip: str | None = None | ||||
| simulation: bool = False | ||||
| replay: bool = False | ||||
| n_dask_workers: int = 2 | ||||
| memory_limit: str = "auto" | ||||
| mujoco_camera_position: str | None = None | ||||
| mujoco_room: str | None = None | ||||
| mujoco_room_from_occupancy: str | None = None | ||||
| mujoco_global_costmap_from_occupancy: str | None = None | ||||
| mujoco_global_map_from_pointcloud: str | None = None | ||||
| mujoco_start_pos: str = "-1.0, 1.0" | ||||
| robot_model: str | None = None | ||||
| robot_width: float = 0.3 | ||||
| robot_rotation_diameter: float = 0.6 | ||||
| planner_strategy: NavigationStrategy = "simple" | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm not a fan of centralized config that's expanded like this in a single place, "core" shouldn't care about your particular "planner_strategy" we have a standard for shared config that comes with Configurable, overlayed on top of ModuleConfig, that also applies defaults. Line 47 in 0c64608
For global config we should be able to easily apply a deep dict, like {
"memory_limit": ...,
"n_dask_workers": ...
# this is class name
"astar_navigator": {
"strategy": ...
}
}then if we want convenient CLI we can play with some sort of deep matching etc, but for a start
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of these are meant to serve as a sort of feature flags: added as a way to test which is working better and removed once we're certain which is best. The issue with a nested config is where to place the value. In this example, I'm okay with using a nested config, though. Do you think it would be better if you added the code to read/pass the config? I'd do it myself, but I'm sensing a slight difference of opinion here (for example on yaml too) so it would be cool if I could see what you have in mind. 😃
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aha this is important so let's make sure that I understand your perspective first also, don't want to trample over it if I don't understand it :D :D
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had a talk, implementing a config system in a separate PR |
||||
| astar_algorithm: AStarAlgorithm = "min_cost" | ||||
| planner_robot_speed: float | None = None | ||||
|
|
||||
| model_config = SettingsConfigDict( | ||||
| env_file=".env", | ||||
|
|
@@ -40,3 +58,14 @@ def unitree_connection_type(self) -> str: | |||
| if self.simulation: | ||||
| return "mujoco" | ||||
| return "webrtc" | ||||
|
|
||||
| @cached_property | ||||
| def mujoco_start_pos_float(self) -> tuple[float, float]: | ||||
| x, y = _get_all_numbers(self.mujoco_start_pos) | ||||
| return (x, y) | ||||
|
|
||||
| @cached_property | ||||
| def mujoco_camera_position_float(self) -> tuple[float, ...] | None: | ||||
| if self.mujoco_camera_position is None: | ||||
| return None | ||||
| return tuple(_get_all_numbers(self.mujoco_camera_position)) | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # Copyright 2025 Dimensional 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. | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
|
|
||
| from dimos.mapping.occupancy.gradient import gradient | ||
| from dimos.msgs.nav_msgs.OccupancyGrid import OccupancyGrid | ||
| from dimos.utils.data import get_data | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def occupancy() -> OccupancyGrid: | ||
| return OccupancyGrid(np.load(get_data("occupancy_simple.npy"))) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def occupancy_gradient(occupancy) -> OccupancyGrid: | ||
| return gradient(occupancy, max_distance=1.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think we should move all these into some planner/ dir or something? I assume we either need most or none
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I'm not sure. There are quite a few things which need to be moved in there and doing it in this PR would make it even bigger than it already is. I'll add this on my todo list.