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

NIF panic with list of struct #1011

Open
maennchen opened this issue Oct 29, 2024 · 3 comments
Open

NIF panic with list of struct #1011

maennchen opened this issue Oct 29, 2024 · 3 comments

Comments

@maennchen
Copy link
Contributor

Code

Mix.install([{:explorer, "~> 0.10.0"}])

name_dtype = {"names",
{:list,
 {:struct,
  [
    {"language", :string},
    {"name", :string},
    {"transliteration", :category},
    {"type", :category}
  ]}}}

[
  %{names: []},
  %{names: [%{name: "CABK", type: "acronym", language: nil, transliteration: "none"}]}
]
|> Explorer.DataFrame.new(dtypes: [name_dtype])
|> dbg

Expected

A working Dataframe

Actual

thread '<unnamed>' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/polars-arrow-0.43.1/src/array/binview/mod.rs:327:9:
assertion failed: i < self.len()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread '<unnamed>' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/polars-arrow-0.43.1/src/array/binview/mod.rs:327:9:
assertion failed: i < self.len()
#Inspect.Error<
  got ErlangError with message:

      """
      Erlang error: :nif_panicked
      """

  while inspecting:

      %{
        data: %Explorer.PolarsBackend.DataFrame{
          resource: #Reference<0.3840168779.3188326401.44015>
        },
        remote: nil,
        names: ["names"],
        __struct__: Explorer.DataFrame,
        dtypes: %{
          "names" => {:list,
           {:struct,
            [
              {"language", :string},
              {"name", :string},
              {"transliteration", :category},
              {"type", :category}
            ]}}
        },
        groups: []
      }

  Stacktrace:

    (explorer 0.10.0) Explorer.PolarsBackend.Native.s_to_list(#Explorer.PolarsBackend.Series<
  #Reference<0.3840168779.3188326416.41949>
>)
    (explorer 0.10.0) lib/explorer/polars_backend/shared.ex:24: Explorer.PolarsBackend.Shared.apply_series/3
    (explorer 0.10.0) lib/explorer/backend/data_frame.ex:324: anonymous fn/3 in Explorer.Backend.DataFrame.build_cols_algebra/3
    (elixir 1.17.3) lib/enum.ex:1703: Enum."-map/2-lists^map/1-1-"/2
    (explorer 0.10.0) lib/explorer/backend/data_frame.ex:283: Explorer.Backend.DataFrame.inspect/5
    (explorer 0.10.0) lib/explorer/data_frame.ex:6308: Inspect.Explorer.DataFrame.inspect/2
    (elixir 1.17.3) lib/inspect/algebra.ex:347: Inspect.Algebra.to_doc/2
    (elixir 1.17.3) lib/io.ex:481: IO.inspect/3

>

Note: This error happens while inspecting the result. If I however pass it on to Explorer.DataFrame.dump_ndjson/1, I also get a segmentation fault.

[1]    79707 segmentation fault (core dumped)  elixir bug.exs

Context

In production, the error looks slightly different, but I'm unable to reproduce the exact error without providing the confidential information contained. I can however provide the error without stacktrace:

thread '<unnamed>' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/polars-arrow-0.43.1/src/array/binview/mod.rs:327:9:
                                                                                                                                assertion failed: i < self.len()
[info] Sent 500 in 9ms
[error] ** (ErlangError) Erlang error: :nif_panicked
    (explorer 0.10.0) Explorer.PolarsBackend.Native.s_to_list(#Explorer.PolarsBackend.Series<
  shape: (527,)
  Series: 'names' [list[struct[4]]]
  [
        null
        null
        null
        []
        []
        …
        null
        null
        null
        null
        null
  ]
>)

Since the part with assertion failed: i < self.len() is the same, I think the provided reproduction should represent the error sufficiently.

@billylanchantin
Copy link
Contributor

Hi @maennchen! We appreciate you putting our struct dtype logic through its paces :P I'll take a look at this today or tomorrow.

Related, I want to add a property test like this one but for DataFrame.new:

https://github.com/elixir-explorer/explorer/blob/main/test/explorer/series/inferred_dtype_property_test.exs

The generators would be similar, but the goal would be to see if we get a panic. Hopefully such a test would catch more bugs like these up front.

@billylanchantin
Copy link
Contributor

billylanchantin commented Nov 23, 2024

Progress so far

I've boiled down the problem to this:

dtypes = [{"col", {:list, {:struct, [{"field", :category}]}}}]
content = %{"field" => "example"}
works = [[content], []]
fails = [[], [content]]
DF.new([{"col", works}], dtypes: dtypes) |> dbg #=> works
DF.new([{"col", fails}], dtypes: dtypes) |> dbg #=> fails with panic

The actual panic occurs here:

AnyValue::Categorical(idx, mapping, _) => Ok(mapping.get(idx).encode(env)),

The Categorical's internal string mapping is empty, so mapping.get(idx) fails no matter what idx is.

But the real problem stems from how we built the series originally:

Series::new(name.into(), lists).cast(&dtype).map_err(|err| {
let message = format!("from_list/2 cannot create series of lists: {err:?}");
Error::RaiseTerm(Box::new(message))
})

Somehow, the result of .cast(&dtype) depends on the order of lists. The fact that [[content], []] works but [[], [content]] panics proves that.

This seems like a bug in Polars to me. But maybe .cast() doesn't work like I expect it to, and we need to be providing more type hints to ensure a deterministic output.


EDIT: Managed to reproduce this in Python. I think this is a Polars bug.

>>> import polars as pl

>>> dtype = pl.datatypes.List(pl.datatypes.Struct({"field": pl.datatypes.Categorical()}))

>>> pl.DataFrame({"col": [[{"field": "example"}], []]}, schema={"col": dtype})
shape: (2, 1)
┌─────────────────┐
│ col             │
│ ---             │
│ list[struct[1]] │
╞═════════════════╡
│ [{"example"}]   │
│ []              │
└─────────────────┘

>>> pl.DataFrame({"col": [[], [{"field": "example"}]]}, schema={"col": dtype})
thread '<unnamed>' panicked at /Users/runner/work/polars/polars/crates/polars-arrow/src/array/binview/mod.rs:308:9:
assertion failed: i < self.len()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/path/to/.venv/lib/python3.12/site-packages/polars/dataframe/frame.py", line 1149, in __repr__
    return self.__str__()
           ^^^^^^^^^^^^^^
  File "/path/to/.venv/lib/python3.12/site-packages/polars/dataframe/frame.py", line 1146, in __str__
    return self._df.as_str()
           ^^^^^^^^^^^^^^^^^
pyo3_runtime.PanicException: assertion failed: i < self.len()

@billylanchantin
Copy link
Contributor

billylanchantin commented Nov 23, 2024

billylanchantin added a commit that referenced this issue Nov 28, 2024
If we exclude `:category`, we can avoid #1011.
Note: there may still be issues with `:category`
but we won't be able to find them until #1011 is
resolved.
billylanchantin added a commit that referenced this issue Nov 28, 2024
* Allow specifying the dtype generator directly

Before we could only specify the leaf part of the
dtype tree generator. This change lets us
alternately specify the whole thing.

* Remove all `:skip` tags

If we exclude `:category`, we can avoid #1011.
Note: there may still be issues with `:category`
but we won't be able to find them until #1011 is
resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants