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

Monitor subprocess #431

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

jennydaman
Copy link

Resolves #382

This PR adds functionality to the codecarbon monitor command.

When you run codecarbon monitor, the previous behavior is kept the same.

When you run codecarbon monitor -- some positional --arguments -lah, the positional arguments are ran as a subprocess and monitored with the option tracking_mode="process".

Copy link
Contributor

@benoit-cty benoit-cty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
Thanks for the contribution. Can you add a documentation in https://github.com/mlco2/codecarbon/blob/master/docs/edit/usage.rst ?

@@ -73,17 +89,18 @@ def init():
@click.option(
"--api/--no-api", default=True, help="Choose to call Code Carbon API or not. (yes)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to myself : document the "--no-api" parameter and remove the beta mention for the API.

measure_power_secs=measure_power_secs,
api_call_interval=api_call_interval,
save_to_api=api,
tracking_mode="process",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it works with process. It need a test with a process that consume RAM to see the difference with machine because process does not work for GPU and CPU yet.

@SaboniAmine
Copy link
Collaborator

@benoit-cty besides documentation, is there any other blocker on this PR ?

@benoit-cty
Copy link
Contributor

@benoit-cty besides documentation, is there any other blocker on this PR ?

It has to be tested to ensure it works as expected.

@itamarst
Copy link

Was just looking for this feature! Hope it can get merged eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants