From 6606d19606fe8ec24d3d0202600c863ab6e61636 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 15:26:14 +0100 Subject: [PATCH 01/12] add support for `rerun rrd compare --unordered` --- crates/top/rerun/src/commands/rrd/compare.rs | 48 +++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/crates/top/rerun/src/commands/rrd/compare.rs b/crates/top/rerun/src/commands/rrd/compare.rs index 3faa45e09b2f..12372b792cd7 100644 --- a/crates/top/rerun/src/commands/rrd/compare.rs +++ b/crates/top/rerun/src/commands/rrd/compare.rs @@ -6,6 +6,8 @@ use std::{ use anyhow::Context as _; use itertools::{izip, Itertools}; +use re_chunk::Chunk; + // --- #[derive(Debug, Clone, clap::Parser)] @@ -13,6 +15,15 @@ pub struct CompareCommand { path_to_rrd1: String, path_to_rrd2: String, + /// If specified, the comparison will focus purely on semantics, ignoring order. + /// + /// The Rerun data model is itself unordered, and because many of the internal pipelines are + /// asynchronous by nature, it is very easy to end up with semantically identical, but + /// differently ordered data. + /// In most cases, the distinction is irrelevant, and you'd rather the comparison succeeds. + #[clap(long, default_value_t = false)] + unordered: bool, + /// If specified, dumps both .rrd files as tables. #[clap(long, default_value_t = false)] full_dump: bool, @@ -27,6 +38,7 @@ impl CompareCommand { let Self { path_to_rrd1, path_to_rrd2, + unordered, full_dump, } = self; @@ -64,17 +76,31 @@ impl CompareCommand { re_format::format_uint(chunks2.len()), ); - for (chunk1, chunk2) in izip!(chunks1, chunks2) { - anyhow::ensure!( - re_chunk::Chunk::are_similar(&chunk1, &chunk2), - "Chunks do not match:\n{}", - similar_asserts::SimpleDiff::from_str( - &format!("{chunk1}"), - &format!("{chunk2}"), - "got", - "expected", - ), - ); + if *unordered { + let mut chunks2: Vec>> = chunks2.into_iter().map(Some).collect_vec(); + 'outer: for chunk1 in chunks1 { + for chunk2 in chunks2.iter_mut().filter(|c| c.is_some()) { + #[allow(clippy::unwrap_used)] + if re_chunk::Chunk::are_similar(&chunk1, chunk2.as_ref().unwrap()) { + *chunk2 = None; + continue 'outer; + } + } + anyhow::bail!("Couldn't find a match for the following chunk:\n{chunk1}"); + } + } else { + for (chunk1, chunk2) in izip!(chunks1, chunks2) { + anyhow::ensure!( + re_chunk::Chunk::are_similar(&chunk1, &chunk2), + "Chunks do not match:\n{}", + similar_asserts::SimpleDiff::from_str( + &format!("{chunk1}"), + &format!("{chunk2}"), + "got", + "expected", + ), + ); + } } re_log::debug!("{path_to_rrd1:?} and {path_to_rrd2:?} are similar enough."); From 8e70d44ba2fccc793e5b73fe13391089957cec41 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 15:27:36 +0100 Subject: [PATCH 02/12] make snippet roundtrips unordered --- scripts/roundtrip_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/roundtrip_utils.py b/scripts/roundtrip_utils.py index 85775b5a238f..5dbbd47211d0 100644 --- a/scripts/roundtrip_utils.py +++ b/scripts/roundtrip_utils.py @@ -58,7 +58,7 @@ def roundtrip_env(*, save_path: str | None = None) -> dict[str, str]: def run_comparison(rrd0_path: str, rrd1_path: str, full_dump: bool) -> None: - cmd = ["rerun", "rrd", "compare"] + cmd = ["rerun", "rrd", "compare", "--unordered"] if full_dump: cmd += ["--full-dump"] cmd += [rrd0_path, rrd1_path] From 08c89a2f31a3fb5bf131833a4748bbcce0f7e821 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:01:12 +0100 Subject: [PATCH 03/12] video_auto_frames --- docs/snippets/snippets.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 83fa22a045d4..53519c04ebff 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -286,11 +286,6 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/video_auto_frames" = [ # This mixes `log` and `send_columns`. Since `log` is suspect to delays by the batcher, this test gets flaky. - "cpp", - "py", - "rust", -] # `$config_dir` will be replaced with the absolute path of `docs/snippets`. # Note that the snippet comparison tool will automatically run `/tests/assets/download_test_assets.py` before running the snippets. From ad23cea81a915a5585e0ab7103c32aa147a9c5d2 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:05:27 +0100 Subject: [PATCH 04/12] image_send_columns --- docs/snippets/all/archetypes/image_send_columns.cpp | 2 +- docs/snippets/all/archetypes/image_send_columns.py | 4 ++-- docs/snippets/all/archetypes/image_send_columns.rs | 10 ++-------- docs/snippets/snippets.toml | 4 ---- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/docs/snippets/all/archetypes/image_send_columns.cpp b/docs/snippets/all/archetypes/image_send_columns.cpp index f179bdfefc9b..d7c6d40fde1b 100644 --- a/docs/snippets/all/archetypes/image_send_columns.cpp +++ b/docs/snippets/all/archetypes/image_send_columns.cpp @@ -32,7 +32,7 @@ int main() { rerun::ColorModel::RGB, rerun::ChannelDatatype::U8 ); - rec.log_static("images", rerun::borrow(&format), rerun::Image::IndicatorComponent()); + rec.log_static("images", rerun::Image::update_fields().with_format(format)); // Split up the image data into several components referencing the underlying data. const size_t image_size_in_bytes = width * height * 3; diff --git a/docs/snippets/all/archetypes/image_send_columns.py b/docs/snippets/all/archetypes/image_send_columns.py index 3359fe5d7d72..c735eafb780c 100644 --- a/docs/snippets/all/archetypes/image_send_columns.py +++ b/docs/snippets/all/archetypes/image_send_columns.py @@ -16,8 +16,8 @@ images[t, 50:150, (t * 10) : (t * 10 + 100), 1] = 255 # Log the ImageFormat and indicator once, as static. -format_static = rr.components.ImageFormat(width=width, height=height, color_model="RGB", channel_datatype="U8") -rr.send_columns("images", indexes=[], columns=rr.Image.columns(format=format_static)) +format = rr.components.ImageFormat(width=width, height=height, color_model="RGB", channel_datatype="U8") +rr.log("images", rr.Image.from_fields(format=format), static=True) # Send all images at once. rr.send_columns( diff --git a/docs/snippets/all/archetypes/image_send_columns.rs b/docs/snippets/all/archetypes/image_send_columns.rs index a157ced26562..cb4983e272b1 100644 --- a/docs/snippets/all/archetypes/image_send_columns.rs +++ b/docs/snippets/all/archetypes/image_send_columns.rs @@ -20,15 +20,9 @@ fn main() -> Result<(), Box> { .fill(255); } - // Send the ImageFormat and indicator once, as static. + // Log the ImageFormat and indicator once, as static. let format = rerun::components::ImageFormat::rgb8([width as _, height as _]); - rec.send_columns( - "images", - [], - rerun::Image::update_fields() - .with_format(format) - .columns_of_unit_batches()?, - )?; + rec.log_static("images", &rerun::Image::update_fields().with_format(format))?; // Split up the image data into several components referencing the underlying data. let image_size_in_bytes = width * height * 3; diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 53519c04ebff..0443b442b0ea 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -277,10 +277,6 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/image_send_columns" = [ # This mixes `log` and `send_columns`. Since `log` is suspect to delays by the batcher, this test gets flaky. - "cpp", - "rust", -] "archetypes/mesh3d_instancing" = [ # TODO(#3235): Slight floating point differences in deg to rad conversion. "cpp", "py", From a4c6f44ea5d0586dee119fed734ad910f482d5a3 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:36:05 +0100 Subject: [PATCH 05/12] arrows3d_simple --- docs/snippets/snippets.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 0443b442b0ea..ba73e33e7920 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -198,10 +198,8 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/arrows3d_simple" = [ # TODO(#3206): examples use different RNGs - "cpp", +"archetypes/arrows3d_simple" = [ # Python uses doubles "py", - "rust", ] "archetypes/asset3d_out_of_tree" = [ # float issues since calculation is done slightly differently (also, Python uses doubles) "cpp", From 99a0cb9baf9c2d2123b5a887adfcead12ed92c9d Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:36:19 +0100 Subject: [PATCH 06/12] asset3d_out_of_tree --- docs/snippets/snippets.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index ba73e33e7920..f53961980178 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -201,11 +201,6 @@ quick_start = [ # These examples don't have exactly the same implementation. "archetypes/arrows3d_simple" = [ # Python uses doubles "py", ] -"archetypes/asset3d_out_of_tree" = [ # float issues since calculation is done slightly differently (also, Python uses doubles) - "cpp", - "py", - "rust", -] "archetypes/bar_chart" = [ # On Windows this logs f64 instead of u64 unless a numpy array with explicit type is used. "py", ] From fc93cf156d7ddf6b9d429f4f5c329385a399fac8 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:41:23 +0100 Subject: [PATCH 07/12] mesh3d_partial_updates --- docs/snippets/all/archetypes/mesh3d_partial_updates.cpp | 5 ++++- docs/snippets/snippets.toml | 6 ++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/snippets/all/archetypes/mesh3d_partial_updates.cpp b/docs/snippets/all/archetypes/mesh3d_partial_updates.cpp index c23404909fbc..9ca7143fc2c5 100644 --- a/docs/snippets/all/archetypes/mesh3d_partial_updates.cpp +++ b/docs/snippets/all/archetypes/mesh3d_partial_updates.cpp @@ -43,6 +43,9 @@ int main() { mul_pos(factor, vertex_positions[1]), mul_pos(factor, vertex_positions[2]), }; - rec.log("triangle", modified_vertex_positions); + rec.log( + "triangle", + rerun::Mesh3D::update_fields().with_vertex_positions(modified_vertex_positions) + ); } } diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index f53961980178..a3b2eae22a85 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -214,11 +214,9 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/mesh3d_partial_updates" = [ # TODO(andreas): use new partial update api - "cpp", +"archetypes/mesh3d_partial_updates" = [ # Python uses doubles "py", - "rust", -] # float precision issues +] "archetypes/pinhole_simple" = [ # TODO(#3206): examples use different RNGs "cpp", "py", From 58ba45839e9307bd9d503b09fad2f2dd2c58f35d Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:46:15 +0100 Subject: [PATCH 08/12] series_point_style --- docs/snippets/snippets.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index a3b2eae22a85..52ed8f201ea1 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -240,11 +240,6 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/series_point_style" = [ # TODO(#5116): trigonometric functions have slightly different outcomes - "cpp", - "py", - "rust", -] "archetypes/series_line_style" = [ # TODO(#5116):trigonometric functions have slightly different outcomes "cpp", "py", From bd1aa417653fc05e8ce69f4b6825ef31edfee3aa Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:46:52 +0100 Subject: [PATCH 09/12] series_line_style --- docs/snippets/snippets.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 52ed8f201ea1..9d8ebe7ec10b 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -240,11 +240,6 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/series_line_style" = [ # TODO(#5116):trigonometric functions have slightly different outcomes - "cpp", - "py", - "rust", -] "archetypes/text_log_integration" = [ # The entity path will differ because the Rust code is part of a library "cpp", "py", From 561c0ccd5094379dac890f01f57875931c44316a Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:48:01 +0100 Subject: [PATCH 10/12] text_log_integration --- docs/snippets/snippets.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 9d8ebe7ec10b..9d8ab21a1f46 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -240,7 +240,7 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/text_log_integration" = [ # The entity path will differ because the Rust code is part of a library +"archetypes/text_log_integration" = [ # The entity path will differ because the integrations work differently "cpp", "py", "rust", From d28cd814b6e9c231ac96540856ba88eaa5f412e5 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 28 Jan 2025 16:52:14 +0100 Subject: [PATCH 11/12] mesh3d_instancing --- docs/snippets/snippets.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 9d8ab21a1f46..28fff444a310 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -258,10 +258,8 @@ quick_start = [ # These examples don't have exactly the same implementation. "py", "rust", ] -"archetypes/mesh3d_instancing" = [ # TODO(#3235): Slight floating point differences in deg to rad conversion. - "cpp", +"archetypes/mesh3d_instancing" = [ # Python uses doubles "py", - "rust", ] # `$config_dir` will be replaced with the absolute path of `docs/snippets`. From a1f57ed2c969b15e264b284449ec1c761845ba07 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Wed, 29 Jan 2025 09:33:58 +0100 Subject: [PATCH 12/12] fallback to ordered in case of failure for better error message --- crates/top/rerun/src/commands/rrd/compare.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/top/rerun/src/commands/rrd/compare.rs b/crates/top/rerun/src/commands/rrd/compare.rs index 12372b792cd7..df5cf55f479f 100644 --- a/crates/top/rerun/src/commands/rrd/compare.rs +++ b/crates/top/rerun/src/commands/rrd/compare.rs @@ -76,19 +76,24 @@ impl CompareCommand { re_format::format_uint(chunks2.len()), ); + let mut unordered_failed = false; if *unordered { - let mut chunks2: Vec>> = chunks2.into_iter().map(Some).collect_vec(); - 'outer: for chunk1 in chunks1 { - for chunk2 in chunks2.iter_mut().filter(|c| c.is_some()) { + let mut chunks2_opt: Vec>> = + chunks2.clone().into_iter().map(Some).collect_vec(); + 'outer: for chunk1 in &chunks1 { + for chunk2 in chunks2_opt.iter_mut().filter(|c| c.is_some()) { #[allow(clippy::unwrap_used)] - if re_chunk::Chunk::are_similar(&chunk1, chunk2.as_ref().unwrap()) { + if re_chunk::Chunk::are_similar(chunk1, chunk2.as_ref().unwrap()) { *chunk2 = None; continue 'outer; } } - anyhow::bail!("Couldn't find a match for the following chunk:\n{chunk1}"); + unordered_failed = true; + break; } - } else { + } + + if !*unordered || unordered_failed { for (chunk1, chunk2) in izip!(chunks1, chunks2) { anyhow::ensure!( re_chunk::Chunk::are_similar(&chunk1, &chunk2),