-
Notifications
You must be signed in to change notification settings - Fork 373
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
Implement graph components and archetypes #7500
Conversation
In this design, are the node id's global? |
crates/store/re_types/definitions/rerun/components/graph_edge.fbs
Outdated
Show resolved
Hide resolved
@nikolausWest Good point! So far I've made the assumption that all node IDs are global and can be referenced across entities to allow edges that connect nodes from different entities (similar to We could probably use a namespaced approach (e.g. based on entity path) by changing the way we gather the nodes from the entities that are currently being visualized. Then, we would have to store this entity information in the edges though. However, it's probably simpler for the user to encode this information in the node IDs themselves. In the case of Do you have a particular use case for namespaced node IDs in mind? How would you expect Rerun to behave in that case? |
Class Ids aren't actually global in the sense that to resolve a class id into e.g. a color, we walk up the graph to the first Annotation Context and use that to look up the value. That means these are actually scoped.
I'm rather thinking about what happens if a user has multiple graphs. It would be a bit strange if edges started going between them because the user didn't correctly partition their node ids. |
One option to consider here could be to have two types of edges. One that is within-entity edges (just a pair of node ids). The other could be between entity edges (destination entity, optional(source id), optional(destination id)). There might be several ways to model that but that would at least be the general idea |
@nikolausWest Brief update: Edges now have two additional attributes
The following shows a new toy example of the new logging API, which I also streamlined: https://github.com/rerun-io/rerun/pull/7500/files#diff-d054c306b388fcc1e8daf9f0477735519df3eeb486030979c62478b5d43404dcR36-R66. I've also improved the debug graph viewer to show lists of nodes, edges, and their corresponding entity path. I'm currently working on getting overrides in the UI to work so that we can color nodes and edges to more easily understand the data model. This also helps me better understand the visualizer concepts. I have one outstanding design decision: The current systems allows for edges to live in completely unrelated parts of the entity hierarchy. This means that when the user choses to visualize a certain sub-entity not all edges are retrieved by default and the user has to manually specify the additional entities that contain the edges using Entity Path Filters:
Since this can be confusing due to the lack of discoverability, I think we should pull in global edges from outside the current hierarchy and visualizing them as dummy nodes. In the current design this forces us to iterate over all edges starting from the root. Should we mitigate this by introducing a new |
@grtlr I actually think having edges on different entities than nodes is the main reason to go with your proposal. That allows users to put different meta data on edges and nodes which is a very important feature. If you can think of a way to still keep it more local that might be worth while.
I'm not completely sure about this. Maybe @Wumpf has some thoughts? |
Haven't been following the entire discussion, but there's a lot of issues with pulling data from outside of a viewer's query/entity-pathfilter:
|
I think it's pretty clear we shouldn't include data from outside the entity path query. I think one question that leaves though is how to handle edges that are within the included entities but refer to nodes that are not. Perhaps some kind of greyed out nodes could make sense there (maybe even an option to include or exclude those) |
Thank you @Wumpf for the clarification—that makes a lot of sense! @nikolausWest I agree that we should show those as edges to some dummy nodes. In fact this is how I stumbled upon the above problem in the first place (undirected edges). |
d0e1194
to
acbefcb
Compare
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.
Let's merge this! 🎉 (as soon as ci is ✅)
<!-- Open the PR up as a draft until you feel it is ready for a proper review. Do not make PR:s from your own `main` branch, as that makes it difficult for reviewers to add their own fixes. Add any improvements to the branch as new commits to make it easier for reviewers to follow the progress. All commits will be squashed to a single commit once the PR is merged into `main`. Make sure you mention any issues that this PR closes in the description, as well as any other related issues. To get an auto-generated PR description you can put "copilot:summary" or "copilot:walkthrough" anywhere. --> Tracking issue: #7897 This implements basic graph primitives in the Rerun data model. This is a first step towards visualizing node-link diagrams in Rerun (related issue: #6898). In addition to the changes to the data model, this PR adds two example crates, `node_link_graph` and `graph_view` to the Rust examples that show how these primitives can be used. ## Design Decisions - Nodes and edges are stored as `components` and can be batched. To have a single node per entity we can use Rerun’s [[clamping mechanism](https://rerun.io/docs/concepts/batches#component-clamping)](https://rerun.io/docs/concepts/batches#component-clamping). - `GraphNodeId` is modeled as ~`u32` to improve performance when using `petgraph`~ strings for better user experience. - A node is unique identified by combining its `GraphNodeId` and its `EntityPath`. - Labels of the nodes can be set via the `labels` component and toggled via `show_labels` - ~Hierarchical graphs can be modeled through entity paths. For edges that cross entity boundaries we can insert dummy nodes to properly render subparts of the hierarchy.~ - ~Nodes and edges need to be logged to the same entity, otherwise the selections will collide. We provider helper functions / conversions to link nodes that are stored in different entities.~ > [!WARNING] > This PR has changed considerably from its initial version: > * No linking of nodes between entities (an therefore hierarchy) yet. > * For now, Graphs are local only and therefore have to be logged to the same entity. ## Logging example ```rs rec.log( "simple", &rerun::GraphNodes::new(["a", "b", "c"]) .with_positions([(0.0, 100.0), (-100.0, 0.0), (100.0, 0.0)]) .with_labels(["A", "B", "C"]), )?; // Note: We log to the same entity here. rec.log( "simple", &rerun::GraphEdges::new([("a", "b"), ("b", "c"), ("c", "a")]).with_directed_edges(), )?; ``` ## TODOs - [x] ~Get rid of the `Default` derive for `GraphNodeId` and `GraphEdge` in the flatbuffer definitions.~ - [x] Improve ergonomics for generating graph edges during logging. - [x] Ensure that logging works from Python and C++ too. - [x] Fix remaining lints. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7500?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7500?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7500) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --------- Co-authored-by: Antoine Beyeler <[email protected]> Co-authored-by: Andreas Reich <[email protected]>
🥳 |
Tracking issue: #7897
This implements basic graph primitives in the Rerun data model. This is a first step towards visualizing node-link diagrams in Rerun (related issue: #6898).
In addition to the changes to the data model, this PR adds two example crates,
node_link_graph
andgraph_view
to the Rust examples that show how these primitives can be used.Design Decisions
components
and can be batched. To have a single node per entity we can use Rerun’s [clamping mechanism](https://rerun.io/docs/concepts/batches#component-clamping).GraphNodeId
is modeled asstrings for better user experience.u32
to improve performance when usingpetgraph
GraphNodeId
and itsEntityPath
.labels
component and toggled viashow_labels
Hierarchical graphs can be modeled through entity paths. For edges that cross entity boundaries we can insert dummy nodes to properly render subparts of the hierarchy.Nodes and edges need to be logged to the same entity, otherwise the selections will collide. We provider helper functions / conversions to link nodes that are stored in different entities.Warning
This PR has changed considerably from its initial version:
Logging example
TODOs
Get rid of theDefault
derive forGraphNodeId
andGraphEdge
in the flatbuffer definitions.Checklist
main
build: rerun.io/viewernightly
build: rerun.io/viewerCHANGELOG.md
and the migration guideTo run all checks from
main
, comment on the PR with@rerun-bot full-check
.