search

electricitymaps / bloom-contrib

Making carbon footprint data available to everyone.
https://www.bloomclimate.com
MIT License
435 stars 104 forks source link

Added car.js #258

Closed ayakoaohara closed 4 years ago

ayakoaohara commented 4 years ago

I added co2eq/car.js and modified co2eq/transportation.js To make this pull request small, car.js now only contains functions with generic car models (specified by the size and the engine type). The data is from https://docs.google.com/spreadsheets/d/1f1j9EeVn9czOZBJKLXvgPwnmldakxuJ7/edit#gid=439900614

Specific car models with brand name and year(data from Climobil) must be added to this .js file later.

ayakoaohara commented 4 years ago

From Issue #246

martincollignon commented 4 years ago

This is great. type probably needs to be changed to engineType. @FelixDQ given that 1) we have another input type for fuel (or electricity), how do you think we could structure so we can take that as possible variable (the source that @ayakoaohara is using has both options possible :-)), so someone can input the gCO2/km/passenger with or without fuel 2) how would you structure the file to add the brand/model/year combination? There is also the options with this one to get the gCO2/km/passenger with or without fuel from the sources.

This is just so @ayakoaohara can keep on doing her amazing work!!

FelixDQ commented 4 years ago

This is great. type probably needs to be changed to engineType. @FelixDQ given that

  1. we have another input type for fuel (or electricity), how do you think we could structure so we can take that as possible variable (the source that @ayakoaohara is using has both options possible :-)), so someone can input the gCO2/km/passenger with or without fuel
  2. how would you structure the file to add the brand/model/year combination? There is also the options with this one to get the gCO2/km/passenger with or without fuel from the sources.

This is just so @ayakoaohara can keep on doing her amazing work!!

  1. We can add a parameter footprintIgnoreFuel that the model could use to calculate the footprint, but let's do that in a separate PR.
  2. The brand/model/year combinations should be supplied in a json file similar to airports.json which car.js can use to calculate the footprint.
ayakoaohara commented 4 years ago

Thank you for reviewing!

In this pull request, I can write a car model (size, engineType, brand, id, activityType, dateTime, durationHours, distanceKilometers, transportationMode, and pathLonLats). I can go ahead and delete CAR_CATEGORIES currently in car.js (maybe rename the file to co2eq/car/index.js), and create co2eq/car/cars.json that contains the car categories (only generic categories for now). This way, cars categories with brand/model/year combinations can be added to this .json file later.

After that, in a separate pull request, I could go ahead and write footprintIgnoreFuel to co2eq/car/index.js.

Let me know how this sounds.

FelixDQ commented 4 years ago

That sounds great!

ayakoaohara commented 4 years ago

I wrote co2eq/car/cars.json with generic car categories, and I have a question about where to find additional gCO2/km carbon intensity data.

For when engineType is present & size is missing, there are categories defined only by the engineType. For when engineType is missing (unkown) & size is present, there are categories defined only by the size. For when both are missing, there is a category called "average" For when both inputs are available, there are categories defined by both size and engineType.

Although gCO2/km is available for categories defined by engineType and categories by both size and engineType, I couldn't find gCO2/km for an "average" car and for a car only defined by its size on https://docs.google.com/spreadsheets/d/1f1j9EeVn9czOZBJKLXvgPwnmldakxuJ7/edit#gid=1278401347. Is there a suggestion on where to find the data for these?

martincollignon commented 4 years ago

Hi @ayakoaohara , thanks for your continued investigation! The work you're doing is amazing.

Hopefully this helps: https://docs.google.com/spreadsheets/d/1f1j9EeVn9czOZBJKLXvgPwnmldakxuJ7/edit#gid=159468561 ?

ayakoaohara commented 4 years ago

Thank you for the helpful link. To clarify, should data on kgCO2e be used, or kgCO2? The numbers that I used in the initial version of car.js in this pull request was kgCO2, so I will change them to kgCO2e if that is ideal!

martincollignon commented 4 years ago

KgCO2e is what we're looking for! 😊

On Fri, Nov 29, 2019, 02:36 ayakoaohara notifications@github.com wrote:

