Fix for Float Overflow resulting in VM Crash

[lkarthee]

Before

Series.from_list([1_055_028_234_663_852_885_981_170_418_348_451_692_544.0], dtype: {:f, 32})
thread '<unnamed>' panicked at src/series.rs:86:1:
called `Result::unwrap()` on an `Err` value: {error, badarg}
stack backtrace:
libunwind: stepWithCompactEncoding - invalid compact unwind encoding
Abort trap: 6

After

Explorer.Series.from_list([1_055_028_234_663_852_885_981_170_418_348_451_692_544.0], dtype: {:f, 32})
** (ArgumentError) argument error
    (explorer 0.9.0-dev) Explorer.PolarsBackend.Native.s_from_list_f32("", [1.0550282346638529e39])
    (explorer 0.9.0-dev) lib/explorer/polars_backend/series.ex:24: Explorer.PolarsBackend.Series.from_list/2
    (explorer 0.9.0-dev) lib/explorer/series.ex:417: Explorer.Series.from_list/2
    iex:1: (file)

[philss]

Just a question, but LGTM!

[philss]

Is it necessary to use Rustler's NifResult here? Do you know if the Result<ExSeries, ExplorerError> could be used instead?

[lkarthee]

Two reasons:

• Result<ExSeries, ExplorerError> will unwrap to {:ok, series} or {:error, _} instead of series. This will break our existing code as others return ex_series.
• NifResult<ExSeries> will unwrap to series or raises ArgumentError/ErlangError. NifResult<T> is more than type alias of Result<T, Error>. It is special as it can Return(T) or Raise(T) (an ErlangError) or BadArg (an ArgumentError). ArgumentError is what other from_list raise - will help us in writing a wrapper for descriptive errors (Jose's suggestion in the other pr).

[philss]

Ok, thanks!

[lkarthee]

should this be panic or ErlangError with message ? Let me know if I have to revert to panic!

[josevalim]

It doesn't matter much honestly. As long as it comes out properly when we show it to the user.

[lkarthee]

There are other places in series.rs file where panic! is being used - won't it crash VM ? Is it the case that validations are done in Elixir, so these panic clause won't ever trigger ?

[philss]

won't it crash VM?

It won´t. Rustler catches the panics for us :)

Is it the case that validations are done in Elixir, so these panic clause won't ever trigger ?

yeah, normally we add validations so it won´t trigger the panics.

[lkarthee]

Thank you

[lkarthee]

@philss one last question, does rustler handle nifpanics differently in dev vs release builds? Looks like title of PR is valid only for dev/test environments.

[philss]

@lkarthee no, it shouldn´t be different. I guess the problem is related to the lib that catches the panics.

libunwind: stepWithCompactEncoding - invalid compact unwind encoding

I'm not sure, but looks like this is some encoding problem there.

[lkarthee]

Two reasons:

• Result<ExSeries, ExplorerError> will unwrap to {:ok, series} or {:error, _} instead of series. This will break our existing code as others return ex_series.
• NifResult<ExSeries> will unwrap to series or raises ArgumentError/ErlangError. NifResult<T> is more than type alias of Result<T, Error>. It is special as it can Return(T) or Raise(T) (an ErlangError) or BadArg (an ArgumentError). ArgumentError is what other from_list raise - will help us in writing a wrapper for descriptive errors (Jose's suggestion in the other pr).

[josevalim]

💚 💙 💜 💛 ❤️