-
Notifications
You must be signed in to change notification settings - Fork 276
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
feat: enable codecarbon by default #1428
feat: enable codecarbon by default #1428
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.
Nice, thanks! Let's get the tests passing as well.
It seems that the emission tracker fails due to multiple workers, as it works fine when I run it without |
Since this is technically breaking I will move it to v2.0.0 |
Based one this issue, it seems like we can* allow multiple runs, but the tracking will be somewhat skewed. It won't matter in the tests, but will for sure affect real runs if multiple runs are done on the same machine. I think we would also add a line in README and/or a warning to let users know. |
Sounds like this can be merged into v2.0.0? |
Yes, just missing a line in README and/or a warning in the comment above. |
@isaac-chung I added logging message |
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.
Thanks!
Also added some clarity around the variable as we flip enable/disable, true/false.
This PR enables code carbon by default. Closes #1427
Checklist
make test
.make lint
.