Conversation
- Add get_carbon_intensity_timeseries_for_phase() to generate carbon intensity data at regular intervals throughout phases
- Add store_phase_carbon_intensity_metric() to store phase-specific carbon intensity as measurement metrics using existing tables
- Update phase_stats.py to generate and store carbon intensity timeseries per phase
- Replace single midpoint interpolation with average of timeseries for more accurate phase carbon calculations
- Rename interpolate_carbon_intensity to get_carbon_intensity_at_timestamp for clarity
- Store timeseries data using pattern: grid_carbon_intensity_phase / {location}_{index:03}_{phase_name}
- Improve logging messages and documentation
- Add comprehensive tests for new timeseries functionality
- Foundation for future energy×carbon temporal integration calculations
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
… measurement start and end times
|
Eco CI Output [RUN-ID: 18030396455]: 🌳 CO2 Data: Total cost of whole PR so far:
|
|||||||||||||||||||||||||||||||||||
|
Before I review this PR, can you please slim this a little. I personally feel that the comments in the functions add little to no value and make it very hard for me to read the code. This was ok in dependency_resolver as this is a stand alone component. In GMT I think it should not be there. Example: def get_carbon_intensity_history(self, location: str, start_time: str, end_time: str) -> List[Dict[str, Any]]:
"""
Fetch carbon intensity history from Elephant service.
Args:
location: Location code (e.g., "DE", "ES-IB-MA")
start_time: Start time in ISO 8601 format (e.g., "2025-09-22T10:50:00Z")
end_time: End time in ISO 8601 format (e.g., "2025-09-22T10:55:00Z")
Returns:
List of carbon intensity data points:
[{"location": "DE", "time": "2025-09-22T10:00:00Z", "carbon_intensity": 185.0}, ...]
Raises:
Exception: On any service error, network issue, or invalid response
"""
url = f"{self.base_url}/carbon-intensity/history"
params = {
'location': location,
'startTime': start_time,
'endTime': end_time,
'interpolate': 'true' # we also want to get data points that are adjacent to the requested time range, to be ensure we always get at least one data point
}
--- to be slimmed down to
def get_carbon_intensity_history(self, location: str, start_time: str, end_time: str) -> List[Dict[str, Any]]:
url = f"{self.base_url}/carbon-intensity/history"
params = {
'location': location, #Location code (e.g., "DE", "ES-IB-MA")
'startTime': start_time, #ISO 8601 format (e.g., "2025-09-22T10:50:00Z")
'endTime': end_time, #ISO 8601 format (e.g., "2025-09-22T10:55:00Z")
'interpolate': 'true' # we also want to get data points that are adjacent to the requested time range, to be ensure we always get at least one data point
}In this format it is very clear to me still what the function does and what kind of format the arguments have without having this big docs block.
Example: # Bulk insert measurement values
_bulk_insert_measurement_values(measurement_metric_id, value_timestamp_pairs)=> The function is already well named. No need for the comment to re-iterate.
|
|
I removed the docstrings and some comments.
No, I have not thought about making it a proper metrics provider. It seems that I have interpreted the following design guideline incorrectly:
If you want to have one metric provider that produces a metric called "grid_carbon_intensity" then you might have issues with the display name in the frontend (see
Done.
I changed the metric names to |
|
The test |
Resolves #1330
Documentation: green-coding-solutions/documentation#126
Open questions:
round()or store it as mg in the database (and show it as mg in the frontend)?system,global,container, etc. don't make sense here I think. Maybegridorzonefor the dynamic value, but for static? The metric name for the dynamic carbon intensity is currentlygrid_carbon_intensity_dynamic, therefore "dynamic" is displayed as the scope.dev-no-phase-statsis disabled)?Screenshots
The following two screenshots were taken from the synthetic test case
test_phase_stats_dynamic_grid_carbon_intensity.