Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove instance keys (Archetype-less, PoV-less, join-less queries) #5303

Closed
teh-cmc opened this issue Feb 27, 2024 · 0 comments · Fixed by #6104
Closed

Remove instance keys (Archetype-less, PoV-less, join-less queries) #5303

teh-cmc opened this issue Feb 27, 2024 · 0 comments · Fixed by #6104
Assignees
@teh-cmc
Labels
🔩 data model 🪵 Log & send APIs Affects the user-facing API for all languages ⛃ re_datastore affects the datastore itself

Comments

@teh-cmc
Copy link
Member

teh-cmc commented Feb 27, 2024

Random thoughts off the top of my head:

  • Remove InstanceKey from logging APIs
  • Rework re_data_store accordingly:
    • No auto-generated instance keys
    • No clustering key checks
    • Probably lots of tests to fix
  • Rework re_query accordingly:
    • Make it archetype-less, key-less, PoV-less, join-less
    • Do we still need re_query at all? If we do, it can be vastly simplified
    • Probably lots of tests to fix
  • Rework re_query_cache accordingly:
    • Make it archetype-less, key-less, PoV-less, join-less
    • Probably lots of tests to fix
  • Update every downstream crates accordingly:
    • Stuff driven by InstanceKey (e.g. hover) should be driven by
    • Visualizers
  • Introduce a new Rerun-agnostic Identifier component
    • Just a u64 for now, until we have datatype conversions
    • Doesn't do any magic, just a way to have persistent IDs to drive visualizer features

This is not an exhaustive list, InstanceKeys are everywhere.

It will be way simpler to build all of this on the side while keeping the existing stuff untouched and then switch everything, rather than living across two worlds for who knows how long.
I.e. build all of that stuff bite-size pieces in parallel, and only start removing the old stuff when things are in place.
@teh-cmc teh-cmc added ⛃ re_datastore affects the datastore itself 🔩 data model 🪵 Log & send APIs Affects the user-facing API for all languages labels Feb 27, 2024
@teh-cmc teh-cmc added this to the 0.15 milestone Feb 27, 2024
@teh-cmc teh-cmc self-assigned this Feb 27, 2024
@jleibs jleibs mentioned this issue Mar 5, 2024
Merged
5 tasks
jleibs added a commit that referenced this issue Mar 5, 2024
@jleibs
Remove the check for WrongNumberOfInstances (#5399)
9f29bd0 
### What
Also removed instances of the restriction in documentation.

I believe, in theory this means we can remove a bunch of places where we
determine and pass around num_instances. However, doing that requires
cleaning up the splat-determination logic and the special
splat-instance-key-value, which I assume will come as part of:
 - #5303

### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5399/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5399/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5399/index.html?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)!

- [PR Build Summary](https://build.rerun.io/pr/5399)
- [Docs
preview](https://rerun.io/preview/140181bdafde95319a803565065aa3b5ed9efa63/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/140181bdafde95319a803565065aa3b5ed9efa63/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@teh-cmc teh-cmc removed this from the 0.15 milestone Mar 15, 2024
@teh-cmc teh-cmc mentioned this issue Mar 27, 2024
Closed
@teh-cmc teh-cmc mentioned this issue Apr 12, 2024
Closed
@teh-cmc teh-cmc mentioned this issue Apr 24, 2024
Merged
16 tasks
teh-cmc added a commit that referenced this issue Apr 26, 2024
@teh-cmc
Remove instance keys and explicit splatting everywhere (#6104)
893de2f 
Annihilate everything related to instance keys.

---


- [x] Remove `InstanceKey`
- [x] Remove `NumInstances`
- [x] Remove explicit/client-side splats
- [x] Remove cluster keys and autogenerated cells
- [x] Clean up every other little thing related to any of the above
- [x] Update batching docs
- [x] Check that Kiss-ICP now works smoothly
- [x] Check roundtrips
- [x] Check `rerun snippets/*.rrd`
- [x] Rebase and run checklist
- [x] Run full check bot

---

Findings (none blocking, not sure any are new -- to be investigated
later):
- Depth clouds with visible history are broken in a different way than
before
- `OutOfTreeTransform` might be broken? (see associated snippet)
- Does `DisconnectedSpace` even work? (see associated snippet)
- Is something wrong with labels when there are lots of instances?

---

- Fixes #5303
- Fixes #1014
- Fixes #1777
- Fixes #1793
- Fixes #1893
- Fixes #5685
- Fixes #1014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@teh-cmc teh-cmc

Labels
🔩 data model 🪵 Log & send APIs Affects the user-facing API for all languages ⛃ re_datastore affects the datastore itself
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove instance keys and explicit splatting everywhere rerun-io/rerun
1 participant
@teh-cmc