-
Notifications
You must be signed in to change notification settings - Fork 631
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
[ENH] Support tuning any model and extend LSTMModel in docs to support multi-target datasets #1449
base: main
Are you sure you want to change the base?
Conversation
190589a
to
5fc682a
Compare
What's blocking this? |
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
I don't understand why the docs test fails (I haven't changed anything there) and why the Black test fails if I ran >> black --check .
All done! ✨ 🍰 ✨
54 files would be left unchanged. |
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1449 +/- ##
==========================================
- Coverage 90.19% 89.96% -0.23%
==========================================
Files 30 33 +3
Lines 4724 4985 +261
==========================================
+ Hits 4261 4485 +224
- Misses 463 500 +37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7245c00
to
622d979
Compare
Description
This PR fixes my own issues:
It allows to tune "any" model, generalizing the
optimize_hypetparameters
function fromTemporalFusionTransformer
model, and it also add a "better"LSTMModel
than the one shown in the documentation, which I tried to use on a multi-target dataset without success. My version does work on multi-target datasets, thanks to an extension toAutoRegressiveBaseModel
.My version of
AutoRegressiveBaseModel
inherits from the original to override a couple of methods. I should've probably just edited the original, but I'd like to hear your feedback before doing that. I'm also sure there was a simpler way forAutoRegressiveBaseModel
to work with multi-target data. When I tried to just run the Documentation's example ofLSTMModel
but on a multi-target dummy dataset, it just did not work out of the box.Checklist
pre-commit install
.To run hooks independent of commit, execute
pre-commit run --all-files
Make sure to have fun coding!