C++ ARIMA fixes and refactoring

[filipcacky]

Fixes #937. The code no longer segfaults in that scenario, but tries to allocate ~194 GB of memory and gets killed by oomkiller instead, would be probably nice to add an available memory check.

I had to bump cpp version to 20 in order to use std::span.

All i did was change types to size_t/uint32 where it made sense, added const where it was possible and I also added some asserts on preconditions.

I didn't find any tests, but i ran it before/after on the following configurations:

ARIMA(order=(6, 1, 0))
ARIMA(order=(1, 1, 0), seasonal_order=(1, 0, 0), season_length=steps_day)
ARIMA(order=(1, 0, 0), seasonal_order=(1, 0, 0), season_length=steps_day)
ARIMA(order=(1, 0, 0), seasonal_order=(1, 1, 0), season_length=steps_day)
ARIMA(order=(0, 0, 0), seasonal_order=(1, 0, 0), season_length=steps_day)
ARIMA(order=(0, 0, 0), seasonal_order=(1, 1, 0), season_length=steps_day)

and got the exact same results.

[CLAassistant]

All committers have signed the CLA.

[jmoralez]

Thanks a lot! Just a small question, do you know if using C++20 would cause problems for users on old MacOS systems? We publish wheels for MacOS 10.13, which is fairly old and I'm not sure if there could be runtime crashes due to something not being in the system's libc++

[filipcacky]

According to https://en.cppreference.com/w/cpp/compiler_support/20, std::span has been supported by apple clang since version 10.0.0 (no idea which MacOS version that corresponds to). It's an STL component, the specialized template gets baked in to the binary, so it should be fine as long as it builds.

It's not used in the library interface, if it was there could be ABI issues if the library and calling code were compiled with different versions of clang and stdc++ (program loads the dynamic library and calls a function that has std::span as one of its arguments).

I have no way to test it on MacOS 10.13, but I strongly doubt any issues could happen at runtime. With optimization, the entire std::span should get optimized away, leaving a raw pointer in its place.

[jmoralez]

Thanks a lot! Just some small comments.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codspeed-hq]

CodSpeed Performance Report

Merging #939 will not alter performance

_{Comparing filipcacky:feature/cpp_arima_cleanup (6d2af42) with main (8680a13)}

Summary

✅ 6 untouched benchmarks

[filipcacky]

I'm a bit confused about the failed tests on pip install, not really familiar with what is happening there. Is that something i should be concerned with?

[jmoralez]

Is that something i should be concerned with?

Don't worry, those were failing tests due to the float64 indices that were being generated. Everything looks good now, I just reverted a change that is made with newer versions of nbdev and was making the lint check fail, once the checks pass I'll merge this. Thanks a lot for your help!

If you're interested, the ETS model could also use some cleanup or if you want to migrate some of the other models that currently use numba (described in #753 (comment)) we'd welcome that.