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.

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

Crash on arrow2 when using component schema with optional color + optional tensordata #4971

Open
Wumpf opened this issue Jan 30, 2024 · 3 comments
Labels
🏹 arrow concerning arrow codegen/idl 💣 crash crash, deadlock/freeze, do-no-start

Comments

@Wumpf
Copy link
Member

Wumpf commented Jan 30, 2024

This issue blocks:

the new schema in question:

/// Material properties of a mesh.
table Material (
  "attr.rust.derive": "PartialEq"
) {
  /// Optional color multiplier.
  albedo_factor: rerun.datatypes.Rgba32 (nullable, order: 100);

  /// Optional albedo texture.
  ///
  /// Used with `vertex_texcoords` on `Mesh3D`.
  /// Currently supports only sRGB(A) textures, ignoring alpha.
  /// (meaning that the tensor must have 3 or 4 channels and use the `u8` format)
  albedo_texture: rerun.datatypes.TensorData (nullable, order: 200);
}

Repro steps:

  • checkout 92bf597 (this is an older version of andreas/log-textured-meshes)
  • run cargo run -p raw_mesh

panic! Callstack:

2024-01-30 15:31:49.168 Python[99564:11373452] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to /var/folders/sj/mhn0_knj71139prvc0zzmsh40000gn/T/org.python.python.savedState
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/result.rs:1652:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/result.rs:1077:23
   4: re_arrow2::array::struct_::StructArray::new
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/array/struct_/mod.rs:124:9
   5: re_arrow2::array::growable::structure::GrowableStruct::to
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/array/growable/structure.rs:72:9
   6: <re_arrow2::array::growable::structure::GrowableStruct as re_arrow2::array::growable::Growable>::as_box
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/array/growable/structure.rs:124:18
   7: re_arrow2::array::growable::structure::GrowableStruct::to::{{closure}}
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/array/growable/structure.rs:70:53
   8: core::iter::adapters::map::map_try_fold::{{closure}}
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/iter/adapters/map.rs:91:28
   9: core::iter::traits::iterator::Iterator::try_fold
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/iter/traits/iterator.rs:2461:21
  10: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::try_fold
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/iter/adapters/map.rs:117:9
  11: <I as alloc::vec::in_place_collect::SpecInPlaceCollect<T,I>>::collect_in_place
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/alloc/src/vec/in_place_collect.rs:258:13
  12: alloc::vec::in_place_collect::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/alloc/src/vec/in_place_collect.rs:182:28
  13: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/alloc/src/vec/mod.rs:2749:9
  14: core::iter::traits::iterator::Iterator::collect
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/iter/traits/iterator.rs:2053:9
  15: re_arrow2::array::growable::structure::GrowableStruct::to
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/array/growable/structure.rs:70:65
  16: <re_arrow2::array::growable::structure::GrowableStruct as re_arrow2::array::growable::Growable>::as_box
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/array/growable/structure.rs:124:18
  17: re_arrow2::compute::concatenate::concatenate
             at /Users/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/re_arrow2-0.17.4/src/compute/concatenate.rs:45:8
  18: re_log_types::data_table::DataTable::serialize_data_column
             at ./crates/re_log_types/src/data_table.rs:856:17
  19: re_log_types::data_table::DataTable::serialize_data_columns
             at ./crates/re_log_types/src/data_table.rs:777:39
  20: re_log_types::data_table::DataTable::serialize
             at ./crates/re_log_types/src/data_table.rs:617:47
  21: re_log_types::data_table::DataTable::to_arrow_msg
             at ./crates/re_log_types/src/data_table.rs:1066:31
  22: re_sdk::recording_stream::forwarding_thread
             at ./crates/re_sdk/src/recording_stream.rs:1065:43
@Wumpf Wumpf added 🏹 arrow concerning arrow 💣 crash crash, deadlock/freeze, do-no-start labels Jan 30, 2024
@emilk
Copy link
Member

emilk commented Jan 30, 2024

And the message is the panic is:

OutOfSpec("The children must have an equal number of values. However, the values at index 1 have a length of 0, which is different from values at index 0, 38.")

from

https://github.com/rerun-io/re_arrow2/blob/33a32000001df800e4840d92c33b03e7007311e1/src/array/struct_/mod.rs#L89-L92

Sounds like we're maybe only adding albedo_factor, but not albedo_texture?

@emilk
Copy link
Member

emilk commented Jan 30, 2024

I think this could be a bug in our codegen

@jleibs
Copy link
Member

jleibs commented Feb 1, 2024

I think this could be a bug in our codegen

This really seems limited to concatenation pathway in arrow. For example, with RERUN_FLUSH_NUM_ROWS=1, everything works.

I'm fairly certain the offending code is here:
https://github.com/jorgecarleitao/arrow2/blob/main/src/array/growable/union.rs#L94-L99

This stems from the fact that unions have no way of representing null since they don't have their own validity map, but in order to be able to batch a nullable struct, there needs to be a way of representing possibly null-elements in the nested union child-array.

At a minimum extend_validity should panic here and just declare this doesn't work with a much less cryptic error message like: "Can't grow a struct containing a nulled union." However, I think the more correct thing is to just push a null element into the first child array and extend the type-array accordingly.

@Wumpf Wumpf removed their assignment Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏹 arrow concerning arrow codegen/idl 💣 crash crash, deadlock/freeze, do-no-start
Projects
None yet
Development

No branches or pull requests

3 participants