Generate MILESTONYR and modify gams scaffold to include it #119
Conversation
| tables_in_file = { | ||
| "ts.dd": ["ALL_TS"], | ||
| "milestonyr.dd": ["MILESTONYR"], |
There was a problem hiding this comment.
I decided to call it milestony.dd instead of milestonyr.inc so that our existing dd_to_csv.py script will automatically convert it and count this table as existing in the ground truth, so once we add this file to demos-dd, the additional rows regression should be resolved
There was a problem hiding this comment.
Actually this is what we would like avoid, because in the existing workflow milestone years are not generated into a dd file, but are added to the run file. Also we should be able to pass the name of the milestone year definition for to the tool to use.
There was a problem hiding this comment.
I don't understand. We want to use the milestone years as computed by our tool from the period definitions in the input XL files, right? Then the tool must output it somewhere, whether the run file or a separate file. (and it's cleaner for it to be output to a separate file than for us to find the appropriate line in scenario.run and modify it)
And similarly, we need to know be given the ground truth values of milestone years for the demos-dd. Otherwise how do we check if our tool is computing them correctly?
Perhaps I'm missing something obvious :)
There was a problem hiding this comment.
Sorry for not being clear on this one. We would need to expand our testing to do this correctly. If we want to test whether our tool generates correct milestone years, we should include information on all the milestone years definitions in the benchmarks and then compare those to the ones generated by the tool.
There was a problem hiding this comment.
we need to be given the ground truth values of milestone years for the demos-dd
Comment: There is no unique ground truth to that, other than what I have said before: The actual set used in any model run must be a subset of the full set of milestone years according to the active period definition, such that when the subset contains N members, those members must be equal to the first N years from that full set. It is a free choice of the user to select the period definition (if several have been defined) and then the subset thereof when launching a model run, and the second choice does not even affect the contents of the DD files (the first choice does affect the B and E parameters). Therefore, I think the tool should also support making that choice, like VEDA does.
There was a problem hiding this comment.
Yes, we plan to add a command line flag to enable the user to pick. But for the tests it is sufficient for the ground truth to contain any valid milestone years.
| @@ -51,7 +51,7 @@ $ BATINCLUDE initmty.mod | |||
| $IF NOT DECLARED REG_BNDCST $Abort "You need to use TIMES v2.3.1 or higher" | |||
|
|
|||
| $BATINCLUDE output.dd | |||
| SET MILESTONYR /2005,2006/; | |||
| $BATINCLUDE milestonyr.dd | |||
| tables_in_file = { | ||
| "ts.dd": ["ALL_TS"], | ||
| "milestonyr.dd": ["MILESTONYR"], |
There was a problem hiding this comment.
"milestonyr.inc": ["MILESTONYR"],
This PR generates MILESTONYR (but does not implement the alternative method from #97).
In order for this to pass regression tests, we also need to add
milestonyr.ddto each demo in olejandro/demos-dd#1 (if they are correctly generated?)Fixes #97 (right?)