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

Update Carbon Intensity Range to 0-1500 g co2eq/kWh #4789

Merged
merged 11 commits into from
Dec 7, 2022

Conversation

BStephenBB
Copy link
Contributor

Issue

Zone g co2eq/kWh values is exceeding the range defined in the frontend, see #4771

Description

Updates to the co2eq/kWh factors are causing the co2eq/kWh range to exceed the range defined by the frontend. This PR increases the range from 0-800 to 0-1500 co2eq/kWh (since the highest current observed range is 1439).

To do this I added an extra step and color to all the co2 scales in the theme file as discussed in #4771. I also increased the width of the ledged by a tad per @VIKTORVAV99 's suggestion. The new width revealed that the legend's text wasn't centered, so I added a line of css to center the text.

I'm wasn't fully sure how to go about picking the new colors and steps; let me know if I should use different ones. I arrived at the colors in this PR by taking the current colors and piping them into this Chroma.js based tool and then adding an extra step. The non-colorblind colors used the same color for the last 2 steps, so I continued to follow that pattern in this PR (was that intentional?). Also, for the non-colorblind colors, the green that was being used was darker than the yellow, so I started the new range with the yellow, but kept the original green as the 1st step in the final range.

The final ranges all have the same lightest and darkest colors as before, there's just more granularity now. Let me know if it would be better to make the endpoints of the range darker/lighter.

Preview

image

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the changes look fine code wise (except for maybe one missing change?) but it seems to have the unintended consequence of making all countries look "better" on the map than before.

@madsnedergaard what do you think about this change? Maybe it would be better to make the color range non linear using our existing colors and just add a pitch black (#000) as the 6th step?

web/src/helpers/themes.js Outdated Show resolved Hide resolved
@BStephenBB
Copy link
Contributor Author

it seems to have the unintended consequence of making all countries look "better" on the map than before

Great point. Using all black as the final step would work well for the main theme, but the colorblind theme's final color is already essentially black (#000010) so it wouldn't help as much there.

@madsnedergaard
Copy link
Member

Hey, we have discussed this internally and are leaning towards the idea of adding a pitch black step at the very end instead of changing all colors :)

But maybe still adjust all colors in color-blind theme to ensure it's usable?

steps: [0, 150, 600, 750, 800],
colors: ['#2AA364', '#F5EB4D', '#9E4229', '#381D02', '#381D02'],
steps: [0, 300, 600, 900, 1200, 1500],
colors: ['#2AA364', '#F5EB4D', '#C6983A', '#885022', '#381D02', '#381D02'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To follow up on my comment, here's an example of what I was trying to say 😄

How about something like this instead? Also seems like we shouldn't use the same color twice at the end, it only made sense before because of the 750/800 step being so close (don't know why it was like that actually).

Suggested change
colors: ['#2AA364', '#F5EB4D', '#C6983A', '#885022', '#381D02', '#381D02'],
colors: ['#2AA364', '#F5EB4D', '#B46C32', '#6B3015', '#2A1602', '#000000']

Screenshot 2022-11-22 at 22 59 32

@VIKTORVAV99
Copy link
Member

VIKTORVAV99 commented Nov 22, 2022

Hey, we have discussed this internally and are leaning towards the idea of adding a pitch black step at the very end instead of changing all colors :)

But maybe still adjust all colors in color-blind theme to ensure its usable?

Do we still want to update the steps, or do we want to go with a non-linear (current) approach?
Linear:

steps: [0, 300, 600, 900, 1200, 1500],
colors: ['#2AA364', '#F5EB4D', '#B46C32', '#6B3015', '#2A1602', '#000']

Linear scale image:
image

Non-linear

steps: [0, 150, 600, 750, 800, 1500],
colors: ['#2AA364', '#F5EB4D', '#B46C32', '#6B3015', '#2A1602', '#000']

Non-linear scale image:
image

The benefit of the non-linear option is that we would use the same colors for the same values as we use today with the exception of values over 800 g co2eq/kWh and keeping the established color representation.

On the other hand, it's not as intuitive and might look strange both to existing users but maybe even more so to new users.

@BStephenBB
Copy link
Contributor Author

Hey, we have discussed this internally and are leaning towards the idea of adding a pitch black step at the very end instead of changing all colors :)

But maybe still adjust all colors in color-blind theme to ensure it's usable?

Sounds good; updated the themes to use pitch black as the final step. I double checked the colorblind theme and it's still colorblind safe ✅

Do we still want to update the steps, or do we want to go with a non-linear (current) approach?

Happy to make this change too, will defer to other maintainers though, just let me know 🙂

Thanks for the patience and for making it so easy for new contributors to contribute! Also, should I squash all the intermediate commits before the final merge to keep the git history tidy?

@VIKTORVAV99
Copy link
Member

Thanks for the patience and for making it so easy for new contributors to contribute! Also, should I squash all the intermediate commits before the final merge to keep the git history tidy?

We want as many people as possible helping out with what they feel comfortable doing so thank you for taking the time to do this (even with our discussion in here).

There is no need to squash the commits as we use squash merge to merge all PRs and it will do so automatically.

@madsnedergaard
Copy link
Member

Thanks for the patience and for making it so easy for new contributors to contribute!

Super happy to hear that you find it a good experience!
And thanks for making this contribution in the first place, can't wait to see it in action 🙏

