-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add calculation-period to reporting elements #279
Add calculation-period to reporting elements #279
Conversation
We discussed about this change on Green-Software-Foundation#123 . Add workload-start-time and calculation-period to SCI reporting elements as optional fields. It helps us to understand the impact of start times and processing times of workloads on SCI.
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 but requires potentially to consider the unit of time to be ms
.
@@ -22,6 +22,8 @@ The following list contains the REQUIRED and OPTIONAL data elements to be report | |||
| Software or Product Version | metadata/entity-version | MUST | Text | | | |||
| SCI Specification Version | metadata/sci-version | MUST | Numeric | The version of the SCI specification against which this report is being made. | | |||
| Date of Calculation | metadata/calculation-date | MUST | Date | Following a format described in [RFC3339], [a subset](https://ijmacd.github.io/rfc3339-iso8601/) of ISO 8601 | | |||
| Start date of the workload | metadata/workload-start-time | MAY | Timestamp | Following a format described in [RFC3339], [a subset](https://ijmacd.github.io/rfc3339-iso8601/) of ISO 8601 | | |||
| Processing period for calculation | metadata/calculation-period | MAY | Numeric | Time taken in seconds to complete the process defined by `R`. | |
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.
Should the time be in ms
since some tasks / processes might be much more granular than taking seconds?
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've updated to use ms instead of sec.
I thought it may be suitable to use sec here because the task period could be long (e.g. ML job). However we should also consider short-term task (e.g. transaction). So I accepted the suggestion from @atg-abhishek .
WG: we need to think more carefully about the appendix overall before we address specific issues/questions. |
@Henry-WattTime |
See issue #283 for resolution of this PR. |
We discussed about this change on #123 . This PR is based on this comment.
Add
workload-start-time
andcalculation-period
to SCI reporting elements as optional fields.It helps us to understand the impact of start times and processing times of workloads on SCI.
We can analyze, and feedback to run the job more efficiency with this PR. So I hope merge this PR to v1.0.