-
-
Notifications
You must be signed in to change notification settings - Fork 237
Monitor subprocess #431
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
base: master
Are you sure you want to change the base?
Monitor subprocess #431
Conversation
benoit-cty
left a comment
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.
Hello,
Thanks for the contribution. Can you add a documentation in https://github.com/mlco2/codecarbon/blob/master/docs/edit/usage.rst ?
| help="Number of measures before calling API. (30).", | ||
| ) | ||
| @click.option( | ||
| "--api/--no-api", default=True, help="Choose to call Code Carbon API or not. (yes)" |
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.
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", |
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.
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.
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.
I confirm launching it like this does not work sadly. For instrumenting a process, it needs to be in the same process it is instrumenting. It will be cool in the future to do something like "instrument process 1234" passing the id.
For now I suggest to have codecarbon monitor to measure whole machine only but start/stop when the process starts/stops. See #1004 .
What do you think @jennydaman ?
|
@benoit-cty besides documentation, is there any other blocker on this PR ? |
It has to be tested to ensure it works as expected. |
|
Was just looking for this feature! Hope it can get merged eventually. |
|
Thanks for this PR @jennydaman, it is a good idea! interested in applying those changes? Sorry I started the change in #1004 without knowing of the existence of this branch. My bad :( We have moved the cli from click to typer, this is why you see this many conflicts in this PR. |
|
Hi @inimaz worry not about "stealing" work! This PR is a bit stale, I'll keep it in mind but it might take me some weekends to get to. |
Resolves #382
This PR adds functionality to the
codecarbon monitorcommand.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 optiontracking_mode="process".