Checklist for Refactoring CI/CD of sktime-dl

sktime / sktime-dl

DEPRECATED, now in sktime - companion package for deep learning based on TensorFlow

BSD 3-Clause "New" or "Revised" License

596 stars 79 forks source link

Checklist for Refactoring CI/CD of sktime-dl #88

Closed Riyabelle25 closed 8 months ago

Riyabelle25 commented 3 years ago

Checklist of suggested bug-fixes

[x] replace Travis with GitHub Actions
[ ] clean up build_tools/build.sh
[ ] use Miniconda GitHub Action
[ ] exclude TF version < 2
[ ] add pre-commit yaml config file and GitHub actions (similar to sktime)
[ ] rename master branch to main
[ ] replace maint_tools/linting.sh with pre-commit
[ ] replace stage: Deploy from azure-pipelines.yml with new release process similar to sktime
[ ] remove support for Python 3.6 (only run on 3.7 for now, once everything is set up we can add 3.8 and 3.9)

To Reproduce

Expected behavior

Additional context

Versions

Riyabelle25 commented 3 years ago

Hi all! We're updating the sktime-dl package, and these are some additional bug fixes that @mloning and I discussed. I have started working on this checklist, do add suggestions. Hopefully, we get sktime-dl back up and running soon!

jnrusson1 commented 3 years ago

Hi all. I've done some exploration into why the final unit test is failing. The MCDCNN model fails when used as an estimator in a forecast reduction algorithm but works when used as a regressor. This is because it will only work on nested dataframes and does not work on 3d numpy arrays.

@Riyabelle25 version of sktime-dl utils is converting the nested dataframe into (row,time,variable) format. Is sktime now operating with the format of (row,variable,time)?

The MCDCNN model is currently designed to work with the (row,time,variable) format which is why it's not working when given 3d numpy arrays. I'm wondering if this means that the unit tests need to be expanded to test with both nested input and with 3d input and check they are equivalent.

I'm also not sure if all of the sktime-dl algorithms are set up expecting (row,time,variable).

jnrusson1 commented 3 years ago

Quick update to the last comment. The standard deep learning conventions for tensorflow seem to be to have the channel last, so it makes more sense to do it in the way @Riyabelle25 has done it already. I think an extra function can be used to help handle 3d numpy of the format (n_batch , n_dim, n_time).

Riyabelle25 commented 3 years ago

Thanks, @jnrusson1 for shedding light on that pesky test :confounded: Perhaps like you said, adding another method in utils/data.py updating the newer TensorFlow conventions would solve this- will you be adding this in #87?

fkiraly commented 8 months ago

obsolete as estimators in sktime are now subject to programmatic tests in TestAllClassifiers and the standard CI.