pik-piam / remind

Contains the REMIND-specific routines for data and model output manipulation.
3 stars 18 forks source link

inconsistent treatment of subset regions in reportEmi() #97

Closed 0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q closed 3 years ago

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 3 years ago

https://github.com/pik-piam/remind/blob/997f83065a7b8e62c5f735e791d7304e78dbe841/R/reportEmi.R#L29-L33

If output is NULL to begin with (i.e., the function is run alone), it will contain names(regionSubsetList) afterwards (e.g., EUR and NEU). If it is not NULL, however, (i.e., the function is run from convGDX2MIX()), it won't.
Shouldn't there be a consistent way to handle this?

(Traces back to https://redmine.pik-potsdam.de/projects/mo/repository/16/revisions/13975/diff/remind/R/reportEmi.R)

Renato-Rodrigues commented 3 years ago

I am not sure I understood what the problem is. :)

I assume the reportEmi function only wants as input the pure REMIND regions without any aggregated regions. In the first case as you are running standalone, the regionSubsetList will be empty in the reportFE call and you only have to remove the world region (GLO) from the magpie object, in the second case, as you are not running standalone, you need to remove both GLO (the World region) and whatever additional region aggregations were added.

I am not sure from your message if you have any bug related with that or if you are just suggesting that there are more elegant ways to write this part of the code.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 3 years ago

I assume the reportEmi function only wants as input the pure REMIND regions without any aggregated regions. In the first case as you are running standalone, the regionSubsetList will be empty in the reportFE call and you only have to remove the world region (GLO) from the magpie object,

This presumes regionSubsetList to be empty. If it is not (maybe because somebody is debugging the function interactively) you effectively get all individual regions "and whatever additional region aggregations were added". And a laundry-list of errors.

So reportEmi() structurally depends on non-documented requirements on its inputs. Or would you have guessed that simply shifting reportEmi() to the top of the pile in convGDX2MIF() would break it?

Renato-Rodrigues commented 3 years ago

A simply solution for this special case is to just remove the regionSubsetList parameter on the second line that you copied above (it will assume to be null then inside of the reportFE function).

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 3 years ago

I'm working through this right now. regionSubsetList could use more elegant programming throughout the package. And documentation ;)