pik-piam / magpie4

R package | MAgPIE outputs R package for MAgPIE version 4.x
GNU Lesser General Public License v3.0
1 stars 23 forks source link

trade function returns a warning for recent magpie runs #5

Closed tscheypidi closed 4 years ago

tscheypidi commented 4 years ago

The trade function returns a warning about inconcistencies in wood and woodfuel demand and production:

In trade(gdx, level = "regglo", type = "net-exports") : For the following categories, demand EXCEEDS production (????) (on top of balanceflow): wood, woodfuel

This needs to be fixed. Either in the trade function or in MAgPIE itself (not sure where the problem originates from)

abhimishr commented 4 years ago

This is coming from

products <- expand.set(gdx, products)

production<-production(gdx,level=level,products=products,product_aggr=product_aggr,attributes=attributes)

demand <- dimSums(demand(gdx,level=level,products=products,product_aggr=product_aggr,attributes=attributes),dim=3.1)

At this point if you make a diff, wood and woodfuel doesn't exist and in principle it shouldn't make a warning (because calculation is based on k_trade).

But at the point of calculating diff we re-run the production function again instead of using production from above. This of-course causes issues of wood and woodfuel being brought over again (from kall)

diff <- (production(gdx,level="glo")-dimSums(demand(gdx,level="glo"),dim=3.1))

Here it throws an error.

Material demand for wood and woodfuel should be made 0 because there is no production from anywhere. Same issue @bodirsky and I were facing some time ago and one fix was to include wood and woodfuel in trade set but will need to be tested. If it can be fixed with inclusion in trade module I can make a PR again. If it has to come from materials module and if its fine by @bodirsky I can do that as well.

abhimishr commented 4 years ago

It can also stay like this until I merge my dynamic forestry branch because there its fixed.

abhimishr commented 4 years ago

Should be fixed by d178abd081853c680cde62b27a78c74ff3580e1f

Please re-open this if that's not sufficient.

tscheypidi commented 4 years ago

I think I have to reopen it:

The fix in trade.R does not look correct to me: Assume for instance a case in which demand is bigger than demand for wood as well as for another non-forestry good. In this case your message about wood and woodfuel would be returned, but the other commodity ignored.

Other case would be an inconsistency in a non-forestry good, but 0 production for wood and woodfuel. Same situation here (the problem itself would be omitted).

In addition, your explanations suggest, that there is an inconsistency in the model. In that case the warning should pop up as something needs to be fixed in the model, right?

Is it possible to create a PR for the develop branch which is fixing this?

tscheypidi commented 4 years ago

and could you please revert your trade fix in the magpie4 package?

abhimishr commented 4 years ago

The fix in trade.R does not look correct to me: Assume for instance a case in which demand is bigger than demand for wood as well as for another non-forestry good. In this case your message about wood and woodfuel would be returned, but the other commodity ignored.

This is a special case. If demand exceeds production for any other commodity which is not in kforestrythe other message will be printed.

Is it possible to create a PR for the develop branch which is fixing this?

My current dev branch doesn't have this. If it needs to go via fixing material demand for wood and woodfuel to 0 then the module author has to fix it (if needed). Material demand doesn't come from dynamic forestry implementation and I won't be able to test it at the moment. This warning will be redundant anyways when dynamic forestry is merged to MAgPIE 4.2.x or 4.3.

abhimishr commented 4 years ago

Done via 19fe88a949bb80b6b25cce93cb298285e4355e72 and additional message printed for runs without dynamic forestry. Warning message will still persist.

abhimishr commented 4 years ago

Can this be closed?

abhimishr commented 4 years ago

possible fix with https://github.com/magpiemodel/magpie/pull/187 in case approved and merged

abhimishr commented 4 years ago

This was fixed from 0b8b395d280d6fe7f21b48819d3de003f3a7dfc6