Thank you for the helpful link. To clarify, should data on kgCO2e be used, or kgCO2? The numbers that I used in the initial version of car.js in this pull request was kgCO2, so I will change them to kgCO2e if that is ideal!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tmrowco/tmrowapp-contrib/pull/258?email_source=notifications&email_token=AAT333SX7XQCI7VFDJYRNETQWBWYRA5CNFSM4JR657WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFNU2DY#issuecomment-559631631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAT333RN6NSTZPB4SFXUMYLQWBWYRANCNFSM4JR657WA .

ayakoaohara commented 4 years ago

Sorry for the delayed responses, and thank you for answering my questions:) I added a directory co2eq/car, and added index.js and cars.json (previously co2eq/car.js). The data in cars.json is kgCO2/km from https://docs.google.com/spreadsheets/d/1f1j9EeVn9czOZBJKLXvgPwnmldakxuJ7/edit#gid=1278401347 index.js I also modified carbonIntensityGeneric() (co2eq/car/index.js) so it would be consistent with keys defined in cars.json. (I am planning to write a wrapper function so that it can calculate kgCO2/km with missing parameters (size, engineType, or brand)).

ayakoaohara commented 4 years ago

Thank you for the review! I just updated the files so they would align with your comments. If you could check them again, that would be great.

martincollignon commented 4 years ago

Hi @ayakoaohara ! I'll let @corradio check the code :-)

Last thing: because of it seems that small, medium, large categorisation is ill-defined (a developer or a user would not clearly understand what each category encompasses), we think it's wiser to use the Euro Car Segments (I've input the values here: https://docs.google.com/spreadsheets/d/1f1j9EeVn9czOZBJKLXvgPwnmldakxuJ7/edit#gid=227529294 ), as it can be linked back to individual models (if we need to), and is more widely used by the EU and manufacturer associations.

We would also use euroCarSegment instead of size, where the values are the Segment letter.

Hopefully that makes sense to you as well!

ayakoaohara commented 4 years ago

Great! I will change the code to use euroCarSegments, rather than size. Thank you for reviewing!

ayakoaohara commented 4 years ago

I have a question on which engine types to include in the code. Currently, the list in co2eq/car/cars.json has diesel, petrol, hybrid, cng, lpg as engine types. Should I also include "Plug-in Hybrid Electric Vehicle" and "Battery Electric Vehicle" as inputs for engine types in cars.json?

martincollignon commented 4 years ago

Yes, let's include them as well!

ayakoaohara commented 4 years ago

Thank you for the reviews! In co2eq/car/cars.json, I erased size as category and added euroCarSegment. In addition, "plugInHybridElectric" and "BatteryHybrid" are included as engineTypes.

corradio commented 4 years ago

That's amazing @ayakoaohara ! We're almost there! As the euroCarSegment takes different values, and because we want to avoid typos when developers will create activities, we tend to define the possible values in this file: https://github.com/tmrowco/tmrowapp-contrib/blob/master/definitions.js I therefore suggest constants for both euroCarSegment and engineType:

const EUROCARSEGMENT_DUALPURPOSE4X4 = 'dualPurpose4x4';
...
const ENGINETYPE_DIESEL = 'diesel';

Also it'd be great to add a comment that says that those values are used in co2eq/car/cars.json. This ensures that we can change the constants later on.

