-
-
Notifications
You must be signed in to change notification settings - Fork 202
[Version 3] New heuristic for RAM #804
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
Conversation
For the record, here is the proposition of DeepSeek R1 :
Example Calculations:
Trade-offs:
This approach prioritizes accuracy for older systems while maintaining simplicity and avoiding reliance on admin-only data. |
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.
Copilot reviewed 41 out of 48 changed files in this pull request and generated 1 comment.
Files not reviewed (7)
- docs/.buildinfo: Language not supported
- docs/_sources/examples.rst.txt: Language not supported
- docs/_sources/methodology.rst.txt: Language not supported
- docs/_sources/parameters.rst.txt: Language not supported
- docs/edit/examples.rst: Language not supported
- docs/edit/methodology.rst: Language not supported
- docs/edit/parameters.rst: Language not supported
Comments suppressed due to low confidence (2)
docs/examples.html:172
- The word 'begginning' appears to be misspelled; consider changing it to 'beginning'.
You instantiate the tracker and call the <cite>start()</cite> method at the begginning of the Notebook.
codecarbon/output_methods/file.py:55
- Consider if the removal of the dropna call is intentional; without it, columns with only NaN values may persist in the CSV output.
new_df = new_df.dropna(axis=1, how="all")
4ac0901
to
90d6ec9
Compare
@@ -66,14 +66,17 @@ def is_rapl_available() -> bool: | |||
def is_psutil_available(): | |||
try: | |||
nice = psutil.cpu_times().nice | |||
if nice > 0.1: | |||
if nice > 0.0001: |
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.
Because on huge CPU, it could be above 0 but under 0.1.
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.
Merci Benoît !
@@ -61,7 +63,10 @@ class Energy: | |||
|
|||
@classmethod | |||
def from_power_and_time(cls, *, power: "Power", time: "Time") -> "Energy": | |||
return cls(kWh=power.kW * time.hours) | |||
assert isinstance(power.kW, float) |
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.
Quel est l'intérêt de l'assert ici ? ça ne risque pas de casser le programme à un endroit non désiré ?
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.
Cette endroit est la base du calcul, si jamais il n'y a pas la bonne unité il y a plein d'effets de bord.
Ca m'est arrivé lors du dev et j'ai mis du temps à trouver la source du problème. Je me suis dit qu'en le vérifiant ici, cela évitera des problèmes à l'avenir.
# Minimum 5W for x86 as requested (2 sticks at 2.5W) | ||
min_power = base_power_per_dimm * 2 | ||
|
||
# Estimate power based on DIMM count with decreasing marginal power per DIMM as count increases |
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.
ça pourrait être intéressant de faire des statistiques sur les setups les plus courants. Peut être via les données remontées par l'API
Breaking change in RAM power estimation.