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

Compute power only in start - stop window #378

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Compute power only in start - stop window #378

merged 1 commit into from
Sep 19, 2024

Conversation

pgmpablo157321
Copy link
Contributor

No description provided.

@pgmpablo157321 pgmpablo157321 requested review from a team as code owners July 31, 2024 22:37
Copy link

github-actions bot commented Jul 31, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

conversion_eff = logline.value["value"]

# If power stop is not found, set it to the latest power reading
if not power_stop_found:
Copy link
Contributor

Choose a reason for hiding this comment

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

is it expected that submission may not have the power-stop line? if not, maybe we should raise an error or log a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not expected. But it it happens, we can take the last power measurement as the stop. I updated it to log a warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# If power stop is not found, set it to the first power reading
if not power_stop_found:
power_stop = last_timestamp
print("WARNING: Power stop not found, taking last_timestamp as the power measurement stop")

for logline in loglines:
if logline.key == "power_measurement_start":
power_start = logline.timestamp
prev_timestamp = logline.timestamp
if logline.key == "power_reading":
agg_power += (logline.value['value'] * (logline.timestamp - prev_timestamp))
power_start_found = True
Copy link
Contributor

@xyhuang xyhuang Aug 20, 2024

Choose a reason for hiding this comment

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

if the power-start line is not found in the log, should we raise an error or log a warning? i guess it might cause an inaccurate calculation if we assumed 0 start time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should rase an error. Just updated the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# If power start is not found, raise an error
if not power_start_found:
raise ValueError("Power start timestamp not found")

@pgmpablo157321 pgmpablo157321 force-pushed the compute_power branch 4 times, most recently from f990bc8 to 92c325c Compare September 18, 2024 20:12
@xyhuang xyhuang merged commit c401ff0 into master Sep 19, 2024
1 check passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants