Feature/partition configs #56
Conversation
Schmluk
left a comment
Schmluk
left a comment
There was a problem hiding this comment.
Looks great, thanks a lot for adding these features!
| partitions: | ||
| 2: | ||
| edges: {scale: 0.1, alpha: 0.2, color: {type: UniformEdgeColorAdapter}} | ||
| 2p*: |
| interlayer_insertion_skip: 0 | ||
| edges: {scale: 0.01, alpha: 0.5, color: {type: UniformEdgeColorAdapter}} | ||
| interlayer_edges: | ||
| - {from: 3*, to: 2p*, draw: false} |
There was a problem hiding this comment.
-
Minor: Would it make sense to imply * if no partitions are mentioned? i.e.
from: 3would apply to all partitions of 3, that would seem intuitive. -
Minor minor: Would
*p1work to link up specific partitions only? Not sure if needed though.
There was a problem hiding this comment.
I think that would make more sense in the final version (that gets put in spark-dsg), but I think that would lead to confusing behavior for configs that hadn't been updated yet at the moment (e.g., 2 being interperted as all partitions would mean that the agent layers pick up the object layer config as the default config)
| //! @brief draw intralayer edges | ||
| //! @brief Draw edges | ||
| bool draw = true; | ||
| //! @brief intralayer edge size |
| nh_(nh), | ||
| graph_config_("scene_graph", config.graph, [this]() { has_change_ = true; }), | ||
| pub_(nh.create_publisher<MarkerArray>("graph", rclcpp::QoS(1).transient_local())), | ||
| has_change_(false) { |
| * - [LayerId]p* (e.g., '2p*'), which selects Layer 2, Partitions >= 1 | ||
| * - [LayerId]* (e.g., '2*'), which selects Layer 2, Partitions >= 0 | ||
| */ | ||
| struct LayerKeySelector { |
There was a problem hiding this comment.
Minor/later: This whole functionality would probably be really useful to have in a unified version in spark_dsg, that somewhat naturally goes from names to IDs and back (including the wildcard and partition selection), I think that'd be really awesome and might make some interfaces more flexible and easier to understand. That might also go hand in hand with the 'layers as views' idea. (Maybe could be implemented similar to the NodeSymbols)
E.g. the idea that something like this is correct syntax is really cool:
for (const auto& [id, node]: graph.layer("PLACESp[1-3]") { ... }
There was a problem hiding this comment.
I had a similar thought 😄
nathanhhughes
force-pushed
the
feature/partition_configs
branch
from
85d13f4 to
7ae4f63
Compare
Took longer than I was hoping, but finally fixed the visualizer to work how we want:
I am seeing the dynamic config gui freeze after a little bit, will add that to the list of bugs I need to track down (the hydra objects are broken still and there seems to be some issues with backend node merging)