Skip to content
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

Check memory unit #67

Open
ggael opened this issue Sep 20, 2022 · 3 comments
Open

Check memory unit #67

ggael opened this issue Sep 20, 2022 · 3 comments

Comments

@ggael
Copy link
Collaborator

ggael commented Sep 20, 2022

The memory attribute is expected to be a float number in GB. This means that 1) the GB must be removed, and 2) that the parsers have to guaranty that they parsed a number in the right unit, that is not the case yet.

@pabluk
Copy link
Collaborator

pabluk commented Oct 25, 2022

I was looking at this issue and it could be maybe closed, because for:

  1. all the memory values containing "GB" were fixed/removed on this commit
  2. test_read_format() is currently validating type conversion when reading values

Otherwise, maybe if the idea is to enforce strict floating point number format (e.g 4.0 instead of 4), in that case probably checking int conversion before float could allow to check that:

>>> int('4.0')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: '4.0'
>>> float('4.0')
4.0

and that change will impact all the float type fields of DeviceCarbonFootprintData.

Do you have any advice on this? if this issue is still valid I could add a test for that, fix all parsers that generate float values and updated CSV files.

@ggael
Copy link
Collaborator Author

ggael commented Oct 26, 2022

This issue was more about checking all parsers to make sure that they do verify that the number they read is in GB and make the conversion if it's not the case (AFAIR there are a few cases for which the reported unit is not in GB)

@pabluk
Copy link
Collaborator

pabluk commented Oct 26, 2022

Oh I see, thanks for the explanation.

Looking at the current data on boavizta-data-us.csv indeed, there are some suspicious memory values:

>>> import csv
>>> 
>>> with open('boavizta-data-us.csv', 'rt', encoding='utf-8') as data_file:
...     reader = csv.DictReader(data_file)
...     memory_values = [row['memory'] for row in reader if row['memory']]
... 
>>> unique_values = list(set(memory_values))
>>> unique_values.sort(key=float)
>>> unique_values
['3.0', '4.0', '6.0', '8.0', '8', '12.0', '16.0', '16', '32.0', '32', '48.0', '64.0', '64', '128', '256']

for example, 256GB of RAM on a server could be normal but not for a smartphone:

>>> import csv
>>> 
>>> with open('boavizta-data-us.csv', 'rt', encoding='utf-8') as data_file:
...     reader = csv.DictReader(data_file)
...     for row in reader:
...         if row['memory'] and float(row['memory']) >= 128:
...             print(row['manufacturer'], row['name'], row['subcategory'], row['memory'])
... 
Apple iPhone 11 128GB Smartphone 128
Apple iPhone 11 256GB Smartphone 256
Apple iPhone 12 128GB Smartphone 128
Apple iPhone 12 256GB Smartphone 256
Apple iPhone 8 256GB Smartphone 256
Apple iPhone SE - Gen 2 128GB Smartphone 128
Apple iPhone SE - Gen 2 256GB Smartphone 256
Dell PowerEdge R830 Server 256
Dell PowerEdge R840 Server 128
Dell PowerEdge R930 Server 256

it seems that Apple's parser is incorrectly filling the memory field with storage capacity from iPhones. Even if it isn't a unit/scale issue, the parser need to be reviewed.

pabluk added a commit that referenced this issue Oct 27, 2022
As reported on #67 some hard_drive values entered manually were
misplaced in the memory field.
AirLoren pushed a commit that referenced this issue Oct 28, 2022
* Fix memory value for several iPhone models

As reported on #67 some hard_drive values entered manually were
misplaced in the memory field.

* Fix screen_size value for several iPhone models

Several hard_drive values entered manually were misplaced in the
screen_size field.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants