Integration #3
Integration #3
Conversation
…to unused file (for now) with DICE now being created using the "traditional" Mimi functions.
|
@davidanthoff this is ready for review, I don't have the permissions to add you as a reviewer |
src/dice2013-defmodel.jl
Outdated
| @@ -0,0 +1,117 @@ | |||
| module dice2013 | |||
There was a problem hiding this comment.
Remove this file for now, we don't want @defmodel to be public.
src/dice2013.jl
Outdated
| set_param!(DICE, :welfare, :rr, p[:rr]) | ||
| set_param!(DICE, :welfare, :scale1, p[:scale1]) | ||
| set_param!(DICE, :welfare, :scale2, p[:scale2]) | ||
| connect_param!(DICE, :welfare, :CPC, :neteconomy, :CPC, offset=0) |
There was a problem hiding this comment.
This whole part should be in a function (like in the old version), not global scope code. We generally should not define global variables like this here.
| include("dice2013.jl") | ||
| using dice2013 | ||
|
|
||
| run(DICE) |
There was a problem hiding this comment.
Change this so that we first call some function that gets a model, assign it to m and then run that.
|
|
||
| function getmarginal_dice_models(;emissionyear=2010) | ||
| m1 = getdiceexcel() | ||
| mm = MarginalModel(DICE) |
There was a problem hiding this comment.
Same here, don't use a global like DICE.
src/marginaldamage.jl
Outdated
| # run(m2) | ||
|
|
||
| # return m1, m2 | ||
| # end |
There was a problem hiding this comment.
Don't keep old code around, it is always in the git history if someone wants to see it.
test/runtests.jl
Outdated
| using ExcelReaders | ||
| using DataTables |
There was a problem hiding this comment.
That package is long dead, we should use DataFrames.
src/dice2013.jl
Outdated
| @@ -1,8 +1,10 @@ | |||
| module dice2013 | |||
There was a problem hiding this comment.
I think we should probably follow the convention that module names start with a capital letter here? I think we can also not put this into a module for now, given that we'll have to change this again once we make this a package.
There was a problem hiding this comment.
It seemed to me good practice to define models in their own modules since we use module name + component name to uniquely identify components. Maybe DICE2013? I would think this will eventually be the name of the package/project in any case, right?
There was a problem hiding this comment.
Yes, I think you are right. So maybe then just lets pick the name we will also use for the package. I guess the options are really DICE2013 or Dice2013. I don't feel strongly about it... The first one is more true to the model name, the second one to the julia naming guidelines...
Updated DICE 2013 for new framework.