-
Notifications
You must be signed in to change notification settings - Fork 50
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
Refactor interfaces #187
Refactor interfaces #187
Conversation
…nction of strategies
… `run` This simplifies the strategies since they do not need to deal with the runner environment. Instead, this is handled inside `tune_kernel` in `interface.py`.
…m its constructor
…o the strategies
…l_tuner into refactor_interface
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Some tests are failing, we need to check them. |
I'll run a bunch of tests on this branch on the DAS. If everything works I'll run the examples. If this also works, I guess we can merge the pull request back into master. |
… we do for constant. Can throw exception if not implemented.
All checks are passing here, I have 4 failures in tests on the DAS. So we finish checking and can merge next week. |
All failing tests are due to Numpy 1.24 not being supported yet. @stijnh is looking into it for future proofing on our side (scikit-learn also needs to support Numpy 1.24, they do not do it properly yet). |
I plan to check all the examples on Tuesday the 28th of March, to be sure we did not break anything, and after that run |
…ner into refactor_interface
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
The examples seem to work fine with this branch, some errors spotted are not directly related to changes here, so I believe this can be merged. |
I agree! |
Looks good to me! |
With this pull request we want to structurally, at least at first, refactor some of KT's code to allow developers of components such as observers, strategies, backends, and so on to more easily extend KT.
The discussion for this PL is in issue #177
As of today the PR is not ready for merging.