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

Rename anthroEmissions #116

Closed
wants to merge 1 commit into from

Conversation

rgieseke
Copy link
Contributor

To get started on #111 I looked into what would need to be changed. I hope I found everything and am happy to update this if you prefer any other naming scheme. I chose renaming them from anthroEmisisons to ffiEmissions.

This is in line with RCP data where the category is called 'Fossil fuels and Industry'. In the CSV files the fossil and land use emissions are renamed from ffiEmissions and lucEmissions to ffi_emissions and
luc_emissions for consistency with other categories.

This is in line with RCP data where the category is called 'Fossil fuels
and Industry'. In the CSV files the fossil and land use emissions are
renamed from ffiEmissions and lucEmissions to ffi_emissions and
luc_emissions for consistency with other categories.
@bpbond
Copy link
Member

bpbond commented Sep 17, 2015

Thanks @rgieseke. This looks good and makes a lot of sense; @kvcalvin notes it's entirely consistent with Integrated Assessment terminology too.

The one issue is that this is a major change, in that it changes both the model's inputs and outputs. Per our use of semantic versioning, it should be merged into a (not yet existing) v2 branch, not the current master. @rplzzz ?

@rplzzz
Copy link
Contributor

rplzzz commented Sep 17, 2015

@bpbond Yes, I agree. We can create an rc2.0 branch. As far as I know, there is no way to retarget a pull request, but it's easy enough to do the merge by hand.

@rplzzz
Copy link
Contributor

rplzzz commented Nov 10, 2015

@bpbond Did we decide that this was ready to go onto the (yet to be created) rc2.0 branch? Or is there still stuff to do on it?

@bpbond
Copy link
Member

bpbond commented Nov 10, 2015

Ready to go afaik. Note though we (w/ @Chartin ) at least) still need to discuss branch management at some point though.

@rplzzz
Copy link
Contributor

rplzzz commented Nov 10, 2015

Yeah. We also need to figure out how we're going to manage the documentation, since it lives in the wiki, not the repository. And we'll need to come up with a plan for the next release.

I'll make the new branch and resolve the conflicts with this one this afternoon, so at least that part will be ready to go.

@rgieseke
Copy link
Contributor Author

Some comments/questions:

  • What's actually covered by LUC?
    There is "land use, land use change, and forestry" (LULUCF) but also "Agriculture, Forestry and Other Land Use" (AFOLU)[1]
    Not sure which one is the most appropriate ...
  • Column names
    SOx and NH3 have no trailing "_emissions" in their name. I'd either remove "_emissions" for all (the csv file is likely already named "emissions_scenname") or add it to those two as well for consistency.

Happy to make any amendments to this pull request as you see fit ...

[1] http://theredddesk.org/encyclopaedia/agriculture-forestry-and-other-land-uses

@rplzzz
Copy link
Contributor

rplzzz commented Nov 10, 2015

Since Hector takes its LUC emissions as inputs, "LUC" includes whatever the user chose to include in their input data. When you use hector with GCAM the emissions you get are AFOLU.

@rgieseke
Copy link
Contributor Author

Should it be called AFOLU then, as this seems to be the broadest category?

@cahartin
Copy link
Contributor

anything that does not end in _emissions does not get read into Hector and should be deleted from the emissions files.

@rplzzz
Copy link
Contributor

rplzzz commented Nov 10, 2015

@rgieseke I'll leave that to @bpbond and @cahartin to decide, but personally I think the potential for confusion is small enough to deal with by just adding clarifying notes to the documentation, rather than by generating another change to the API.

@rgieseke
Copy link
Contributor Author

Sure, this makes sense. Just thought to mention it now, as the input files need changes anyway ...

@Chartin
Copy link

Chartin commented Nov 10, 2015

I'm not the chartin you're looking for. Remove me from the thread.
On Nov 10, 2015 7:49 AM, "Ben Bond-Lamberty" notifications@github.com
wrote:

Ready to go afaik. Note though we (w/ @Chartin
https://github.com/chartin ) at least) still need to discuss branch
management at some point though.


Reply to this email directly or view it on GitHub
#116 (comment).

@bpbond
Copy link
Member

bpbond commented Nov 10, 2015

Sorry about that @Chartin . Just click "unsubscribe".

@Chartin
Copy link

Chartin commented Nov 11, 2015

Click where?

On Tue, Nov 10, 2015 at 2:55 PM, Ben Bond-Lamberty <notifications@github.com

wrote:

Sorry about that @Chartin https://github.com/Chartin . Just click
"unsubscribe".


Reply to this email directly or view it on GitHub
#116 (comment).

@bpbond
Copy link
Member

bpbond commented Nov 12, 2015

@Chartin : here's a link to this discussion #116 (comment) . On the webpage, look on the right side for the "Unsubscribe" button. Click it. Hope this resolves it!

@rplzzz rplzzz added this to the v2.0 milestone Nov 21, 2015
@rplzzz
Copy link
Contributor

rplzzz commented Nov 21, 2015

This branch has been merged into the rc2.0.0 branch. It will be merged into master when we are ready for the 2.0 release.

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

Successfully merging this pull request may close these issues.

None yet

5 participants