-
-
Notifications
You must be signed in to change notification settings - Fork 134
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
FEDOT integration #563
FEDOT integration #563
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking the initiative to contribute FEDOT to the automl benchmark! 🎉
First impressions are good, though I left a few minor remarks that should be easily to resolve. I will run some local tests next week as well and provided that all works as expected it looks good to me (after the suggested changes are processed) 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing it myself right now and it seems to work, but code-wise there are a few things that don't work and/or could be approved. Left some more suggestions, sorry for doing this in multiple parts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a nitpick, but also this problem still remains. Just ping me when you think it is ready for review again :)
@PGijsbers I added the changes according to the remaining comments. Also, I rebased my branch to main. |
Thanks! I’ll have a look later this week |
That seems to do the trick. Thanks for your effort! 🎉 |
Thank you, we will investigate it and try to reproduce. |
with Timer() as predict: | ||
predictions = fedot.predict(features=dataset.test.X) | ||
probabilities = None | ||
if is_classification: | ||
probabilities = fedot.predict_proba(features=dataset.test.X, probs_for_all_classes=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment: You should time only one of these predict calls, not both, otherwise your predict time will be twice as long as it should be.
I'd recommend copying the way it is done in the AutoGluon framework exec.py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D’oh! Good catch! Opened an issue for it: #581.
Hi!
I've added integration with FEDOT AutoML framework (https://github.com/aimclub/FEDOT) for tabular tasks.
The local runs were evaluated successfully.
We also plan to add integration for time series forecasting in the next PR.