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.

Sign up for GitHub

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

Jump to bottom

Issue? Current implementation gives EcoIndex between -5 and 94 instead of 0 to 99 #61

Closed
Olivier-Descout opened this issue Aug 18, 2022 · 4 comments · Fixed by #63
Closed

Issue? Current implementation gives EcoIndex between -5 and 94 instead of 0 to 99 #61

Olivier-Descout opened this issue Aug 18, 2022 · 4 comments · Fixed by #63

Comments

@Olivier-Descout
Copy link

Hi,
I suspect there is an issue (or an inconsistency against the Ecoindex specification) in the current EcoIndex implementation located in the file ecoIndex.js.

As it is currently computed, EcoIndex value is between -5 and 94 rather than between 0 and 99 :

  • The smallest possible HTML page (<html></html> containing 3 DOM objects (page + empty header + empty body), generating 1 HTTP request and being 13-byte long) gets an EcoIndex equal to 94.00 (if rounded to 2 digits after the decimal point).
  • The theorically-worst HTML page (594,601 DOM objects; 3,920 HTTP requests; 223,213 kilobytes) gets a EcoIndex equal to -5.

Rachel Pellin's EcoIndex simulator (https://rachelwe.github.io/Simulateur-ecoindex/), which uses ecoIndex.js current implementation, helps highligting this behaviour:

  • the returned EcoIndex for a (theorical) HTML page with 0 DOM element, 0 HTTP request, 0-byte long gives an EcoIndex of 95 instead of 100.
  • the returned EcoIndex for the smallest HTML page described above if 94 instead of 99.
  • the returned EcoIndex for the worst-possible HTML page is -5 instead of 0.
  • if you compute the EcoIndex while using the worst value for one parameter and zero for the two others, the returned EcoIndex is lower than expected: for instance, the combination of 594,601 DOM objects, 0 HTTP requests and 0 kB gives an EcoIndex of 47.5 instead of the expected 50.

In my opinion, the issue lies in the calculateIndex function, which should return:
return ((i - 1) + (value - quantiles[i - 1]) / (quantiles[i] - quantiles[i - 1]));
and
return quantiles.length - 1;
instead of:
return (i + (value - quantiles[i - 1]) / (quantiles[i] - quantiles[i - 1]));
and
return quantiles.length;

If such a change were to be made, the grade computation would obviously have to be also altered, by adding 5 to every threshold:
case score >= 80: return 'A'; // instead of 75
case score >= 70: return 'B'; // instead of 65
...
case score >= 10: return 'F'; // instead of 5

Best Regards,
Olivier Descout
@didierfred
Copy link
Collaborator

didierfred commented Aug 26, 2022 •
edited
Loading

Yes you are right , following a pull request , i worked on a correction regarding this topic a year ago and never merged it : 350ca5a

I will have a look

@bordage
Copy link

bordage commented Aug 26, 2022

Thanks Olivier (and Rachel) for this point. IT's obvious we should implent this patch asap. The good news is that this will have a negligeable side effect to already computed EcoIndex evaluation.

@didierfred didierfred linked a pull request Aug 27, 2022 that will close this issue
Adapt ecoindex calcul (set scale between 0 and 100) #63
Merged
@didierfred
Copy link
Collaborator

The patch is integrated in release 3.1.0 available on chrome web store and firefox add ons

@Olivier-Descout
Copy link
Author

Olivier-Descout commented Sep 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adapt ecoindex calcul (set scale between 0 and 100) cnumr/GreenIT-Analysis
3 participants
@didierfred @bordage @Olivier-Descout