Question for you @martincollignon : for euroCarSegment values, should we use descriptions (i.e. dualPurpose4x4) or should we use the letters (A ... as defined in https://en.wikipedia.org/wiki/Euro_Car_Segment)? What do you think is the most intuitive for a developer that knows about those segment?

martincollignon commented 4 years ago

Let's use A,B, etc. Also easier to avoid spelling errors.

On Wed, Dec 4, 2019, 10:08 Olivier Corradi notifications@github.com wrote:

That's amazing @ayakoaohara https://github.com/ayakoaohara ! We're almost there! As the euroCarSegment takes different values, and because we want to avoid typos when developers will create activities, we tend to define the possible values in this file: https://github.com/tmrowco/tmrowapp-contrib/blob/master/definitions.js I therefore suggest adding a few constants for both euroCarSegment and engineType:

const EUROCARSEGMENT_DUALPURPOSE4X4 = 'dualPurpose4x4';...const ENGINETYPE_DIESEL = 'diesel';

This ensure that we can change the constants later on. Question for you @martincollignon https://github.com/martincollignon : for euroCarSegment values, should we use descriptions (i.e. dualPurpose4x4) or should we use the letters (A ... as defined in https://en.wikipedia.org/wiki/Euro_Car_Segment)? What do you think is the most intuitive for a developer that knows about those segment?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tmrowco/tmrowapp-contrib/pull/258?email_source=notifications&email_token=AAT333URPELK2MHT6HL7ML3QW5XPTA5CNFSM4JR657WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF4IBKA#issuecomment-561545384, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAT333T3WETWCT7XOLU5KVTQW5XPTANCNFSM4JR657WA .

corradio commented 4 years ago

great, then let's adjust the constants as well

On Wed, Dec 4, 2019 at 10:47 AM martincollignon notifications@github.com wrote:

Let's use A,B, etc. Also easier to avoid spelling errors.

On Wed, Dec 4, 2019, 10:08 Olivier Corradi notifications@github.com wrote:

That's amazing @ayakoaohara https://github.com/ayakoaohara ! We're almost there! As the euroCarSegment takes different values, and because we want to avoid typos when developers will create activities, we tend to define the possible values in this file: https://github.com/tmrowco/tmrowapp-contrib/blob/master/definitions.js I therefore suggest adding a few constants for both euroCarSegment and engineType:

const EUROCARSEGMENT_DUALPURPOSE4X4 = 'dualPurpose4x4';...const ENGINETYPE_DIESEL = 'diesel';

This ensure that we can change the constants later on. Question for you @martincollignon https://github.com/martincollignon : for euroCarSegment values, should we use descriptions (i.e. dualPurpose4x4) or should we use the letters (A ... as defined in https://en.wikipedia.org/wiki/Euro_Car_Segment)? What do you think is the most intuitive for a developer that knows about those segment?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/tmrowco/tmrowapp-contrib/pull/258?email_source=notifications&email_token=AAT333URPELK2MHT6HL7ML3QW5XPTA5CNFSM4JR657WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF4IBKA#issuecomment-561545384 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAT333T3WETWCT7XOLU5KVTQW5XPTANCNFSM4JR657WA

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tmrowco/tmrowapp-contrib/pull/258?email_source=notifications&email_token=AAMUIKBOH7K3NWPJ5D262UDQW54BVA5CNFSM4JR657WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF4LWYY#issuecomment-561560419, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMUIKGKMUFJOUIFXFRPJ3DQW54BVANCNFSM4JR657WA .

--

Olivier Corradi

Founder, CEO

https://www.linkedin.com/in/oliviercorradi https://twitter.com/corradio

tmrow.com

ayakoaohara commented 4 years ago

Sorry about this, I selected "update branch" by mistake.

ayakoaohara commented 4 years ago

Thank you again for reviewing the code in detail. In definitions.js, I added new definitions, Euro car segments (EUROCARSEGMENT_A, EUROCARSEGMENT_B,...) and engine types. In addition, values corresponding to "euroCarSegment" in co2eq/car/cars.json now has letters A, B,.., rather than descriptors (mini, dualPurpose4x4,...). I also edited co2eq/index.js so that the function will refer to "euroCarSegment" field, rather than the old "size" field.

corradio commented 4 years ago

Sweet! Now to the last part: deciding when the model should run. @FelixDQ has prepare the following PR (https://github.com/tmrowco/tmrowapp-contrib/pull/262) which allows each model to express when they have sufficient data to run. Would you be able to make the same changes to this particular model? Then we should be good to go!

Thank you again for your patience and contribution. This means a lot to us

ayakoaohara commented 4 years ago

I'm very glad to receive feedback as well! The modelCanRun() that I added in co2eq/car/index.js tests for the input's activity type and transportation mode. Let me know if modification is needed.

martincollignon commented 4 years ago

Amazing @ayakoaohara ! This was great. Let us know if you want to take a stab on the climobil data ;-)

ayakoaohara commented 4 years ago

Thank you for the detailed feedback! I would be glad to work on the climobil data. I will open a pull request, once I write a code for it.