-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implemented alternative experiment tracking functionality #102
Conversation
…andrainst/coral into feature/experiment_tracking
Im on the errors now |
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.
Really like the idea, and great execution! 😃
I've got a couple of small comments, and there's a style check that is currently failing. You can install pre-commit hooks with make install-pre-commit
, which will ensure that your style checks pass before committing. You can also run make clean
to do it manually.
To make the code base slightly more lean, I would suggest that we keep the directory structure flat, with a single module, experiment_tracking.py
, which then contains both the load_extracking_setup
function as well as the MLFlowSetup
and WandbSetup
classes, since they're all quite small. The abstract ExtrackingSetup
can be moved to the data_models
module, as that's where all the other abstract classes are.
@saattrupdan Ready for final review |
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.
LGTM!
No description provided.