Do we still want to update the steps, or do we want to go with a non-linear (current) approach?

That's a great and tricky question 😄

linear

This would make the difference between very low and somewhat low seem smaller - e.g. France at 40g and Denmark at 150g. It would also make Denmark as an example a lot more green than before (color is not 100% correct though):

Screenshot 2022-11-23 at 23 02 25

non-linear

It's does indeed look a bit strange, and the very high numbers are not noticeably different.

A mix of both?

Maybe we should do something in between and keep the first non-linear steps, but "smooth" it out in the higher end like this?

[0, 150, 600, 900, 1200, 1500]

That would solve the problems mentioned above I think.
A downside is that countries at around 900g would seem a bit lighter than before, but I think that's an okay trade-off :)

@VIKTORVAV99
Copy link
Member

VIKTORVAV99 commented Nov 23, 2022

Thanks for the patience and for making it so easy for new contributors to contribute!

Super happy to hear that you find it a good experience!
And thanks for making this contribution in the first place, can't wait to see it in action 🙏

Do we still want to update the steps, or do we want to go with a non-linear (current) approach?

That's a great and tricky question 😄

linear

This would make the difference between very low and somewhat low seem smaller - e.g. France at 40g and Denmark at 150g. It would also make Denmark as an example a lot more green than before (color is not 100% correct though):

Screenshot 2022-11-23 at 23 02 25

non-linear

It's does indeed look a bit strange, and the very high numbers are not noticeably different.

A mix of both?

Maybe we should do something in between and keep the first non-linear steps, but "smooth" it out in the higher end like this?

[0, 150, 600, 900, 1200, 1500]

That would solve the problems mentioned above I think.
A downside is that countries at around 900g would seem a bit lighter than before, but I think that's an okay trade-off :)

It could be a good tradeoff but I'm hesitant to changing our existing colors, granted the new co2eq values already changes the equation a bit but i think we should keep the comparability with any old images/screenshots as high as possible.

It's also worth taking into account that it's only Faroe Islands that uses the full scale (as of now) most other countries potential max CO2 intensity is just above 1100 g co2eq/kWh.
So maybe a better range would be [0, 150, 600, 800, 1100, 1500]?

But I just had an idea that we could explore further at a later date. And that would be basing these colors on the climate goals? Green = under 1°C global warming, yellow = 2°C global warming, red = 3°C global warming, etc (numbers and breakpoint are just examples).

@BStephenBB
Copy link
Contributor Author

Checking back in on this; sounds like we are choosing between:

  • [0, 300, 600, 900, 1200, 1500] (equal spacing, what's currently in the PR)
  • [0, 150, 600, 900, 1200, 1500] (a mix of linear and nonlinear)
  • [0, 150, 600, 800, 1100, 1500] (try to keep colors consistent)

Has anyone thought more about which option to go with?

It's also worth taking into account that it's only Faroe Islands that uses the full scale

Yeah anecdotally it seems pretty common for several regions to go above 800 (the current scale's max), but very rare for anything to be >1000. It's unfortunate that the new scale goes all the way up to 1500. Not really sure what a great solution is though, since as the original issue points out sometimes there is a place that can get into the 1400s.

@madsnedergaard
Copy link
Member

Thanks for checking back on it!
Without testing and seeing it in practice, I think @VIKTORVAV99's proposal ([0, 150, 600, 800, 1100, 1500]) sounds like the best approach - so I vote for going with that :)

@VIKTORVAV99
Copy link
Member

Here is a screenshot of the dark range using the existing colors with the addition of #000 however it's still using the old mock data (we should probably update that) with the old emission factors.

image

It's a little missrepresentative for Germany and Poland but France, Denmark and Norway more or less have the same color as the current theme.

@madsnedergaard
Copy link
Member

If you have it open, can you share a visual comparison of the different scales?

@VIKTORVAV99
Copy link
Member

If you have it open, can you share a visual comparison of the different scales?

Sure!

New scale:
Consumption:
image
Production:
image

Old scale:
Consumption:
image
Production:
image

The change here is really small (which we want) but if you look at Estonia for example you can see a subtle difference.
With the new carbon intensity figures it should be a bigger difference for the zones with a high carbon intensity.

@madsnedergaard
Copy link
Member

LGTM, let's try it! 🥳

@BStephenBB
Copy link
Contributor Author

Sounds good, updated the branch to use [0, 150, 600, 800, 1100, 1500] 👍

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it got lost in the conversation but the screenshots I shared used the old colors for the "normal" themes with the addition of #000 as the 6th step.

I added suggestions to restore that but beyond that everything looks great!

This can be merged as soon as the above change is implemented. Thanks for your patience with this PR.

web/src/helpers/themes.js Outdated Show resolved Hide resolved
web/src/helpers/themes.js Outdated Show resolved Hide resolved
BStephenBB and others added 2 commits December 7, 2022 13:19
Co-authored-by: Viktor Andersson <30777521+VIKTORVAV99@users.noreply.github.com>
Co-authored-by: Viktor Andersson <30777521+VIKTORVAV99@users.noreply.github.com>
@BStephenBB
Copy link
Contributor Author

Oops, missed that part. Thanks for adding the suggestions!

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

Thanks for bearing with us on this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants