Skip to content
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

Add the possibility to declare a framework as abstract #276

Merged
merged 2 commits into from
Apr 6, 2021

Conversation

sebhrusen
Copy link
Collaborator

This is mainly useful to define a "template" framework from which children could extend to inherit all its properties without wanting to be able to run the template itself.

A typical use-case is MLPLan that comes with 2 training engines.

@Innixma
Copy link
Collaborator

Innixma commented Mar 31, 2021

Would it also make sense long-term to convert the def run(dataset, config): to some kind of class FrameworkRunner?

A lot of the data loading code is shared between all of the frameworks, and they can be viewed as individual ML models that take data in and generate output aka output = model.fit(data, **kwargs). This would simplify the work for frameworks to have slightly modified variants of their exec.py, and would allow for an AbstractFrameworkRunner.

@sebhrusen
Copy link
Collaborator Author

sebhrusen commented Mar 31, 2021

@Innixma That's a good idea.
I agree that the run.py is too monolithic currently.
A class would allow to structure this and improve code reusability, sth like:

class FrameworkRunner:
    def __init__(self, config, dataset): pass
    def prepare_data(self): pass
    def fit(self, …): pass
    def predict(self, …): pass
    def get_result(self): pass

Still need to figure out the right API, but it probably makes sense as a future improvement.

@sebhrusen
Copy link
Collaborator Author

#279

@sebhrusen sebhrusen merged commit 06ec3a3 into master Apr 6, 2021
@sebhrusen sebhrusen deleted the improv/abstract_framework_def branch April 6, 2021 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants