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 and explicit splatting everywhere #6104

Merged
merged 8 commits into from
Apr 26, 2024
Merged

Remove instance keys and explicit splatting everywhere #6104

merged 8 commits into from
Apr 26, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Apr 24, 2024 •
edited
Loading

Annihilate everything related to instance keys.

  • Remove InstanceKey
  • Remove NumInstances
  • Remove explicit/client-side splats
  • Remove cluster keys and autogenerated cells
  • Clean up every other little thing related to any of the above
  • Update batching docs
  • Check that Kiss-ICP now works smoothly
  • Check roundtrips
  • Check rerun snippets/*.rrd
  • Rebase and run checklist
  • 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?
  • StoreEvents test failed or what?

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@teh-cmc teh-cmc added ⛃ re_datastore affects the datastore itself do-not-merge Do not merge this PR include in changelog 🔩 data model labels Apr 24, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 24, 2024 •
edited
Loading

Deployed docs

Commit Link
ccd3eb6 https://landing-lzu5ctfjh-rerun.vercel.app/docs

@teh-cmc teh-cmc changed the title Remove instance keys and everything related to them Remove instance keys and explicit splatting everywhere Apr 25, 2024
@teh-cmc teh-cmc force-pushed the cmc/remove_instance_keys branch 7 times, most recently from 89e240b to 2395717 Compare April 25, 2024 11:20
@teh-cmc teh-cmc force-pushed the cmc/data_apis_16_non_cacheable_components branch from ecb7892 to a0ff353 Compare April 25, 2024 16:41
@teh-cmc teh-cmc force-pushed the cmc/remove_instance_keys branch 2 times, most recently from 0cc5e9a to 9c5a705 Compare April 25, 2024 16:49
@teh-cmc teh-cmc added the 🚀 performance Optimization, memory use, etc label Apr 25, 2024
@teh-cmc teh-cmc force-pushed the cmc/remove_instance_keys branch from 156fc4b to d21705d Compare April 25, 2024 17:08
@rerun-io rerun-io deleted a comment from github-actions bot Apr 25, 2024
@rerun-io rerun-io deleted a comment from github-actions bot Apr 25, 2024
@teh-cmc teh-cmc force-pushed the cmc/remove_instance_keys branch from d21705d to dced19b Compare April 25, 2024 17:14
@teh-cmc
Copy link
Member Author

teh-cmc commented Apr 25, 2024

@rerun-bot full-check

@teh-cmc teh-cmc marked this pull request as ready for review April 25, 2024 17:14
@github-actions GitHub Actions
Copy link

Started a full build: https://github.com/rerun-io/rerun/actions/runs/8836695206

emilk
emilk approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Amazing - hell of a job

crates/re_log_types/src/lib.rs Outdated Show resolved Hide resolved
crates/re_log_types/src/lib.rs Outdated Show resolved Hide resolved
@teh-cmc teh-cmc force-pushed the cmc/data_apis_16_non_cacheable_components branch from a0ff353 to 8779474 Compare April 26, 2024 10:47
Base automatically changed from cmc/data_apis_16_non_cacheable_components to main April 26, 2024 10:47
@teh-cmc teh-cmc force-pushed the cmc/remove_instance_keys branch from 105abd8 to ccd3eb6 Compare April 26, 2024 10:55
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Apr 26, 2024
@teh-cmc teh-cmc merged commit 893de2f into main Apr 26, 2024
38 of 40 checks passed
@teh-cmc teh-cmc deleted the cmc/remove_instance_keys branch April 26, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilk emilk emilk approved these changes

Assignees
No one assigned
Labels
🔩 data model include in changelog 🚀 performance Optimization, memory use, etc ⛃ re_datastore affects the datastore itself
Projects
None yet
Milestone
No milestone
2 participants
@teh-cmc @emilk