-
Notifications
You must be signed in to change notification settings - Fork 25
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
Moving GMT to microjoules #932
base: main
Are you sure you want to change the base?
Conversation
Eco-CI Output:
❌ CO2 Data: |
…esolution to phase_stats
…Reworked transformation of values and units
…ergy; added new tests for SR
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.
47 file(s) reviewed, 39 comment(s)
Edit PR Review Bot Settings | Greptile
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.
45 file(s) reviewed, 15 comment(s)
Edit PR Review Bot Settings | Greptile
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.
11 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
case 'centi°C': | ||
return [transformIfNotNull(value, 100), unit.substr(5)]; | ||
case 'Hz': | ||
return [transformIfNotNull(value, 1_000_000_000), `G${unit}`]; |
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.
logic: Hz conversion adds unit without stripping old unit first - will result in 'GHz' instead of just 'GHz'
markline = { | ||
precision: 4, // generally annoying that precision is by default 2. Wrong AVG if values are smaller than 0.001 and no autoscaling! | ||
data: [ {type: "average",label: {formatter: "AVG\n(selection):\n{c}"}}] |
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.
style: Consider using a constant for the precision value of 4 to make it more maintainable
@greptileai
Greptile Summary
This PR implements a significant database schema change and standardization of energy measurements to use microjoules (uJ) instead of millijoules (mJ) across the Green Metrics Tool.
measurement_metrics
andmeasurement_values
with new indexes instructure.sql
and migration scriptphase_stats
table for improved precision trackingThe changes look technically sound but require careful validation of unit conversions and data migration. The PR improves precision through standardized microjoule measurements while maintaining backward compatibility.