Conversation
|
Initial comments:
|
AbyAbraham21
left a comment
There was a problem hiding this comment.
Calculations look right. Some comments to address regarding offline mode.
simvue/run.py
Outdated
| self._emissions_tracker: OfflineSimvueEmissionsTracker | None = ( | ||
| OfflineSimvueEmissionsTracker( | ||
| "simvue", self, self._emission_metrics_interval | ||
| if not (_co2_intensity := self._user_config.eco.co2_intensity): |
There was a problem hiding this comment.
I think it should probably not require this, and should do the same as CC where Simvue comes with some default file which it can read from (that we make sure to update each PyPI release). That way the user just needs to provide a ISO code, not a value.
We could also make it so that the sender makes a call to the carbon intensity data every (eg) hour, and records it in the same file which online mode produces.
There was a problem hiding this comment.
I would argue in doing this we are then held accountable for this value.
| elif enable_emission_metrics is False and self._emissions_tracker: | ||
| self._error("Cannot disable emissions tracker once it has been started") | ||
| elif enable_emission_metrics is False and self._emissions_monitor: | ||
| self._error("Cannot disable emissions monitor once it has been started") |
There was a problem hiding this comment.
Actually this is a remnant of CC when this was not possible due to CC starting a server of sorts.
There was a problem hiding this comment.
Cannot disable because once the loop is running you cannot modify it
|
Final general comment - I strongly think that we should be adding in contributions from GPUs into this initial version of this if not too much work. Other stuff like RAM etc doesnt matter so much as it'd be roughly constant between runs, but if you did a run on a GPU cluster and a run on a CPU cluster, our emissions metrics comparison between the two would basically just be lies :/ |
|
Would suggest making emissions and resource metrics interval the same, and then doing something like: In run.py, in |
cd904dd to
01f8c0b
Compare
ace04f0 to
ff41785
Compare
simvue/config/parameters.py
Outdated
|
|
||
|
|
||
| class MetricsSpecifications(pydantic.BaseModel): | ||
| resources_metrics_interval: pydantic.PositiveInt | None = -1 |
There was a problem hiding this comment.
I would call this 'system_metrics_interval' as it now applies to both resources and emissions metrics
simvue/eco/emissions_monitor.py
Outdated
| ) | ||
|
|
||
| if not isinstance(kwargs.get("thermal_design_power_per_gpu"), float): | ||
| kwargs["thermal_design_power_per_gpu"] = 80.0 |
3dcf88d to
bbe4b4f
Compare
Ecoclient
Issue: #751
Python Version(s) Tested: 3.13
Operating System(s): Ubuntu 24.10
Documentation PR: https://github.com/simvue-io/docs/pull/61
📝 Summary
Replaces CodeCarbon as the source of emission metrics giving measures for CO2 emissions and energy consumption.
🔄 Changes
EmissionsTrackerfromsimvue.Run.ecosubmodule containing:APIClient: Class for connecting to CO2 Signal API.CO2Monitor: Class for calculating values based on CPU usage and current CO2 intensity.CO2Monitorintosimvue.Runand attached metric gathering to heartbeat (similar to resource metrics).✔️ Checklist